[PATCH] D59523: Thread Safety: also look at ObjC methods

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: delesley.
aaron.ballman added a comment.

Adding Delesley in case he has input on design.



================
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:280
     unsigned I = PV->getFunctionScopeIndex();
-
-    if (Ctx && Ctx->FunArgs && FD == Ctx->AttrDecl->getCanonicalDecl()) {
-      // Substitute call arguments for references to function parameters
-      assert(I < Ctx->NumArgs);
-      return translate(Ctx->FunArgs[I], Ctx->Prev);
+    const auto *D = PV->getDeclContext();
+    if (Ctx && Ctx->FunArgs) {
----------------
Please don't use `auto` when the type is not explicitly spelled out in the initialization.


================
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:282
+    if (Ctx && Ctx->FunArgs) {
+      const auto *Canonical = Ctx->AttrDecl->getCanonicalDecl();
+      if (isa<FunctionDecl>(D)
----------------
Same.


================
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:283-285
+      if (isa<FunctionDecl>(D)
+              ? (cast<FunctionDecl>(D)->getCanonicalDecl() == Canonical)
+              : (cast<ObjCMethodDecl>(D)->getCanonicalDecl() == Canonical)) {
----------------
Also somewhat orthogonal to your changes, but... ugh, the lack of any common base between `FunctionDecl` and `ObjcMethodDecl` strikes again. I sort of wish we would introduce a CallableDecl base class that these two (and BlockDecl) could all inherit from to take care of this sort of ugly hack.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59523