[PATCH] Instantiate static constexpr member function of a local struct in a function template earlier.

[Michael Park]

ping.

On 30 April 2015 at 22:46, Michael Park <mcypark at gmail.com> wrote:

> I've got a follow-up patch which I think addresses an existing bug that
> hadn't surfaced.
>
> In the *ParseLateTemplatedFuncDef* function in
> *lib/Parse/ParseTemplate.cpp*, we rebuild the *DeclContext* starting from
> the function decl context up to the translation unit context with the
> following logic:
>
>   // Get the list of DeclContexts to reenter.
>>   SmallVector<DeclContext*, 4> DeclContextsToReenter;
>>   DeclContext *DD = FunD;
>>   while (DD && *!DD->isTranslationUnit()*) {
>>     DeclContextsToReenter.push_back(DD);
>>     DD = DD->getLexicalParent();
>>   }
>
>
> Then we reenter the sequence of decl contexts in reverse, with
> *PushDeclContext*.
>
>   // Reenter template scopes from outermost to innermost.
>>   SmallVectorImpl<DeclContext *>::reverse_iterator II =
>>       DeclContextsToReenter.rbegin();
>>   for (; II != DeclContextsToReenter.rend(); ++II) {
>>     TemplateParamScopeStack.push_back(new ParseScope(this,
>>           Scope::TemplateParamScope));
>>     unsigned NumParamLists =
>>       Actions.ActOnReenterTemplateScope(getCurScope(), cast<Decl>(*II));
>>     CurTemplateDepthTracker.addDepth(NumParamLists);
>>     if (*II != FunD) {
>>       TemplateParamScopeStack.push_back(new ParseScope(this,
>> Scope::DeclScope));
>>
>> *Actions.PushDeclContext(Actions.getCurScope(), *II);*    }
>>   }
>
>
> This means that *Actions*'* CurContext* must be the translation unit
> context, otherwise the assertion in *PushDeclContext* fails. I think the
> intention of the following *ContextRAII* was to satisfy this
> pre-condition, but instead we save the *CurContext* and stay in the
> *CurContext*.
>
> // To restore the context after late parsing.
>> Sema::ContextRAII GlobalSavedContext(Actions, Actions.CurContext);
>
>
> The fix is to save the *CurContext* then jump to the translation unit
> context in order to satisfy the pre-conditions necessary to rebuild and
> reenter the decl contexts.
>
> // To restore the context after late parsing.
>> Sema::ContextRAII GlobalSavedContext(
>
>     Actions, Actions.Context.getTranslationUnitDecl());
>
>
> On 1 May 2015 at 01:25, Michael Park <mcypark at gmail.com> wrote:
>
>> I've got a follow-up patch which I think addresses an existing bug that
>> hadn't surfaced.
>>
>> In the *ParseLateTemplatedFuncDef* function in
>> *lib/Parse/ParseTemplate.cpp*, we rebuild the *DeclContext* starting
>> from the function decl context up to the translation unit context with the
>> following logic:
>>
>>   // Get the list of DeclContexts to reenter.
>>>   SmallVector<DeclContext*, 4> DeclContextsToReenter;
>>>   DeclContext *DD = FunD;
>>>   while (DD && *!DD->isTranslationUnit()*) {
>>>     DeclContextsToReenter.push_back(DD);
>>>     DD = DD->getLexicalParent();
>>>   }
>>
>>
>> Then we reenter the sequence of decl contexts in reverse, with
>> *PushDeclContext*.
>>
>>   // Reenter template scopes from outermost to innermost.
>>>   SmallVectorImpl<DeclContext *>::reverse_iterator II =
>>>       DeclContextsToReenter.rbegin();
>>>   for (; II != DeclContextsToReenter.rend(); ++II) {
>>>     TemplateParamScopeStack.push_back(new ParseScope(this,
>>>           Scope::TemplateParamScope));
>>>     unsigned NumParamLists =
>>>       Actions.ActOnReenterTemplateScope(getCurScope(), cast<Decl>(*II));
>>>     CurTemplateDepthTracker.addDepth(NumParamLists);
>>>     if (*II != FunD) {
>>>       TemplateParamScopeStack.push_back(new ParseScope(this,
>>> Scope::DeclScope));
>>>
>>> *Actions.PushDeclContext(Actions.getCurScope(), *II);*    }
>>>   }
>>
>>
>> This means that *Actions*'* CurContext* must be the translation unit
>> context, otherwise the assertion in *PushDeclContext* fails. I think the
>> intention of the following *ContextRAII* was to satisfy this
>> pre-condition, but instead we save the *CurContext* and stay in the
>> *CurContext*.
>>
>> // To restore the context after late parsing.
>>> Sema::ContextRAII GlobalSavedContext(Actions, Actions.CurContext);
>>
>>
>> The fix is to save the *CurContext* then jump to the translation unit
>> context in order to satisfy the pre-conditions necessary to rebuild and
>> reenter the decl contexts.
>>
>> // To restore the context after late parsing.
>>> Sema::ContextRAII GlobalSavedContext(
>>
>>     Actions, Actions.Context.getTranslationUnitDecl());
>>
>>
>> On 1 May 2015 at 01:12, Michael Park <mcypark at gmail.com> wrote:
>>
>>> I've got a follow-up patch which I think addresses an existing bug that
>>> hadn't surfaced.
>>>
>>> In the *ParseLateTemplatedFuncDef* function in
>>> *lib/Parse/ParseTemplate.cpp*, we build up the sequence of DeclContext
>>>
>>>
>>> On 29 April 2015 at 11:20, Michael Park <mcypark at gmail.com> wrote:
>>>
>>>> Ah, that's unfortunate. I'll take a look. Thanks!
>>>>
>>>> On 28 April 2015 at 20:33, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>
>>>>> One of our buildbots seems unhappy:
>>>>>
>>>>>
>>>>> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/11939/steps/test_all/logs/Clang-Unit%20%3A%3A%20ASTMatchers__ASTMatchersTests__HasAncestor.MatchesClosestAncestor
>>>>>
>>>>> Its testcase is this:
>>>>>
>>>>>       template <typename T> struct C {
>>>>>         void f(int) {
>>>>>           struct I { void g(T) { int x; } } i; i.g(42);
>>>>>         }
>>>>>       };
>>>>>       template struct C<int>;
>>>>>
>>>>> It looks like the problem is some interaction of your patch and
>>>>> delayed template parsing (running the above code through clang with
>>>>> -fdelayed-template-parsing reproduces the issue). Can you take a look?
>>>>>
>>>>> On Tue, Apr 28, 2015 at 5:23 PM, Michael Park <mcypark at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Awesome, Thanks Richard!
>>>>>>
>>>>>> On 28 April 2015 at 20:11, Richard Smith <richard at metafoo.co.uk>
>>>>>> wrote:
>>>>>>
>>>>>>> On Tue, Apr 28, 2015 at 4:00 PM, Michael Park <mcypark at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> The answer is that we don't really know. It would be more
>>>>>>>>> consistent with the non-template case for it to be ill-formed (it also
>>>>>>>>> seems reasonable to say that you can't instantiate a member function until
>>>>>>>>> its class is complete), but the point of instantiation rules for constexpr
>>>>>>>>> functions aren't settled yet; this is part of DR1581 (still open).
>>>>>>>>>
>>>>>>>>
>>>>>>>> Agreed. Thanks for the pointer to DR1581!
>>>>>>>>
>>>>>>>> LGTM
>>>>>>>>
>>>>>>>>
>>>>>>>> As this is my first patch for Clang, I'm not all that familiar with
>>>>>>>> the required process to get this patch committed. Am I required to do
>>>>>>>> anything further on my end?
>>>>>>>>
>>>>>>>
>>>>>>> I've committed your patch as r236063. Feel free to go ahead and
>>>>>>> mark PR20625 as fixed. Thanks!
>>>>>>>
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> MPark.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150507/6a1f7608/attachment.html>