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

[John McCall via cfe-commits]

rjmccall added a comment.

Needs more tests.

1. Dependent template instantiation.
2. Non-dependent template instantiation.
3. Indexes which are themselves pseudo-objects.
4. Templated getters and setters.
5. Non-POD by-value argument types.


================
Comment at: include/clang/AST/BuiltinTypes.def:253
@@ -251,1 +252,3 @@
+PLACEHOLDER_TYPE(MSPropertySubscript, MSPropertySubscriptTy)
+
 #ifdef LAST_BUILTIN_TYPE
----------------
Hmm.  I don't think you need a new placeholder type.  These subscripts are still an ordinary pseudo-object; you can load and store at any point.  Instead, you should detect this case by the expression node used, which can either be an ArraySubscriptExpr or a new node if you find it necessary.

================
Comment at: lib/Sema/SemaExpr.cpp:3934
@@ -3921,1 +3933,3 @@
+  if (!IsMSPropertySubscript &&
+      base->getType()->isNonOverloadPlaceholderType()) {
     ExprResult result = CheckPlaceholderExpr(base);
----------------
I think what you should do here is:
1. Detect whether the base type has pseudo-object type.
2. If so, give the pseudo-object code a chance to work on the expression.  In most cases, it will just load; but if the base is an MSPropertyRefExpr of array type, or an ArraySubscriptExpr, it can leave it alone.
3. If the pseudo-object code returned an expression that's still of pseudo-object type, just build an ArraySubscriptExpr of pseudo-object type after you've folded pseudo-objects in the index as well.

================
Comment at: lib/Sema/SemaExpr.cpp:4115
@@ -4097,1 +4114,3 @@
+          BuiltinType::MSPropertySubscript);
+  if (!LHSExp->getType()->getAs<VectorType>() && !IsMSPropertySubscript) {
     ExprResult Result = DefaultFunctionArrayLvalueConversion(LHSExp);
----------------
Placeholders should never get here.

================
Comment at: lib/Sema/SemaPseudoObject.cpp:1458
@@ -1433,4 +1457,3 @@
   }
 
   return S.ActOnCallExpr(S.getCurScope(), GetterExpr.get(),
----------------
Don't modify ArgExprs in buildGet or buildSet; you might need to build both, e.g. when building a compound assignment or inc/dec.

Since you need to copy into a new array anyway, you might as well accumulate them index expressions in stack order and then reverse them when adding them to this array.


http://reviews.llvm.org/D13336