[PATCH] D13336: [MSVC] 'property' with an empty array in array subscript expression.

[John McCall via cfe-commits]

rjmccall added inline comments.

================
Comment at: include/clang/AST/ExprCXX.h:689
@@ +688,3 @@
+/// indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b), and
+/// p->x[a][b] = i will be turned into p->PutX(a, b, i).
+class MSPropertySubscriptExpr : public Expr {
----------------
Please add to the comment here that this is a syntactic pseudo-object expression.

================
Comment at: lib/Sema/SemaExpr.cpp:3926
@@ -3921,1 +3925,3 @@
+  if (base->getType()->isNonOverloadPlaceholderType() &&
+      !IsMSPropertySubscript) {
     ExprResult result = CheckPlaceholderExpr(base);
----------------
Please factor this check so that it only does extra work when the base has placeholder type, and please extract a function to compute IsMSPropertySubscript.

================
Comment at: lib/Sema/SemaPseudoObject.cpp:1419
@@ +1418,3 @@
+MSPropertyOpBuilder::getBaseMSProperty(MSPropertySubscriptExpr *E) {
+  Expr *Base = E->getBase();
+  CallArgs.insert(CallArgs.begin(), E->getIdx());
----------------
This code will look slightly nicer and be slightly more efficient if you maintain the invariant that parens have been ignored on Base.  That is, IgnoreParens here and when assigning in the loop, and then you can remove them from the type-checks.

Oh.  You also need to capture the index expressions so that they aren't evaluated multiple times when this expression is used as the l-value of an increment, decrement, or compound-assignment.  You then need to un-capture them in the Rebuilder.  Please add code-generation tests that check that the index expression is only evaluated once in these cases.


http://reviews.llvm.org/D13336