[PATCH] D84604: Thread safety analysis: Consider global variables in scope

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri resigned from this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Thanks.
Note that i didn't check that this doesn't cause any new false-positives



================
Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:87-89
+  void test4() {
+    MutexLock lock(&mu); // expected-warning {{acquiring mutex 'mu' requires negative capability '!mu'}}
+  }
----------------
aaronpuchert wrote:
> lebedev.ri wrote:
> > aaronpuchert wrote:
> > > @lebedev.ri, I think that's pretty much your case. On my original change, this would have also warned about `scope`, not just `mu`.
> > I think i'm missing the point here.
> > I originally reverted this because the diagnostics i was seeing were pretty unambiguously )to me) bogus.
> > But the only test change since then ensures that diagnostic is emitted in some case,
> > there are no tests to ensure it is not emitted in some cases.
> > So either my revert was wrong, or this still is either issuing seemingly bogus diagnostic,
> > or lacks test coverage that it doesn't issue said diagnostic.
> > 
> > Which one is it?
> This test fails on the original, reverted change as follows:
> 
> ```
> error: 'warning' diagnostics seen but not expected: 
>   File [...]/clang/test/SemaCXX/warn-thread-safety-negative.cpp Line 88: acquiring mutex 'lock' requires negative capability '!lock'
> 1 error generated.
> ```
> 
> Maybe you're not familiar with the `-verify` mechanism: it doesn't just check that the expected errors/warnings/notes are emitted, it also checks that nothing more is emitted.
> This test fails on the original, reverted change as follows:

Perfect, thanks for checking/confirming!

> Maybe you're not familiar with the -verify mechanism: it doesn't just check that the expected errors/warnings/notes are emitted, it also checks that nothing more is emitted.

I was aware of that, but did not recall that detail until now. Thank you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84604/new/

https://reviews.llvm.org/D84604