[cfe-commits] [PATCH] PR12917: fix lambda mangling within templates

[Richard Smith]

On Fri, Jul 20, 2012 at 8:45 AM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Jul 17, 2012, at 6:09 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Tue, Jul 17, 2012 at 4:35 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
>>
>> On Jul 16, 2012, at 3:28 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> > Hi,
>> >
>> > As PR12917 notes, the mangling of lambads within templates is currently
>> wrong -- not only do we not follow the ABI, we sometimes give the same
>> mangled name to multiple lambdas. We use the templated declaration as the
>> mangling context rather than the instantiation, and we don't take into
>> account that the mangling number can depend on the number of template
>> arguments if the template is variadic.
>> >
>> > The attached patch fixes this by delaying the computation of the
>> mangling information for a lambda in a dependent context until after
>> instantiation.
>> >
>> > Please review!
>>
>> @@ -128,9 +126,12 @@
>>        }
>>      }
>>
>> +    unsigned ManglingNumber;
>>      switch (Kind) {
>>        case Normal:
>> -        if (CurContext->isDependentContext() ||
>> isInInlineFunction(CurContext))
>> +        if ((!ActiveTemplateInstantiations.empty() &&
>> +             !(ContextDecl && isa<ParmVarDecl>(ContextDecl))) ||
>> +            isInInlineFunction(CurContext))
>>            ManglingNumber = Context.getLambdaManglingNumber(Method);
>>          else
>>            ManglingNumber = 0;
>> @@ -140,7 +141,10 @@
>>          break;
>>
>>        case StaticDataMember:
>> -        if (!CurContext->isDependentContext()) {
>> +        if (ActiveTemplateInstantiations.empty()) {
>> +          // A mangling number for a lambda in a static data member of a
>> class
>> +          // template is only assigned when the data member is
>> instantiated
>> +          // from the template.
>>            ManglingNumber = 0;
>>            ContextDecl = 0;
>>            break;
>>
>> I don't understand why we're checking
>> ActiveTemplateInstantiations.empty() in these cases. That only tells us
>> whether there's an instantiation occurring at any point; it isn't
>> necessarily the right answer if we're instantiating, e.g., the declaration
>> of a member template within a class template instantiation.
>
>
> I don't think that matters: we only care about the mangling details for
> fully-instantiated lambdas.
>
>
> ... and because this only happens for default arguments, the bodies of
> functions, and the initializers of variables, it never happens in the
> "substituting into something that may still end up being a template" phase.
> Gotcha, thanks!
>

Tidied up a bit and checked in as r160614. I added a (strictly speaking,
unnecessary) check for CurContext->isDependentContext() to the
ActiveTemplateInstantiations check, since if we somehow managed to get a
reference to an uninstantiated lambda into a mangled name, we would want a
mangling number to be assigned to it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120722/d4bad881/attachment.html>