[cfe-commits] [PATCH] Model C++11 rules for odr-use for variables correctly

[John McCall]

On Jan 27, 2012, at 2:53 PM, Eli Friedman wrote:
> On Thu, Jan 26, 2012 at 7:29 PM, John McCall <rjmccall at apple.com> wrote:
>> On Jan 24, 2012, at 9:34 PM, Eli Friedman wrote:
>>> Patch attached.  Basically, this is implementing the C++11 rules in
>>> [basic.def.odr]p2 for variables allowed in constant expressions.
>>> (Those rules don't matter so much for most code given how we do code
>>> generation, but we have to get the rules exactly right to get implicit
>>> capture in lambda expressions working correctly.)
>>> 
>>> I don't really like this patch, but it seems like it is necessary.
>> 
>> Yeah, I'm really not happy with this.  Obvious alternative:  mark ODR use
>> at the DRE creation site unless the object is not already marked and it
>> meets the constant-loading criteria.  If it does, flag that this full-expression
>> contains a potential reference, then walk back over the completed
>> full-expression looking for PE DREs not direct operands of L2R casts.
> 
> Is there any existing hook that actually triggers where I need it to?
> MaybeCreateExprWithCleanups seems close... but doesn't quite cover
> everything.

What's missing?

>> +static bool DelayVarODRUsedForType(QualType T) {
>> +  // C++11 [basic.def.odr]p1: A variable or non-overloaded function whose name
>> +  // appears as a potentially-evaluated expression is odr-used unless it is
>> +  // an object that satisfies the requirements for appearing in a
>> +  // constant expression (5.19) and the lvalue-to-rvalue conversion (4.1)
>> +  // is immediately applied.
>> +
>> +  // We don't try to check the constant expression bit here because it gives
>> +  // our assertions better coverage.
>> +
>> +  // Check for an "object".
>> +  if (!T->isObjectType())
>> +    return false;
>> +  // The lvalue-to-rvalue conversion can't be applied to arrays.
>> +  if (T->isArrayType())
>> +    return false;
>> +  // lvalue-to-rvalue conversion doesn't apply to classes.
>> +  // FIXME: That isn't completely true... but I doubt anyone cares about the
>> +  // one edge case where we actually do apply lvalue-to-rvalue conversion.
>> +  if (T->isRecordType())
>> +    return false;
>> +  // FIXME: Need to extend MarkUseAsLValue to misc vector expressions.
>> +  if (T->isVectorType())
>> +    return false;
>> +  return true;
>> +}
>> 
>> This can and should be done on canonical types using direct tests.
>> You can leave the other checks as asserts for equivalence.
> 
> You mean isa<ArrayType>(T) etc.?

Yes.

John.