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

[John McCall]

On Jan 27, 2012, at 7:56 PM, Eli Friedman wrote:
> On Fri, Jan 27, 2012 at 3:26 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Fri, Jan 27, 2012 at 3:12 PM, John McCall <rjmccall at apple.com> wrote:
>>> 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?
>> 
>> In essence, things which are constant-expressions in the C++ grammar
>> are missing.  Not too hard to add them, I guess.
> 
> New version attached; it doesn't pass regression tests because I still
> need to add a hook to catch VLA array bounds attached to declarations,
> but is this the direction you were thinking of?

That's the general idea, although I didn't carefully look at where you added
the calls.  Two things, though:
  - The dynamic optimization of not doing this traversal unless we saw
    a DeclRef of a not-yet-used-but-const-and-whatever-else declaration
    is going to be quite important.
  - isDependentContext() is actually both (a) surprisingly expensive and
    (b) extremely easy for Sema to cache.

John.