[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: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/bc6ec39a/attachment.html>