Question about Decls representing member classes of class templates

Hi,

This is a noob question.

I'm working on the no_extern_template attribute discussed at [1] and I encountered something I don't understand in Clang. I traced through the code but couldn't find an answer, so I thought I'd ask here in case the question is easy to answer for someone else.

I have the following class with an explicit template instantiation:

    template <typename T>
    struct Foo {
        struct __attribute__((no_extern_template)) member_class { };
    };

    template struct Foo<int>;

The no_extern_template is the attribute I'm trying to add. My problem is that in Sema::InstantiateClassMembers (triggered by the explicit instantiation), the Decl representing member_class does not seem to have any attribute applied to it. However, I was able to trace code in ProcessDeclAttribute() (in Sema/SemaDeclAttr.cpp) where a Decl representing member_class does have the no_extern_template associated to it. I also noticed that the Decl in ProcessDeclAttribute() and in Sema::InstantiateClassMembers() were different Decl objects, but both representing the same type (or at least they have the same getNameAsString()). Note that I don't have the same problem with static/non-static member functions -- only member classes seem to have multiple Decls going around with different attributes.

My questions are:
- Why are there two Decl instances representing the same class?
- How can I make sure that all Decl instances representing the same class get the same attributes?

I uploaded the patch I have so far here to give some context: ⚙ D51789 [clang] Add the exclude_from_explicit_instantiation attribute

Thanks,
Louis

[1]: http://lists.llvm.org/pipermail/cfe-dev/2018-August/059024.html

I seem to remember adding a bit to the Attr.td class to do exactly this (have an attribute propagate to instantiations). It seems to have changed names unfortunately and my git-fu seems to have failed me here, but there if I would guess, I'd say the following is the current incantation:

// Set to true if this attribute meaningful when applied to or inherited
  // in a class template definition.
  bit MeaningfulToClassTemplateDefinition = 0;

You should be able to put
"MeaningfulToClassTemplateDefintion = 1" in your review and have it work I think?

Thanks for your reply. Unfortunately, naively adding

  let MeaningfulToClassTemplateDefinition = 1;

to the attribute definition does not seem to solve the problem. In Sema::InstantiateClassMembers(), I still see:

  InstantiateClassMembers(): D->getNameAsString() = member_class
  InstantiateClassMembers(): D->hasAttr<NoExternTemplateAttr>() = false

Like I said, this does not happen for member functions or static data members. Member classes must be treated specially somehow but I can’t find out where.

Louis

For functions and static data members, we instantiate their attributes as part of instantiating the declaration. For classes, we only instantiate most attributes as part of instantiating the definition; only attributes marked as “MeaningfulToClassTemplateDefinition” are instantiated as part of the declaration. (And yes, that name is reversed from what the flag means, and in any case should not mention templates as it should affect classes not templates…)

However, it looks like we’re missing the call to instantiate the early-instantiated attributes when we instantiate a member class of a class template specialization. (See TemplateDeclInstantiator::VisitCXXRecordDecl, and note that it does not call InstantiateAttrsForDecl.) The same problem appears to exist for member class templates (TemplateDeclInstantiator::VisitClassTemplateDecl), member partial specializations (TemplateDeclInstantiator::InstantiateClassTemplatePartialSpecialization), and member explicit specializations (TemplateDeclInstantiator::VisitClassTemplateSpecializationDecl).

This bug can easily be observed prior to your patch:

template struct A { struct attribute((abi_tag(“foo”))) X {}; };
#if CHANGE_MANGLING
A::X axi;
#endif
A::X *f() { return 0; }

Note that with CHANGE_MANGLING not defined, we mangle f as _Z1fv, but with it defined, we instead mangle f as _Z1fB3foov. Oops!

So, we should fix all those places above so that they call InstantiateAttrsForDecl, as a precursor to your patch. We should also rename MeaningfulToClassTemplateDefinition to something that reflects what it actually means. (Maybe reverse the sense of it and rename to OnlyMeaningfulOnDefinition, or rename it to InstantiateWithDeclaration, or something like that.)