[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]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8a37a6ce674: [Clang] Fix compat diagnostic to detect a 
nontype template parameter has a… (authored by shafik).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132990

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/gh57362.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -146,6 +146,11 @@
 - A SubstTemplateTypeParmType can now represent the pack index for a
   substitution from an expanded pack.
   `Issue 56099 `_
+- Fix `-Wpre-c++17-compat` crashing Clang when compiling C++20 code which
+  contains deduced template specializations. This Fixes
+  `Issue 57369 `_
+  `Issue 57643 `_
+  `Issue 57793 `_
 
 
 Improvements to Clang's diagnostics


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  

[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 updated this revision to Diff 461088.
shafik marked an inline comment as done.
shafik added a comment.

- Update wording in release note.


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

https://reviews.llvm.org/D132990

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/gh57362.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -146,6 +146,11 @@
 - A SubstTemplateTypeParmType can now represent the pack index for a
   substitution from an expanded pack.
   `Issue 56099 `_
+- Fix `-Wpre-c++17-compat` crashing Clang when compiling C++20 code which
+  contains deduced template specializations. This Fixes
+  `Issue 57369 `_
+  `Issue 57643 `_
+  `Issue 57793 `_
 
 
 Improvements to Clang's diagnostics


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
Index: clang/docs/ReleaseNotes.rst
===
--- 

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:149-151
+- Fix C++17 compatibibility warning that checks for use of auto in template
+  parameters so that it atually has an AutoType before attempting to call
+  getContainedAutoType(). This Fixes

'compatibility' mispelled.

Though since this is a user facing document, I would try to give a less 
mechanical / less local explanation of the problem.




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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 461033.
shafik added a comment.

- Add release notes.


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

https://reviews.llvm.org/D132990

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/gh57362.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -146,6 +146,12 @@
 - A SubstTemplateTypeParmType can now represent the pack index for a
   substitution from an expanded pack.
   `Issue 56099 `_
+- Fix C++17 compatibibility warning that checks for use of auto in template
+  parameters so that it atually has an AutoType before attempting to call
+  getContainedAutoType(). This Fixes
+  `Issue 57369 `_
+  `Issue 57643 `_
+  `Issue 57793 `_
 
 
 Improvements to Clang's diagnostics


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
Index: clang/docs/ReleaseNotes.rst
===
--- 

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Oh by the way, it's missing the Release notes, but otherwise LGTM.


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 a comment.

ping,


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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(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 
+class TemplateClass {};
+
+template  // 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  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 460229.
shafik marked 6 inline comments as done.
shafik added a comment.

- Updating testing to reflect comments.


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

https://reviews.llvm.org/D132990

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/gh57362.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx17.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
Index: clang/test/SemaTemplate/gh57362.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/gh57362.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+// expected-no-diagnostics
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:310-316
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}

I think the issue might not be tested in this file since we do not run it with 
the `-Wpre-c++17-compat` flag


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1534-1538
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);

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`.



Comment at: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp:3-8
+namespace GH57362 {
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};

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.


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you also add a release note for the changes?




Comment at: clang/lib/Sema/SemaTemplate.cpp:1534-1538
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);

Let's get fancy!



Comment at: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+

Please rename the file to end in cxx17 instead of cxx1z :-)


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 marked an inline comment as done.
shafik added inline comments.



Comment at: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp:7
+
+template  // ok, no diagnostic expected
+void func() {}

mizvekov wrote:
> I think this should have a new diagnostic per above, as this is not 
> compatible with C++17.
I split out the tests to C++20 and C++17 parts.


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 457471.
shafik added a comment.

- Updated to check contained deduced type before checking if it is an `AutoType`
- Split out test into C++20 and C++17 parts


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

https://reviews.llvm.org/D132990

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+namespace GH57362 {
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+}
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -306,3 +306,11 @@
 f(A()); // OK
   }
 }
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wpre-c++17-compat %s
+
+namespace GH57362 {
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+}
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -306,3 +306,11 @@
 f(A()); // OK
   }
 }
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,11 +1531,11 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
-Diag(D.getIdentifierLoc(),
- diag::warn_cxx14_compat_template_nontype_parm_auto_type)
-  << QualType(TInfo->getType()->getContainedAutoType(), 0);
-  }
+  if (const auto *T = TInfo->getType()->getContainedDeducedType())
+if (isa(T))
+  Diag(D.getIdentifierLoc(),
+   diag::warn_cxx14_compat_template_nontype_parm_auto_type)
+  << QualType(TInfo->getType()->getContainedAutoType(), 0);
 
   assert(S->isTemplateParamScope() &&
  "Non-type template parameter not in template parameter scope!");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1534-1538
+  if (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);
   }

shafik wrote:
> shafik wrote:
> > mizvekov wrote:
> > > I think checking that the deduced type is undeduced was really 
> > > unnecessary though, as I don't think we update the type source infos 
> > > after deduction in any case.
> > I am not sure about the `DeducedTemplateSpecializationType` I was looking 
> > at the change that brought in that type and it is not clear to me what a 
> > good test would be. I believe outside of C++17 mode they would be hard 
> > errors.  Can you provide an example that would work outside of C++17 mode?
> The `DeducedTemplateSpecializationType` also feels a bit outside the bug I am 
> fixing so I might just choose to address it in a follow-up.
Well the bit on your test below, which you marked with no diagnostic expected, 
is an example.

This is something that works on C++20 but not on C++17, which is what this 
warning is supposed to warn, right?


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);
   }

shafik wrote:
> mizvekov wrote:
> > I think checking that the deduced type is undeduced was really unnecessary 
> > though, as I don't think we update the type source infos after deduction in 
> > any case.
> I am not sure about the `DeducedTemplateSpecializationType` I was looking at 
> the change that brought in that type and it is not clear to me what a good 
> test would be. I believe outside of C++17 mode they would be hard errors.  
> Can you provide an example that would work outside of C++17 mode?
The `DeducedTemplateSpecializationType` also feels a bit outside the bug I am 
fixing so I might just choose to address it in a follow-up.


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);
   }

mizvekov wrote:
> I think checking that the deduced type is undeduced was really unnecessary 
> though, as I don't think we update the type source infos after deduction in 
> any case.
I am not sure about the `DeducedTemplateSpecializationType` I was looking at 
the change that brought in that type and it is not clear to me what a good test 
would be. I believe outside of C++17 mode they would be hard errors.  Can you 
provide an example that would work outside of C++17 mode?


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+

One thing we could do is to run this test also in std=c++17 mode, with a 
different expectancy, as a kind of sanity check that the warning is in sync.


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a reviewer: mizvekov.
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1534-1538
+  if (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);
   }

I think checking that the deduced type is undeduced was really unnecessary 
though, as I don't think we update the type source infos after deduction in any 
case.



Comment at: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp:7
+
+template  // ok, no diagnostic expected
+void func() {}

I think this should have a new diagnostic per above, as this is not compatible 
with C++17.


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

https://reviews.llvm.org/D132990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 created this revision.
shafik added reviewers: aaron.ballman, erichkeane, rsmith.
Herald added a project: All.
shafik requested review of this revision.

Based on the changes introduced by 15361a21e01026e74cb17011b702c7d1c881ae94 it 
looks like C++17 compatibility diagnostic should have been checking 
`getContainedAutoType()`.

This fixes: https://github.com/llvm/llvm-project/issues/57369


https://reviews.llvm.org/D132990

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+
+template  // expected-warning {{non-type template parameters declared 
with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters 
declared with 'decltype(auto)' are incompatible with C++ standards before 
C++17}}
+struct B{};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,7 +1531,7 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
+  if (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);


Index: clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_nontype_diagnostic_cxx1z.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -Wpre-c++17-compat %s
+
+namespace GH57362 {
+template 
+class TemplateClass {};
+
+template  // ok, no diagnostic expected
+void func() {}
+
+template  // expected-warning {{non-type template parameters declared with 'auto' are incompatible with C++ standards before C++17}}
+struct A{};
+
+template  // expected-warning {{non-type template parameters declared with 'decltype(auto)' are incompatible with C++ standards before C++17}}
+struct B{};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1531,7 +1531,7 @@
 
   CheckValidDeclSpecifiers();
 
-  if (TInfo->getType()->isUndeducedType()) {
+  if (TInfo->getType()->getContainedAutoType()) {
 Diag(D.getIdentifierLoc(),
  diag::warn_cxx14_compat_template_nontype_parm_auto_type)
   << QualType(TInfo->getType()->getContainedAutoType(), 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits