rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:775-776
+static bool IsInsideImplicitFullTemplateSpecialization(const DeclContext *DC) {
+ auto *CTSDecl = dyn_cast_or_null<ClassTemplateSpecializationDecl>(
+ DC->getOuterLexicalRecordContext());
+ return CTSDecl && !isa<ClassTemplatePartialSpecializationDecl>(CTSDecl) &&
----------------
This doesn't look right to me; there could be a class template nested inside a
non-templated class, so I think you would need to walk up the enclosing
`DeclContext`s one by one checking each in turn. But, we might be inside a
function template specialization or variable template specialization instead,
in some weird cases:
```
template<typename T> concept C = true;
template<typename T> auto L = []<C<T> U>() {};
struct Q {
template<C<int> U> friend constexpr auto decltype(L<int>)::operator()() const;
};
```
... so I think we want a different approach than looking for an enclosing class
template specialization declaration.
Can we skip this check entirely, and instead always compute and substitute the
template instantiation arguments as is done below? That computation will walk
the enclosing contexts for us in a careful way that properly handles cases like
this lambda-in-variable-template situation. If we find we get zero levels of
template argument list, we can skip doing the actual substitution as an
optimization.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:1303
+ OldTemplate->getTemplateParameters(), false, TPL_TemplateMatch,
+ SourceLocation(), false /* PartialOrdering */);
bool SameReturnType = Context.hasSameType(Old->getDeclaredReturnType(),
----------------
rsmith wrote:
> shafik wrote:
> > nit
> Just remove the final parameter; it has a default argument of `false` and no
> other call site passes `false` here. (I'm working on removing this parameter
> in a different change.)
You can remove the `SourceLocation()` argument too; there's an identical
default argument.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146178/new/
https://reviews.llvm.org/D146178
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
