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

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I found something that would theoretically work:

  P->setArrow((isa<CXXThisExpr>(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 `Ctx->SelfArrow`, otherwise we take `ME->isArrow()`.

But that doesn't work. When translating into the TIL (typed intermediate language), referencing and dereferencing operators are completely ignored.

  struct Foo {
    Mutex mu_;
    void foo1(Foo *f_declared) EXCLUSIVE_LOCKS_REQUIRED(f_declared->mu_);
  };
  
  void test() {
    Foo myfoo;
    myfoo.foo1(&myfoo);  // \
      // expected-warning {{calling function 'foo1' requires holding mutex 'myfoo.mu_' exclusively}}
  }

With the above change we warn that `calling function 'foo1' requires holding mutex 'myfoo->mu_' exclusively`. It should be `(&myfoo)->mu_`, but the `&` is lost. So we can't derive the information that we want from `isArrow` alone.

Now there is a reason why these operators are ignored — the TIL tries to "canonicalize" expressions, so that it detects that `(&myfoo)->mu_` and `myfoo.mu_` are the same thing. Changing that is probably possible, but beyond the scope of this change.

Short of that, we must be able to detect pointers. I think we could use `Type::isAnyPointerType` instead of `Type::isPointerType` in `hasCppPointerType` (and probably rename that).

For later I think we should consider a different canonicalization that doesn't just ignore `&` and `*`.


Repository:
  rC Clang

https://reviews.llvm.org/D52200