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);
----------------
rsmith wrote:
> ldionne wrote:
> > 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.
> 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`.
Thanks a lot for the explanation -- this is really helpful. It's now clear that
this line should not be there.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2971
InstD->setExternLoc(D->getExternLoc());
InstD->setTemplateKeywordLoc(D->getTemplateKeywordLoc());
----------------
ldionne wrote:
> 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.
>
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`).
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
