aaron.ballman added a comment.
I think the CI failure (libarcher.races::lock-unrelated.c) is not related to
this patch but is a tsan thing, but you may want to double-check that just in
case.
I think this looks reasonable, but I'd like to hear from @delesley before
landing.
================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:643
sls_mu.Lock(); // \
- // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
+ // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared
in the same scope}}
} while (getBool());
----------------
aaronpuchert wrote:
> These are just swapped because I'm merging the branches in a different order
> now. I think that's Ok.
I think the new order is actually an improvement because it diagnoses the
second acquisition (diagnosing the first is a bit weirdly predictive for my
tastes).
================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:2806-2816
+void loopReleaseContinue() {
+ RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{mutex
acquired here}}
+ // We have to warn on this join point despite the lock being managed ...
+ for (unsigned i = 1; i < 10; ++i) {
+ x = 1; // ... because we might miss that this doesn't always happen under
lock.
+ if (i == 5) {
+ scope.Unlock();
----------------
How about a test like:
```
void foo() {
RelockableMutexLock scope(&mu, ExclusiveTraits{});
for (unsigned i = 1; i < 10; ++i) {
if (i > 0) // Always true
scope.Lock(); // So we always lock
x = 1; // So I'd assume we don't warn
}
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104261/new/
https://reviews.llvm.org/D104261
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
