[cfe-dev] static constexpr member function of a local struct in a function template is instantiated too late

[Michael Park]

Ah, ok. Thanks Richard!


On 25 August 2014 15:56, Richard Smith <richard at metafoo.co.uk> wrote:

> On Fri, Aug 22, 2014 at 11:39 AM, Michael Park <mcypark at gmail.com> wrote:
>
>> I've followed http://llvm.org/docs/DeveloperPolicy.html#one-off-patches
>> and http://llvm.org/docs/GettingStarted.html#sending-patches-with-git to
>> send an email to *cfe-commits* mailing list with a *git diff* as an
>> attachment. Compared to other emails though my seem to look different so
>> I think I may have done it wrong?
>>
>
> It's fine. One minor thing: we prefer -p0 patches, and git produces -p1
> patches by default. Use 'git diff --no-prefix'.
>
>
>>  On 21 August 2014 21:40, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>>> On Thu, Aug 21, 2014 at 6:22 PM, Michael Park <mcypark at gmail.com> wrote:
>>>
>>>> Hi Richard, thanks for the reply!
>>>>
>>>>  Funnily enough, the member functions of local classes that have
>>>> deduced return types actually don't run into this problem because the
>>>> definition gets instantiated eagerly in order to deduce the return type.
>>>> So the following actually works in clang++-3.4.2.
>>>>
>>>>  template <typename T>
>>>>> void F() {
>>>>>   struct Num {
>>>>>     static constexpr auto Get() { return 42; }
>>>>>   };
>>>>>   static_assert(Num::Get() == 42, "");
>>>>>
>>>>> }
>>>>> int main() {
>>>>>   F<int>();
>>>>> }
>>>>>
>>>>
>>>>  I tried the recommended approach of instantiating at the end of the
>>>> local class instead, like so:
>>>>
>>>> // DR1484 clarifies that the members of a local class are instantiated
>>>>> as part
>>>>> // of the instantiation of their enclosing entity.
>>>>> if (D->isCompleteDefinition() && D->isLocalClass()) {
>>>>>   SemaRef.InstantiateClass(D->getLocation(), Record, D, TemplateArgs,
>>>>>                            TSK_ImplicitInstantiation,
>>>>>                            /*Complain=*/true);
>>>>>   SemaRef.InstantiateClassMembers(D->getLocation(), Record,
>>>>> TemplateArgs,
>>>>>                                   TSK_ImplicitInstantiation);
>>>>>
>>>> *  SemaRef.PerformPendingInstantiations(/*LocalOnly=*/true);*
>>>>
>>>> }
>>>>
>>>>
>>>> Although there are *PeformPendingInstantiations *calls that should be
>>>> removed, it effectively will be a no-op so I just tried this out for now.
>>>> It actually passes *make test* and also passes the example above!
>>>>
>>>> But I worry that instantiating at the end of the local class is still
>>>> too late, because I run into the same issue if I bring the
>>>> *static_assert* into the body of the local struct.
>>>>
>>>> template <typename T>
>>>>
>>>> void F() {
>>>>
>>>>    struct Num {
>>>>>     static constexpr int Get() { return 42; }
>>>>>     static_assert(range == 42, "");
>>>>>   };
>>>>
>>>> }
>>>>
>>>>
>>> The core language is somewhat unclear on whether that's supposed to
>>> work. Note that it doesn't work if F is not a template. I think the
>>> standard committee will end up deciding that this case is ill-formed, so
>>> I'm happy for us to reject it for now.
>>>
>>> This case also works if I change the return type of *Get* to *auto*.
>>>>
>>>> Even I bring out the local struct to namespace scope, it still breaks.
>>>> But I think this might be a different bug. (In this case changing *Get*'s
>>>> return type to *auto* doesn't resolve the problem.)
>>>>
>>>
>>> =) Yep. See
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1626
>>>
>>>
>>>> struct Num {
>>>>>   static constexpr int Get() { return 42; }
>>>>>    static_assert(Get() == 42, "");
>>>>> };
>>>>
>>>>
>>>> P.S. Thanks so much for helping out.
>>>>
>>>>
>>>> On 20 August 2014 22:31, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>
>>>>> On Tue, Aug 12, 2014 at 8:09 PM, Michael Park <mcypark at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Hello,
>>>>>>
>>>>>
>>>>> Hi and welcome! (And sorry for the delay.)
>>>>>
>>>>>
>>>>>> I'm new to clang development and was hoping to get some help in
>>>>>> tackling this bug that I found/filed:
>>>>>> http://llvm.org/bugs/show_bug.cgi?id=20625
>>>>>>
>>>>>> Example:
>>>>>>
>>>>>> template <typename T>
>>>>>>> void F() {
>>>>>>>   struct Num {
>>>>>>>     static constexpr int Get() { return 42; }
>>>>>>>   };
>>>>>>>   constexpr int n = Num::Get();
>>>>>>> }
>>>>>>> int main() {
>>>>>>>   F<int>();
>>>>>>> }
>>>>>>
>>>>>>
>>>>>> The summary of the issue is that the local struct *Num* and its
>>>>>> static constexpr function *Get* is not yet instantiated when
>>>>>> we validate the constexpr variable initialization of *n* during *F*'s
>>>>>> instantiation.
>>>>>>
>>>>>> More details:
>>>>>>
>>>>>> *Num* and *Get* gets added to *PendingLocalImplicitInstantiations*
>>>>>> in *SubstStmt* at *SemaTemplateInstantiationDecl.cpp:3437*, and is
>>>>>> instantiated later by *PerformPendingInstantiations* at
>>>>>> *SemaTemplateInstantiationDecl.cpp:3458*. However, the validation of
>>>>>> the constexpr variable initialization of *n* happens in *SubstStmt* at
>>>>>> which point we don't yet have the definition of *Get* instantiated.
>>>>>>
>>>>>> I'm not exactly sure what the correct approach would be to solve the
>>>>>> problem. It seems that the local implicit instantiations need to be
>>>>>> performed earlier, or the validation of constexpr variables need to be
>>>>>> delayed.
>>>>>>
>>>>>> If someone can point me in the right direction it would be
>>>>>> appreciated.
>>>>>>
>>>>>
>>>>> The implicit instantiations should be performed sooner. (FWIW, the
>>>>> same problem will affect member functions of local classes that have
>>>>> deduced return types.) Perhaps the easiest way to address this would be to
>>>>> perform all the local implicit instantiations discovered while
>>>>> instantiating a local class when we reach the end of that local class,
>>>>> instead of delaying them until the end of the surrounding function. We try
>>>>> to delay these instantiations as much as we can, to reduce the stack usage
>>>>> in recursive template instantiation, but that's probably not going to be
>>>>> significant here because there's not likely to be many layers of AST
>>>>> between the function and the class definition.
>>>>>
>>>>> One place to put this fix would be around
>>>>> SemaTemplateInstantiateDecl.cpp:1104:
>>>>>
>>>>>   // DR1484 clarifies that the members of a local class are
>>>>> instantiated as part
>>>>>   // of the instantiation of their enclosing entity.
>>>>>   if (D->isCompleteDefinition() && D->isLocalClass()) {
>>>>>     SemaRef.InstantiateClass(D->getLocation(), Record, D, TemplateArgs,
>>>>>                              TSK_ImplicitInstantiation,
>>>>>                              /*Complain=*/true);
>>>>>     SemaRef.InstantiateClassMembers(D->getLocation(), Record,
>>>>> TemplateArgs,
>>>>>                                     TSK_ImplicitInstantiation);
>>>>>   }
>>>>>
>>>>> You could save, perform, and restore the pending local implicit
>>>>> instantiations around these calls.
>>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140825/62cadb90/attachment.html>