ldionne marked 2 inline comments as done.
ldionne added inline comments.
================
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);
----------------
erik.pilkington wrote:
> 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.
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.
================
Comment at: clang/test/SemaCXX/PR38913.cpp:19
+};
+C<int>::X<int, long>* c() { return 0; } // CHECK-DAG: @_Z1cB4CTAGv
+
----------------
erik.pilkington wrote:
> 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.
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.
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
