This revision was automatically updated to reflect the committed changes.
Closed by commit rC342600: Thread safety analysis: Handle ObjCIvarRefExpr in
SExprBuilder::translate (authored by aaronpuchert, committed by ).
Changed prior to commit:
aaronpuchert added a comment.
It seems that `self` is an ordinary `DeclRefExpr` unlike `this`, which is a
`CXXThisExpr`. Which means we'd have to make it dependent on the name whether
we drop it, but `self` in C/C++ is just an ordinary variable. So I think I'll
leave the `self->` part for now.
delesley added inline comments.
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:362
+ til::Project *P = new (Arena) til::Project(E, D);
+ if (hasCppPointerType(BE))
+P->setArrow(true);
aaronpuchert wrote:
> rjmccall wrote:
> > aaron.ballman wrote:
> > > I
rjmccall added a comment.
In https://reviews.llvm.org/D52200#1239007, @aaronpuchert wrote:
> I think it should be possible to get rid of `self->` in the warning message
> if we want to, after all `this->` is omitted in C++ as well.
Hmm. It would be consistent to apply the same rule to both
aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.
I think it should be possible to get rid of `self->` in the warning message if
we want to, after all `this->` is omitted in C++ as well.
Comment at: test/SemaObjCXX/warn-thread-safety-analysis.mm:42
aaronpuchert updated this revision to Diff 166051.
aaronpuchert added a comment.
Detect ObjC pointer types as well as ordinary pointers.
Repository:
rC Clang
https://reviews.llvm.org/D52200
Files:
include/clang/Analysis/Analyses/ThreadSafetyCommon.h
lib/Analysis/ThreadSafetyCommon.cpp
aaronpuchert added a comment.
I found something that would theoretically work:
P->setArrow((isa(ME->getBase()) && Ctx && Ctx->SelfArg) ?
Ctx->SelfArrow : ME->isArrow());
So if we have `this` and a context that tells us we have to replace `this` by
something else, then we check
aaronpuchert planned changes to this revision.
aaronpuchert added a comment.
Thanks to both of you for the reviews. I'll see what I can do about the arrows.
My gut feeling is that using `{Member,ObjCIVarRef}Expr::isArrow` is the right
way, but it's not yet obvious to me how.
rjmccall added inline comments.
Comment at: lib/Analysis/ThreadSafetyCommon.cpp:362
+ til::Project *P = new (Arena) til::Project(E, D);
+ if (hasCppPointerType(BE))
+P->setArrow(true);
aaron.ballman wrote:
> I feel like this will always return false.
aaron.ballman added a reviewer: rjmccall.
aaron.ballman added a comment.
Adding a reviewer who knows more about ObjC than I do.
Comment at: include/clang/Analysis/Analyses/ThreadSafetyCommon.h:400
til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx);
+
aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley, lukasza.
Herald added a subscriber: cfe-commits.
This imitates the code for MemberExpr. I hope it is right, for I have
absolutely no understanding of ObjC++.
Fixes 38896.
Repository:
rC Clang
11 matches
Mail list logo
