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

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.


================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1275
+    const ValueDecl *VD = LP->clangDecl();
+    return VD->isDefinedOutsideFunctionOrMethod();
+  }
----------------
aaron.ballman wrote:
> Hmm, I've not seen that function used a whole lot before, but looking at the implementation of it, I think it does what we need it to do here. FWIW, I was expecting something more like this:
> ```
> if (const DeclContext *DC = VD->getLexicalDeclContext())
>   return !DC->getRedeclContext()->isFunctionOrMethod();
> ```
> But I'm not certain if this would ever give a different answer from your approach.
Never seen it before as well, but it does a `DC = DC->getParent()` loop, so there is probably a difference. Think about `DC` being a local struct or Objective-C block declaration.


================
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'}}
+  }
----------------
@lebedev.ri, I think that's pretty much your case. On my original change, this would have also warned about `scope`, not just `mu`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84604