sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11014
// Check that the return type is written as a specialization of
// the template specified as the deduction-guide's name.
ParsedType TrailingReturnType = Chunk.Fun.getTrailingReturnType();
----------------
I'd add here "The template name may not be qualified. [temp.deduct.guide]".
This is pretty important to the logic here and never explicitly written down.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11027
+ auto TKind = SpecifiedName.getKind();
+ // Template names within TemplateSpecializationType are never qualified
+ // template names!
----------------
It's not quite clear what you're implying by this:
- we want to allow qualified names, but they're not QualifiedTemplateNames?
(this was my first reading)
- we want to disallow qualified names, but don't have to worry about
QualifiedTemplateNames.
The latter is correct, because the grammar only allows a simple-template-id
(the name must be an identifier, no NNS allowed).
I'm not actually sure we want a comment here about qualified names at all - not
including it in the || is correct whether or not it could occur here.
By the way, the following code is correctly rejected by clang, but accepted by
GCC and MSVC:
```
namespace ns {
template <typename> class X {};
template <typename T> X(T) -> ns::X<T>;
}
```
(We're in the right scope, so this isn't just about secondary diagnostics)
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11029
+ // template names!
+ if ((TKind == TemplateName::Template ||
+ TKind == TemplateName::UsingTemplate) &&
----------------
The meaning of this || is not clear.
Maybe `bool SimplyWritten = ...`?
Maybe also `// A Using TemplateName can't actually be valid (either it's
qualified, or we're in the wrong scope). But we have diagnosed these problems
already.`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123808/new/
https://reviews.llvm.org/D123808
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
