[cfe-commits] [PATCH] Remove PotentiallyPotentiallyEvaluated

[John McCall]

On Jan 19, 2012, at 10:57 AM, Eli Friedman wrote:
> On Wed, Jan 18, 2012 at 9:50 PM, John McCall <rjmccall at apple.com> wrote:
>> Also, would you mind measuring the code-size increase from adding a new
>> instance of TreeTransform?
> 
> Roughly 74KB in a Release build.  If we care, TreeTransform could be
> optimized to reduce the penalty.

Okay.  That's a pity, but I think we can live with it.  Thanks.

>> +    // Error on DeclRefExprs referring to FieldDecls.
>> +    // FIXME: This actually isn't supposed to be an error!
>> +    ExprResult TransformDeclRefExpr(DeclRefExpr *E) {
>> 
>> What's up with this FIXME?
> 
> My mistake; fixing the remaining field-reference-in-unevaluated cases
> won't require any changes here.
> 
> Fixed version attached.

+    // We need to special-case DeclRefExprs referring to FieldDecls which
+    // are not part of a member pointer formation; normal TreeTransforming
+    // doesn't catch this case because of the way we represent them in the AST.
+    // FIXME: This is a bit ugly; is it really the best way to handle this
+    // case?
+    // 
+    // Filter out member pointer formation

Missing punctuation.  Also, sorry to harp on this, but this comment
describes the exception before the rule.  We want to complain about
any DeclRefExprs we find that refer to fields, *except* that we happen to
use that representation within member pointer constants, so we need to
ignore those as a special case.

I missed this the first time around, but it doesn't look like you're
guarding against walking back into unevaluated contexts as part
of the TreeTransform.  That is, typeof(foo(sizeof(A::m))) is okay,
even if foo turns out to return a polymorphic l-value.

Once you fix that, I think this looks good, thanks!

John.