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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
