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

[Michael Park]

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/20150501/455e38ab/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr20625.patch
Type: application/octet-stream
Size: 2567 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150501/455e38ab/attachment.obj>