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

[Alexey Bataev via cfe-commits]

ABataev marked an inline comment as done.

================
Comment at: lib/Sema/SemaPseudoObject.cpp:1627
@@ -1579,1 +1626,3 @@
+        cast<MSPropertyRefExpr>(Base->IgnoreParens())->getBaseExpr());
+    return MSPropertyRefRebuilder(S, BaseOVE->getSourceExpr()).rebuild(E);
   } else {
----------------
rjmccall wrote:
> Hmm.  Just like the ObjCSubscriptRefExpr case, this will need to strip the OVEs off the index expressions, or else you'll end up stacking OVEs and asserting in IRGen.
> 
> The deeper problem here is that we have too much redundancy in all these places that have to walk over the different pseudo-object expressions.  We can remove some of that by simplifying the design of Rebuilder, which is really over-engineered for its task.  It should be fine to make it non-templated, merge all of the rebuildSpecific cases into it (and therefore remove all of the subclasses), and give it a callback to invoke at all the capture points in source order.
> 
> I think the callback can just be a llvm::function_ref<Expr*(Expr*, unsigned)>.  The second parameter is the position of the capture point in source order, so for example in your case the base of the MSPropertyRefExpr will be 0, the index of the innermost MSPropertySubscriptExpr will be 1, and so on.  That'll be important for callers that care about which capture point is which.
> 
> The various rebuildAndCaptureObject implementations should use a callback that returns the appropriate captured expression for the position.
> 
> stripOpaqueValuesFromPseudoObjectRef should use a callback that returns cast<OpaqueValueExpr>(e)->getSourceExpr(); it shouldn't need to split out the different cases at all.
John, we don't need to strip the OVEs off the index expressions, because index expressions in MSPropertySubscriptExpr are not folded to OVEs. They are folded to OVEs only for CallArgs array. But in rebuilded MSPropertySubscriptExpr only base MSPropertyRefExpr is folded to OVE (we don't need to put indexes in MSPropertySubscriptExpr, because they are not used during codegen and we can keep original expressions). So, the code should work as is.
I reworked Rebuilder just like you said.



http://reviews.llvm.org/D13336