[clang] Make sure attributes on member classes are applied properly

[ldionne]

Summary

Attributes on member classes of class templates (and other similar entities)
are not currently instantiated. This was discovered by Richard Smith here:

http://lists.llvm.org/pipermail/cfe-dev/2018-September/059291.html

This commit makes sure that attributes are instantiated properly.

PR38913

[ldionne]

This patch is missing support for partial specializations and explicit specializations. The part I don't understand is how to get the CXXRecordDecls to have the right attribute below. Here's the AST dump for the test file with the current patch:

TranslationUnitDecl 0x7f8a2a00a8e8 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x7f8a2a00b1c0 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x7f8a2a00ae80 '__int128'
|-TypedefDecl 0x7f8a2a00b228 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x7f8a2a00aea0 'unsigned __int128'
|-TypedefDecl 0x7f8a2a00b558 <<invalid sloc>> <invalid sloc> implicit __NSConstantString '__NSConstantString_tag'
| `-RecordType 0x7f8a2a00b300 '__NSConstantString_tag'
|   `-CXXRecord 0x7f8a2a00b278 '__NSConstantString_tag'
|-TypedefDecl 0x7f8a28827600 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x7f8a2a00b5b0 'char *'
|   `-BuiltinType 0x7f8a2a00a980 'char'
|-TypedefDecl 0x7f8a28827928 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list '__va_list_tag [1]'
| `-ConstantArrayType 0x7f8a288278d0 '__va_list_tag [1]' 1
|   `-RecordType 0x7f8a288276e0 '__va_list_tag'
|     `-CXXRecord 0x7f8a28827650 '__va_list_tag'
|-ClassTemplateDecl 0x7f8a28827ab8 </Users/ldionne/work/clang-attribute/clang/test/SemaCXX/PR38913.cpp:7:1, col:77> col:26 A
| |-TemplateTypeParmDecl 0x7f8a28827978 <col:10, col:16> col:16 class depth 0 index 0 T
| |-CXXRecordDecl 0x7f8a28827a30 <col:19, col:77> col:26 struct A definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-CXXRecordDecl 0x7f8a28827d00 <col:19, col:26> col:26 implicit struct A
| | `-CXXRecordDecl 0x7f8a28827e38 <col:30, col:74> col:70 struct X definition
| |   |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| |   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |   | |-MoveConstructor exists simple trivial needs_implicit
| |   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |   | |-MoveAssignment exists simple trivial needs_implicit
| |   | `-Destructor simple irrelevant trivial needs_implicit
| |   |-AbiTagAttr 0x7f8a28827f48 <col:52, col:66> ATAG
| |   `-CXXRecordDecl 0x7f8a28827fa8 <col:30, col:70> col:70 implicit struct X
| `-ClassTemplateSpecializationDecl 0x7f8a28828088 <col:1, col:77> col:26 struct A definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveConstructor exists simple trivial needs_implicit
|   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   |-CXXRecordDecl 0x7f8a28828280 prev 0x7f8a28828088 <col:19, col:26> col:26 implicit struct A
|   `-CXXRecordDecl 0x7f8a28828308 <col:30, col:70> col:70 referenced struct X
|     `-AbiTagAttr 0x7f8a288283b0 <col:52> ATAG
|-FunctionDecl 0x7f8a2885a200 <line:8:1, col:28> col:12 a 'A<int>::X *()'
| `-CompoundStmt 0x7f8a2885a328 <col:16, col:28>
|   `-ReturnStmt 0x7f8a2885a310 <col:18, col:25>
|     `-ImplicitCastExpr 0x7f8a2885a2f8 <col:25> 'A<int>::X *' <NullToPointer>
|       `-IntegerLiteral 0x7f8a2885a2d8 <col:25> 'int' 0
|-ClassTemplateDecl 0x7f8a2885a448 <line:11:1, col:95> col:26 B
| |-TemplateTypeParmDecl 0x7f8a2885a340 <col:10, col:16> col:16 class depth 0 index 0 T
| |-CXXRecordDecl 0x7f8a2885a3c0 <col:19, col:95> col:26 struct B definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-CXXRecordDecl 0x7f8a2885a690 <col:19, col:26> col:26 implicit struct B
| | `-ClassTemplateDecl 0x7f8a2885a888 <col:30, col:92> col:88 X
| |   |-TemplateTypeParmDecl 0x7f8a2885a718 <col:39, col:45> col:45 class depth 1 index 0 U
| |   `-CXXRecordDecl 0x7f8a2885a800 <col:48, col:92> col:88 struct X definition
| |     |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |     | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| |     | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |     | |-MoveConstructor exists simple trivial needs_implicit
| |     | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |     | |-MoveAssignment exists simple trivial needs_implicit
| |     | `-Destructor simple irrelevant trivial needs_implicit
| |     |-AbiTagAttr 0x7f8a2885aad0 <col:70, col:84> BTAG
| |     `-CXXRecordDecl 0x7f8a2885ab38 <col:48, col:88> col:88 implicit struct X
| `-ClassTemplateSpecializationDecl 0x7f8a2885ac18 <col:1, col:95> col:26 struct B definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveConstructor exists simple trivial needs_implicit
|   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   |-CXXRecordDecl 0x7f8a2885adf8 prev 0x7f8a2885ac18 <col:19, col:26> col:26 implicit struct B
|   `-ClassTemplateDecl 0x7f8a2885af88 <col:30, col:88> col:88 X
|     |-TemplateTypeParmDecl 0x7f8a2885ae80 <col:39, col:45> col:45 class depth 0 index 0 U
|     |-CXXRecordDecl 0x7f8a2885af00 <col:48, col:88> col:88 struct X
|     | `-AbiTagAttr 0x7f8a2885b038 <col:70> BTAG
|     |-ClassTemplateSpecializationDecl 0x7f8a2885b290 <col:30, col:88> col:88 struct X
|     | |-TemplateArgument type 'int'
|     | `-AbiTagAttr 0x7f8a2885b390 <col:70> BTAG
|     `-AbiTagAttr 0x7f8a2885afd8 <col:70> BTAG
|-FunctionDecl 0x7f8a2885b5d8 <line:12:1, col:33> col:17 b 'B<int>::X<int> *()'
| `-CompoundStmt 0x7f8a2885b6c0 <col:21, col:33>
|   `-ReturnStmt 0x7f8a2885b6a8 <col:23, col:30>
|     `-ImplicitCastExpr 0x7f8a2885b690 <col:30> 'B<int>::X<int> *' <NullToPointer>
|       `-IntegerLiteral 0x7f8a2885b670 <col:30> 'int' 0
|-ClassTemplateDecl 0x7f8a2885b7e8 <line:15:1, line:18:1> line:15:26 C
| |-TemplateTypeParmDecl 0x7f8a2885b6d8 <col:10, col:16> col:16 class depth 0 index 0 T
| |-CXXRecordDecl 0x7f8a2885b760 <col:19, line:18:1> line:15:26 struct C definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-CXXRecordDecl 0x7f8a2885ba30 <col:19, col:26> col:26 implicit struct C
| | |-ClassTemplateDecl 0x7f8a2885bc70 <line:16:3, col:41> col:37 X
| | | |-TemplateTypeParmDecl 0x7f8a2885bab8 <col:12, col:18> col:18 class depth 1 index 0 U
| | | |-TemplateTypeParmDecl 0x7f8a2885bb28 <col:21, col:27> col:27 class depth 1 index 1 V
| | | `-CXXRecordDecl 0x7f8a2885bbe8 <col:30, col:41> col:37 struct X definition
| | |   |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | |   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | |   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |   | |-MoveConstructor exists simple trivial needs_implicit
| | |   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | |   | |-MoveAssignment exists simple trivial needs_implicit
| | |   | `-Destructor simple irrelevant trivial needs_implicit
| | |   `-CXXRecordDecl 0x7f8a2885bf00 <col:30, col:37> col:37 implicit struct X
| | `-ClassTemplatePartialSpecializationDecl 0x7f8a29803000 <line:17:3, col:73> col:61 struct X definition
| |   |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| |   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |   | |-MoveConstructor exists simple trivial needs_implicit
| |   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |   | |-MoveAssignment exists simple trivial needs_implicit
| |   | `-Destructor simple irrelevant trivial needs_implicit
| |   |-TemplateArgument type 'int'
| |   |-TemplateArgument type 'type-parameter-1-0'
| |   |-TemplateTypeParmDecl 0x7f8a2885bfa8 <col:12, col:18> col:18 referenced class depth 1 index 0 V
| |   |-AbiTagAttr 0x7f8a29803138 <col:43, col:57> CTAG
| |   `-CXXRecordDecl 0x7f8a298032b8 <col:21, col:61> col:61 implicit struct X
| `-ClassTemplateSpecializationDecl 0x7f8a29803398 <line:15:1, line:18:1> line:15:26 struct C definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveConstructor exists simple trivial needs_implicit
|   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   |-CXXRecordDecl 0x7f8a29803578 prev 0x7f8a29803398 <col:19, col:26> col:26 implicit struct C
|   `-ClassTemplateDecl 0x7f8a298037b0 <line:16:3, col:37> col:37 X
|     |-TemplateTypeParmDecl 0x7f8a29803600 <col:12, col:18> col:18 class depth 0 index 0 U
|     |-TemplateTypeParmDecl 0x7f8a29803668 <col:21, col:27> col:27 class depth 0 index 1 V
|     |-CXXRecordDecl 0x7f8a29803728 <col:30, col:37> col:37 struct X
|     `-ClassTemplateSpecializationDecl 0x7f8a29803e30 <col:3, col:37> col:37 struct X
|       |-TemplateArgument type 'int'
|       `-TemplateArgument type 'long'
|-FunctionDecl 0x7f8a29804990 <line:19:1, col:39> col:23 c 'C<int>::X<int, long> *()'
| `-CompoundStmt 0x7f8a29804a78 <col:27, col:39>
|   `-ReturnStmt 0x7f8a29804a60 <col:29, col:36>
|     `-ImplicitCastExpr 0x7f8a29804a48 <col:36> 'C<int>::X<int, long> *' <NullToPointer>
|       `-IntegerLiteral 0x7f8a29804a28 <col:36> 'int' 0
|-ClassTemplateDecl 0x7f8a29804b98 <line:22:1, line:25:1> line:22:26 D
| |-TemplateTypeParmDecl 0x7f8a29804a90 <col:10, col:16> col:16 class depth 0 index 0 T
| |-CXXRecordDecl 0x7f8a29804b10 <col:19, line:25:1> line:22:26 struct D definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-CXXRecordDecl 0x7f8a29804de0 <col:19, col:26> col:26 implicit struct D
| | |-ClassTemplateDecl 0x7f8a29804f78 <line:23:3, col:32> col:28 X
| | | |-TemplateTypeParmDecl 0x7f8a29804e68 <col:12, col:18> col:18 class depth 1 index 0 U
| | | |-CXXRecordDecl 0x7f8a29804ef0 <col:21, col:32> col:28 struct X definition
| | | | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | | | |-MoveConstructor exists simple trivial needs_implicit
| | | | | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | | | | |-MoveAssignment exists simple trivial needs_implicit
| | | | | `-Destructor simple irrelevant trivial needs_implicit
| | | | `-CXXRecordDecl 0x7f8a298051c0 <col:21, col:28> col:28 implicit struct X
| | | `-ClassTemplateSpecialization 0x7f8a298052c0 'X'
| | `-ClassTemplateSpecializationDecl 0x7f8a298052c0 <line:24:3, col:63> col:54 struct X definition
| |   |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| |   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |   | |-MoveConstructor exists simple trivial needs_implicit
| |   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |   | |-MoveAssignment exists simple trivial needs_implicit
| |   | `-Destructor simple irrelevant trivial needs_implicit
| |   |-TemplateArgument type 'int'
| |   |-AbiTagAttr 0x7f8a29805470 <col:36, col:50> DTAG
| |   `-CXXRecordDecl 0x7f8a298055c8 <col:14, col:54> col:54 implicit struct X
| `-ClassTemplateSpecializationDecl 0x7f8a298056a8 <line:22:1, line:25:1> line:22:26 struct D definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveConstructor exists simple trivial needs_implicit
|   | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   |-CXXRecordDecl 0x7f8a2a03b6a8 prev 0x7f8a298056a8 <col:19, col:26> col:26 implicit struct D
|   |-ClassTemplateDecl 0x7f8a2a03b838 <line:23:3, col:28> col:28 X
|   | |-TemplateTypeParmDecl 0x7f8a2a03b730 <col:12, col:18> col:18 class depth 0 index 0 U
|   | |-CXXRecordDecl 0x7f8a2a03b7b0 <col:21, col:28> col:28 struct X
|   | `-ClassTemplateSpecialization 0x7f8a2a03ba18 'X'
|   `-ClassTemplateSpecializationDecl 0x7f8a2a03ba18 <line:24:3, col:63> col:54 struct X definition
|     |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|     | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
|     | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
|     | |-MoveConstructor exists simple trivial needs_implicit
|     | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
|     | |-MoveAssignment exists simple trivial needs_implicit
|     | `-Destructor simple irrelevant trivial needs_implicit
|     |-TemplateArgument type 'int'
|     |-AbiTagAttr 0x7f8a2a03bbe0 <col:36> DTAG
|     `-CXXRecordDecl 0x7f8a2a03bcb0 prev 0x7f8a2a03ba18 <col:14, col:54> col:54 implicit struct X
`-FunctionDecl 0x7f8a2a03bf98 <line:26:1, col:33> col:17 d 'D<int>::X<int> *()'
  `-CompoundStmt 0x7f8a2a03c080 <col:21, col:33>
    `-ReturnStmt 0x7f8a2a03c068 <col:23, col:30>
      `-ImplicitCastExpr 0x7f8a2a03c050 <col:30> 'D<int>::X<int> *' <NullToPointer>
        `-IntegerLiteral 0x7f8a2a03c030 <col:30> 'int' 0

As you can see, the CXXRecordDecls have the right attributes for the member class and member class template cases, but not for the partial and explicit specializations.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

2971 ↗

(On Diff #165120)

I tried a couple variations of things like:

SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, InstD, LateAttrs, StartingScope);

and

SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, ClassTemplate->getTemplatedDecl(), LateAttrs, StartingScope);

For some reason, none of them end up instantiating the attribute on the CXXRecordDecl that is used to determine the mangling.

3247 ↗

(On Diff #165120)

I tried adding

SemaRef.InstantiateAttrsForDecl(TemplateArgs, ClassTemplate->getTemplatedDecl(), InstPartialSpec, LateAttrs, StartingScope);

here, but just like for explicit specializations, that doesn't instantiate the attributes on the CXXRecordDecl used to determine mangling.

clang/test/SemaCXX/PR38913.cpp

19 ↗

(On Diff #165120)

This test is failing in the current iteration of the patch.

26 ↗

(On Diff #165120)

This test is failing in the current iteration of the patch.

[erik.pilkington]

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1267–1269 ↗

(On Diff #165120)

nit: 80 chars.

1268 ↗

(On Diff #165120)

Why are you instantiating the attributes onto the ClassTemplateDecl? At the very least it seems wrong to instantiate attributes from the pattern to the ClassTemplateDecl.

1498 ↗

(On Diff #165120)

This line looks too long too.

3247 ↗

(On Diff #165120)

You mean SemaRef.InstantiateAttrsForDecl(TemplateArgs, PartialSpec, InstPartialSpec, LateAttrs, StartingScope);?

clang/test/SemaCXX/PR38913.cpp

19 ↗

(On Diff #165120)

I think the problem here is that we don't instantiate X<int, long> because it's behind the pointer, so we're just considering the primary template. Looks like we do add the attribute (with the fix above) here:

template<class T> struct C {
  template<class U, class V> struct X { };
  template<class V> struct __attribute__((abi_tag("CTAG"))) X<int, V> { };
};
C<int>::X<int, long> c() { return 0; }

But we don't get the right mangling, for some reason. Note that this problem is present even in the non-member case.

I don't think attributes on specializations have particularly good support anyways, so I wouldn't really lose any sleep if we "left this to a follow-up". Maybe @rsmith has some thoughts here.

[ldionne]

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1268 ↗

(On Diff #165120)

My understanding was that

• Inst represented B<int>::X<T> (the template)
• RecordInst represented B<int>::X<int> (the template specialization, which is a "real struct")
• Pattern represented B<T>::X<T>

If that is right, then it seemed to make sense that both B<int>::X<T> and B<int>::X<int> should have the attribute, since B<T>::X<T> has it. I might be misunderstanding something though.

clang/test/SemaCXX/PR38913.cpp

19 ↗

(On Diff #165120)

I'll drop support for specializations for now. The current patch is enough to unblock me on the no_extern_template attribute, which is the goal of all of this.

[rsmith]

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1268 ↗

(On Diff #165120)

That's almost right.

• Inst represents B<int>::X (the template declaration in the instantiation of B<int>).
• RecordInst represents B<int>::X<U> (the class declaration within the template declaration in the instantiation of B<int>).
• Pattern represents B<T>::X<U> (the class declaration within the template declaration in the definition of B<int>).

And we don't yet have a value for U, so there is no B<int>::X<int> here.

That is:

template<typename T>
  struct B {
    template<typename U> // #1
      struct X; // Pattern
  };
// implicit instantiation of B<int>:
template<> struct B<int> {
  template<typename U> // Inst
    struct X; // RecordInst
};

A template-declaration can never have attributes on it (there is no syntactic location where they can be written), so we should never be instantiating attributes onto it. If it could, we should instantiate those attributes from #1, not from Pattern.

3247 ↗

(On Diff #165120)

erik's suggestion would be the right thing to do. This case is for instantiating a class-scope partial specialization inside a template to form another class-scope partial specialization:

template<typename T> struct A {
  template<typename U> struct B;
  template<typename U> struct ATTRS B<U*>; // PartialSpec
};
// implicit instantiation of A<int> looks like:
template<> struct A<int> {
  template<typename U> struct B;
  template<typename U> struct ATTRS B<U*>; // InstPartialSpec
};

... where we want to instantiate attributes from PartialSpec to InstPartialSpec. Unlike in the class template case, we don't have separate AST nodes for the template and the class within it for partial specializations (I think our general consensus is that's a design mistake, but addressing it would be a substantial refactoring).

clang/test/SemaCXX/PR38913.cpp

19 ↗

(On Diff #165120)

This should call TemplateDeclInstantiator::VisitCXXRecordDecl to create the member class declaration when instantiating the outer class template specialization C<int>. I'm not sure why that isn't instantiating the attributes from the declaration.

[ldionne]

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1268 ↗

(On Diff #165120)

Thanks a lot for the explanation -- this is really helpful. It's now clear that this line should not be there.

2971 ↗

(On Diff #165120)

So, assuming this:

template<class T> struct Foo {
  template<class U> struct X { };
  template<> struct ATTRS X<int> { };
};

Based on what @rsmith says about partial specializations, my understanding is that:

• D is the explicit specialization inside the general class template: Foo<T>::X<int>
• InstD is the explicit specialization inside the specialization of Foo<int>: Foo<int>::X<int>

And hence, what we want to do here is apply the attributes from Foo<T>::X<int> onto Foo<int>::X<int>:

SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, InstD, LateAttrs, StartingScope);

However, like I said previously this is not sufficient and I'll just not implement this for specializations for the time being (for the sake of making progress on no_extern_template).

[ldionne]

Drop support for partial specializations and explicit specializations.
Address comments by Erik.

[ldionne]

I created https://bugs.llvm.org/show_bug.cgi?id=38942 to keep track of the problem for partial specializations and explicit specializations. Moving forward with this patch will allow me to land the no_extern_template attribute.

[erik.pilkington]

Looks good to me! @rsmith should probably have the final word though. Thanks for fixing this.

lib/Sema/SemaTemplateInstantiateDecl.cpp

Decl *TemplateDeclInstantiator::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   CXXRecordDecl *RecordInst = CXXRecordDecl::Create(
       SemaRef.Context, Pattern->getTagKind(), DC, Pattern->getBeginLoc(),
       Pattern->getLocation(), Pattern->getIdentifier(), PrevDecl,
       /*DelayTypeCreation=*/true);
 
   if (QualifierLoc)
     RecordInst->setQualifierInfo(QualifierLoc);
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs,
+                                                              StartingScope);
+
   ClassTemplateDecl *Inst
     = ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(),
                                 D->getIdentifier(), InstParams, RecordInst);
   assert(!(isFriend && Owner->isDependentContext()));
   Inst->setPreviousDecl(PrevClassTemplate);
 
   RecordInst->setDescribedClassTemplate(Inst);
 
Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
   CXXRecordDecl *Record = CXXRecordDecl::Create(
       SemaRef.Context, D->getTagKind(), Owner, D->getBeginLoc(),
       D->getLocation(), D->getIdentifier(), PrevDecl);
 
   // Substitute the nested name specifier, if any.
   if (SubstQualifier(D, Record))
     return nullptr;
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
+                                                              StartingScope);
+
   Record->setImplicit(D->isImplicit());
   // FIXME: Check against AS_none is an ugly hack to work around the issue that
   // the tag decls introduced by friend class declarations don't have an access
   // specifier. Remove once this area of the code gets sorted out.
   if (D->getAccess() != AS_none)
     Record->setAccess(D->getAccess());
   if (!D->isInjectedClassName())
     Record->setInstantiationOfMemberClass(D, TSK_ImplicitInstantiation);

test/SemaCXX/PR38913.cpp

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+// PR38913
+// Check that we instantiate attributes on declarations for...
+
+// ...a member class of a class template specialization
+template<class T> struct A { struct __attribute__((abi_tag("ATAG"))) X { }; };
+A<int>::X* a() { return 0; } // CHECK-DAG: @_Z1aB4ATAGv
+
+// ...a member class template
+template<class T> struct B { template<class U> struct __attribute__((abi_tag("BTAG"))) X { }; };
+B<int>::X<int>* b() { return 0; } // CHECK-DAG: @_Z1bB4BTAGv