rjmccall added inline comments.

================
Comment at: lib/Sema/SemaPseudoObject.cpp:1627
@@ -1579,1 +1626,3 @@
+        cast<MSPropertyRefExpr>(Base->IgnoreParens())->getBaseExpr());
+    return MSPropertyRefRebuilder(S, BaseOVE->getSourceExpr()).rebuild(E);
   } else {
----------------
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.


http://reviews.llvm.org/D13336



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to