erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2854-2858
-template <>
-bool DeducedArgsNeedReplacement<ClassTemplatePartialSpecializationDecl>(
- ClassTemplatePartialSpecializationDecl *Spec) {
- return !Spec->isClassScopeExplicitSpecialization();
-}
----------------
rsmith wrote:
> It's not obvious to me what this was doing, so I'm not sure whether it's
> correct to remove it. Can you explain? It makes me uncomfortable that we
> would treat class template partial specializations and variable template
> partial specializations differently, at least without a good explanation for
> why that's appropriate -- perhaps we should apply this same set of changes to
> variable template partial specializations too, and remove this mechanism
> entirely?
Absolutely! And I suspect at a certain point we are going to want to remove
this entirely, it is a bit of a hack (and I couldn't come up with a good repro
that this caused a problem, and the `VarTemplateSpecializationDecl` code does a
lot of work).
Basically, when doing the `CheckDeducedArgumentConstraints` below, we get the
instantiation args list (see this being used on 2869).
However, the 'inner most' args when this is a Partial Specialization end up
being 'wrong', since they come from the partial specialization. This is used
to determine whether we need to replace them with the args we already have.
However, this patch now makes it so `ClassTemplatePartialSpecializationDecl`s
no longer generate template arguments in `getTemplateInstantiationArgs` (which
has seen a bit of a refactor since you were here last, since it was a bit of a
difficult to understand function that didn't seem to cover many cases well, and
had a lot of repetation).
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:322
+ // they mean.
+ R = Response::UseNextDecl(PartialClassTemplSpec);
} else if (const auto *ClassTemplSpec =
----------------
rsmith wrote:
> Can we produce a "done" response instead here? It doesn't seem right to carry
> on to the next level here without adding a level of template arguments -- if
> any further arguments were somehow collected, they would be collected at the
> wrong template depth. But, as far as I can determine, we should never
> encounter a partial specialization declaration except in cases where there is
> nothing remaining to be substituted (all outer substitutions would be
> identity substitutions). I think the same thing applies when we reach a class
> template declaration (the first branch in `HandleRecordDecl`) -- we should
> just be able to stop in that case too, rather than collecting some identity
> template arguments and carrying on to an outer level.
>
> If we can't produce a "done" response here for some reason, then we should
> collect a level of identity template arguments instead.
For constraints, the arguments are relative to namespace scope as far as I can
tell. If we ended up 'stopping' we would miss references to the outer
template, so something like:
```
template<typename T>
struct Outer {
template<typename U>
struct Inner {
void foo() requires (is_same<T, U>){}
};
};
```
Right? The substitution fo 'T' would be invalid at that point?
Also, what do you mean by 'identity' template arguments? I'm not sure I follow.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147722/new/
https://reviews.llvm.org/D147722
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
