[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[John McCall via Phabricator via cfe-commits]

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. However, I wonder if `IVRE->getBase()->isArrow()` is more appropriate here?
The base of an `ObjCIvarRefExpr` will never directly be a C pointer type.  I don't know whether this data structure looks through casts, but it's certainly *possible* to cast arbitrarily weird C stuff to ObjC pointer type.  On the other hand, by and large nobody actually ever does that, so I wouldn't make a special case for it here.

`ObjCIvarRefExpr` just has an `isArrow()` method directly if this is just supposed to be capturing the difference between `.` and `->`.  But then, so does `MemberExpr`, and in that case you're doing this odd analysis of the base instead of trusting `isArrow()`, so I don't know what to tell you to do.

Overall it is appropriate to treat an ObjC ivar reference as a perfectly ordinary projection.  The only thing that's special about it is that the actual offset isn't necessarily statically known, but ivars still behave as if they're all independently declared, so I wouldn't worry about it.


Repository:
  rC Clang

https://reviews.llvm.org/D52200