aaronpuchert added inline comments.
================ Comment at: clang/lib/Analysis/ThreadSafety.cpp:868 ThreadSafetyHandler &Handler) const override { - if (!managed() && !asserted() && !negative() && !isUniversal()) { + if (!asserted() && !negative() && !isUniversal()) { Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc, ---------------- One might ask: what about asserted capabilities? I plan to introduce a warning when they are released, because that can't be consistent, and then they can't disappear on back edges without warning. For negative capabilities we'd presumably see a warning for the "positive" capability instead. Not sure how universal capabilities are typically used. Presumably one could release such a capability in a loop? Then on the other hand, code using such capabilities is probably not very interested in false negatives. ================ Comment at: clang/lib/Analysis/ThreadSafety.cpp:2227-2228 /// \param LEK2 The error message to report if a mutex is missing from Lset2 void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, const FactSet &FSet2, SourceLocation JoinLoc, ---------------- Presumably we should call these `EntrySet` and `ExitSet` instead? The second parameter is always the exit set of an existing block, the first parameter the entry set of a (sometimes new) block. ================ 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()); ---------------- These are just swapped because I'm merging the branches in a different order now. I think that's Ok. ================ Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:2788 + // We have to warn on this join point despite the lock being managed ... + for (unsigned i = 1; i < 10; ++i) { // expected-warning {{expecting mutex 'mu' to be held at start of each loop}} + x = 1; // ... because we might miss that this doesn't always happen under lock. ---------------- This warning is new. ================ Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:2813 + scope.Unlock(); + continue; // expected-warning {{expecting mutex 'mu' to be held at start of each loop}} + } ---------------- This is also new. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104261/new/ https://reviews.llvm.org/D104261 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits