erik.pilkington added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1267-1269
+ // Instantiate the attributes on both the template declaration and the
associated record declaration.
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs,
StartingScope);
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst,
LateAttrs, StartingScope);
----------------
nit: 80 chars.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1268
+ // Instantiate the attributes on both the template declaration and the
associated record declaration.
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs,
StartingScope);
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst,
LateAttrs, StartingScope);
----------------
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.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1498
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
StartingScope);
+
----------------
This line looks too long too.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3247
InstPartialSpec->setTypeAsWritten(WrittenTy);
// Check the completed partial specialization.
----------------
ldionne wrote:
> 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.
>
You mean `SemaRef.InstantiateAttrsForDecl(TemplateArgs, PartialSpec,
InstPartialSpec, LateAttrs, StartingScope);`?
================
Comment at: clang/test/SemaCXX/PR38913.cpp:19
+};
+C<int>::X<int, long>* c() { return 0; } // CHECK-DAG: @_Z1cB4CTAGv
+
----------------
ldionne wrote:
> This test is failing in the current iteration of the patch.
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.
Repository:
rC Clang
https://reviews.llvm.org/D51997
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
