[clang] [Concepts] Avoid substituting into constraints for invalid TemplateDecls (PR #75697)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 16 23:41:51 PDT 2024


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/75697

>From a6e01586f5d406b51492d973cb693ba32cc63d99 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 16 Dec 2023 18:49:59 +0800
Subject: [PATCH 1/3] [Concepts] Avoid substituting into constraints for
 invalid TemplateDecls

Fixes https://github.com/llvm/llvm-project/issues/73885.

Substituting into constraints for invalid TemplateDecls might still
yield dependent expressions and end up crashing later while
attempting evaluation.
---
 clang/docs/ReleaseNotes.rst                           |  3 +++
 clang/lib/Sema/SemaConcept.cpp                        |  4 ++++
 clang/test/SemaTemplate/instantiate-requires-expr.cpp | 10 ++++++++++
 3 files changed, 17 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b4b5352a306c1..83f98b9792525 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -831,6 +831,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 <https://github.com/llvm/llvm-project/issues/73112>`_)
 
+- Fixed a crash when substituting into constraint expressions for invalid variable templates.
+  Fixes: (`#73885 <https://github.com/llvm/llvm-project/issues/73885>`_)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 719c6aab74e01..73683fee7c148 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -353,6 +353,10 @@ static ExprResult calculateConstraintSatisfaction(
           if (Inst.isInvalid())
             return ExprError();
 
+          // An empty expression for substitution failure messages.
+          if (Template && Template->isInvalidDecl())
+            return ExprEmpty();
+
           llvm::FoldingSetNodeID ID;
           if (Template &&
               DiagRecursiveConstraintEval(S, ID, Template, AtomicExpr, MLTAL)) {
diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
index ba82fc1313fc9..fb4127182d1cb 100644
--- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -227,3 +227,13 @@ struct r6 {};
 
 using r6i = r6<int>;
 // expected-error at -1 {{constraints not satisfied for class template 'r6' [with T = int]}}
+
+namespace GH73885 {
+template <typename T> // expected-error {{extraneous template parameter list}}
+template <typename T> // expected-error {{'T' shadows template parameter}}
+                      // expected-note at -2 {{template parameter is declared here}}
+requires(T{})
+T e_v;
+double e = e_v<double>;
+// expected-error at -1 {{constraints not satisfied for variable template 'e_v' [with T = double]}}
+}

>From 4085b4fa5868f62f970d2f3e2b15b663bf4f7a3a Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 12 Jul 2024 22:33:05 +0800
Subject: [PATCH 2/3] Revert "[Concepts] Avoid substituting into constraints
 for invalid TemplateDecls"

This reverts commit a6e01586f5d406b51492d973cb693ba32cc63d99.
---
 clang/docs/ReleaseNotes.rst                           |  3 ---
 clang/lib/Sema/SemaConcept.cpp                        |  4 ----
 clang/test/SemaTemplate/instantiate-requires-expr.cpp | 10 ----------
 3 files changed, 17 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fa79d27cc84d4..838cb69f647ee 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -976,9 +976,6 @@ Bug Fixes to C++ Support
 - Fixed a bug where references to lambda capture inside a ``noexcept`` specifier were not correctly
   instantiated. (#GH95735).
 
-- Fixed a crash when substituting into constraint expressions for invalid variable templates.
-  Fixes: (`#73885 <https://github.com/llvm/llvm-project/issues/73885>`_)
-
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628)
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 52628bdcc304b..202dd86c67f62 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -353,10 +353,6 @@ static ExprResult calculateConstraintSatisfaction(
           if (Inst.isInvalid())
             return ExprError();
 
-          // An empty expression for substitution failure messages.
-          if (Template && Template->isInvalidDecl())
-            return ExprEmpty();
-
           llvm::FoldingSetNodeID ID;
           if (Template &&
               DiagRecursiveConstraintEval(S, ID, Template, AtomicExpr, MLTAL)) {
diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
index 90f16ce4c6f33..516708bf4c875 100644
--- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -227,13 +227,3 @@ struct r6 {};
 
 using r6i = r6<int>;
 // expected-error at -1 {{constraints not satisfied for class template 'r6' [with T = int]}}
-
-namespace GH73885 {
-template <typename T> // expected-error {{extraneous template parameter list}}
-template <typename T> // expected-error {{'T' shadows template parameter}}
-                      // expected-note at -2 {{template parameter is declared here}}
-requires(T{})
-T e_v;
-double e = e_v<double>;
-// expected-error at -1 {{constraints not satisfied for variable template 'e_v' [with T = double]}}
-}

>From 1680fa415f8b871b71bf7b76e78177ea5bf54b8d Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 17 Jul 2024 14:41:15 +0800
Subject: [PATCH 3/3] Move the check to CheckConstraintSatisfaction()

---
 clang/docs/ReleaseNotes.rst                     |  1 +
 clang/lib/Sema/SemaConcept.cpp                  |  7 +++++++
 .../SemaTemplate/instantiate-requires-expr.cpp  | 17 +++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 615e8879ac474..0c9d4de3d709a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1039,6 +1039,7 @@ Bug Fixes to C++ Support
 - Fixed failed assertion when resolving context of defaulted comparison method outside of struct. (#GH96043).
 - Clang now diagnoses explicit object parameters in member pointers and other contexts where they should not appear.
   Fixes (#GH85992).
+- Fixed a crash-on-invalid bug involving extraneous template parameter with concept substitution. (#GH73885)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 54891150da20f..e58232856bd2d 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -483,6 +483,13 @@ bool Sema::CheckConstraintSatisfaction(
         *this, nullptr, ConstraintExprs, ConvertedConstraints,
         TemplateArgsLists, TemplateIDRange, OutSatisfaction);
   }
+  // Invalid templates could make their way here. Substituting them could result
+  // in dependent expressions.
+  // FIXME: Say something in the diagnostic note?
+  if (Template->isInvalidDecl()) {
+    OutSatisfaction.IsSatisfied = false;
+    return true;
+  }
 
   // A list of the template argument list flattened in a predictible manner for
   // the purposes of caching. The ConstraintSatisfaction type is in AST so it
diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
index 516708bf4c875..934f22ce65887 100644
--- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -227,3 +227,20 @@ struct r6 {};
 
 using r6i = r6<int>;
 // expected-error at -1 {{constraints not satisfied for class template 'r6' [with T = int]}}
+
+namespace GH73885 {
+
+template <class> // expected-error {{extraneous}}
+template <class T> requires(T{})
+bool e_v = true;
+
+static_assert(e_v<bool>);
+
+// We do accept the following as an extension. See Sema::MatchTemplateParametersToScopeSpecifier()
+template <>  // expected-warning {{extraneous}}
+template <class T> requires(T{})
+bool e = true;
+
+static_assert(e_v<bool>);
+
+} // namespace GH73885



More information about the cfe-commits mailing list