[PATCH] D59523: Thread Safety: also look at ObjC methods
JF Bastien via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 22 13:42:23 PDT 2019
jfb added inline comments.
================
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) {
----------------
aaron.ballman wrote:
> Please don't use `auto` when the type is not explicitly spelled out in the initialization.
It's a `DeclContext` as spelled in `getDeclContext`, but sure I can drop the `auto` nonetheless.
================
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:282
+ if (Ctx && Ctx->FunArgs) {
+ const auto *Canonical = Ctx->AttrDecl->getCanonicalDecl();
+ if (isa<FunctionDecl>(D)
----------------
aaron.ballman wrote:
> Same.
It's a `Decl` (that's canonical), but same.
================
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:288
+ assert(I < Ctx->NumArgs);
+ return translate(Ctx->FunArgs[I], Ctx->Prev);
+ }
----------------
aaronpuchert wrote:
> Does your test run into this with an `ObjCMethodDecl`? I see how we run into the assignment to `VD` down below, but I don't understand how we could get here without annotating a method.
The problem is in the cast above:
```
(lldb)
frame #5: clang::threadSafety::SExprBuilder::translateDeclRefExpr(this, DRE, Ctx) at ThreadSafetyCommon.cpp:280:9
277 // Function parameters require substitution and/or renaming.
278 if (const auto *PV = dyn_cast_or_null<ParmVarDecl>(VD)) {
279 const auto *FD =
-> 280 cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl();
281 unsigned I = PV->getFunctionScopeIndex();
282
283 if (Ctx && Ctx->FunArgs && FD == Ctx->AttrDecl->getCanonicalDecl()) {
```
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59523/new/
https://reviews.llvm.org/D59523
More information about the cfe-commits
mailing list