[PATCH] D111639: [Sema] check PseudoObject when rebuilding CXXOperatorCallExpr in template instantiation

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/Sema/TreeTransform.h:14576
         !Second->getType()->isOverloadableType())
       return getSema().CreateBuiltinArraySubscriptExpr(
           First, Callee->getBeginLoc(), Second, OpLoc);
----------------
Can we come here to subscript pointer types? Something like:

```
struct Foo {
  void putM(int* rhs) { _m = rhs; }
  int* getM() { return _m; }
  __declspec(property(get = getM, put = putM)) int* theData;
};
void bar(Foo *p, int idx) {
  p->theData[idx] = 42;
}
```


================
Comment at: clang/lib/Sema/TreeTransform.h:14578
           First, Callee->getBeginLoc(), Second, OpLoc);
   } else if (Op == OO_Arrow) {
     // -> is never a builtin operation.
----------------
Similarly, do we need to cover pseudo objects in this codepath? `p->theData->m2`?


================
Comment at: clang/test/SemaCXX/PR51855.cpp:27
+// CHECK: call void @_ZN1SC1Es
+// CHECK: call {{(signext )?i16}} @_ZN1S4getMEv
+// CHECK: call void @_ZN1S4putMEs
----------------
There are many platforms with varying default calling conventions. You will either need to generalize these checks to match such conventions, or choose a specific triple to test with.

I can think of two targets to try that might fail these check lines: ARM targets, and i686-windows-gnu, which will use `x86_thiscallcc`.


================
Comment at: clang/test/SemaCXX/PR51855.cpp:71
+// CHECK: call void @_ZN1S4putMEs
+// CHECK: call {{(signext )?i16}} @_ZN1S4getMEv
----------------
Please add test coverage for the interesting operators in the code under edit: OO_Subscript, OO_Arrow, OO_Amp. Please also add equivalent coverage to the ObjC test cases, or give me a link to show that it's already covered.

Maybe OO_Amp is unnecessary, I'm not sure how it is supposed to work for psuedo objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111639