ilya-biryukov added a comment.
My only ask is to add a few more tests (see the relevant comment), otherwise LG.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:1293
+ // templates first; the remaining checks follow.
+ bool SameTemplateParameterList = TemplateParameterListsAreEqual(
+ NewTemplate->getTemplateParameters(),
----------------
Hopefully this does not regress performance too much now that we keep matching
template even if we don't need the result.
No need to take any action (the code looks simpler this way), just something to
keep in mind in case it backfires.
================
Comment at: clang/test/SemaTemplate/concepts-using-decl.cpp:3
+
+namespace heads_without_concepts {
+struct base {
----------------
NIT: maybe move this to the end of the file?
I would expect concept-specific tests to go first given the name of the file.
It's still useful to capture the discovered corner-cases, obviously, just not
as the first thing, maybe.
================
Comment at: clang/test/SemaTemplate/concepts-using-decl.cpp:112
+ expect<1>(baz{}.foo<Empty>()); // expected-error {{call to member function
'foo' is ambiguous}}
+}
+}
----------------
Could you also check that `requires` clauses and constraints in template
parameters do not hide each other?
```
template <IsEmpty T> ...
// vs
template <class T> requires IsEmpty<T> ...
// vs
template <class T> void foo() requires IsEmpty<T> ...
```
Should not hide each other.
Another interesting case (that probably does not yet work):
```
struct base { template <class T> void foo(); };
struct derived : base {
using base::foo;
template <IsEmpty T> void foo();
};
```
^^ `derived().foo<Empty>()` will probably produce an ambiguity now (as we don't
have an explicit requires clause here). I don't think it's worth fixing now,
but keeping a test for it with a FIXME seems reasonable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136440/new/
https://reviews.llvm.org/D136440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
