[PATCH] D132990: [Clang] Fix compat diagnostic to detect a nontype template parameter has a placeholder type using getContainedAutoType()

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1534-1538
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+    if (isa<AutoType>(T))
+      Diag(D.getIdentifierLoc(),
+           diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+          << QualType(TInfo->getType()->getContainedAutoType(), 0);
----------------
mizvekov wrote:
> aaron.ballman wrote:
> > Let's get fancy!
> You would use `getContainedDeducedType` if you expected to handle DeducedTypes in general, not just AutoTypes.
> 
> So if you only want to handle AutoTypes, there is no point in using `getContainedDeducedType`.
I am going to keep the `getContainedDeducedType(...)` b/c I do plan on coming back to this and adding the other diagnostic but I won't get fancy at this point but will happily consider it on the follow-up.


================
Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:310-316
+namespace GH57362 {
+template <int num>
+class TemplateClass {};
+
+template <TemplateClass nttp> // ok, no diagnostic expected
+void func() {}
+}
----------------
mizvekov wrote:
> I think the issue might not be tested in this file since we do not run it with the `-Wpre-c++17-compat` flag
Good catch, I will just create a standalone `gh57362.cpp` file since this does not fit neatly anywhere else. 


================
Comment at: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp:3-8
+namespace GH57362 {
+template <auto n> // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template <decltype(auto) n> // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
----------------
mizvekov wrote:
> I don't understand why these cases are grouped under GH57362 issue, they are cases that worked fine without this patch, we just didn't have tests for them.
Good point. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132990/new/

https://reviews.llvm.org/D132990