[Concepts] Avoid substituting into constraints for invalid TemplateDecls

[zyn0217]

Fixes #73885.

Substituting into constraints for invalid TemplateDecls might still yield dependent expressions and end up crashing later while attempting evaluation.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

Fixes #73885.

Substituting into constraints for invalid TemplateDecls might still yield dependent expressions and end up crashing later while attempting evaluation.

---

Full diff: https://github.com/llvm/llvm-project/pull/75697.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/Sema/SemaConcept.cpp (+4)
• (modified) clang/test/SemaTemplate/instantiate-requires-expr.cpp (+10)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b4b5352a306c1c..83f98b97925250 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 719c6aab74e017..73683fee7c1487 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 ba82fc1313fc95..fb4127182d1cb6 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@-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@-2 {{template parameter is declared here}}
+requires(T{})
+T e_v;
+double e = e_v<double>;
+// expected-error@-1 {{constraints not satisfied for variable template 'e_v' [with T = double]}}
+}

[cor3ntin]

@zyn0217 This fell under the cracks, I'm sorry.
I think this is good enough as is

[zyn0217]

Hmmm, actually I'm thinking about whether we can move the check to somewhere in Parser so that these don't end up in the evaluation function.

[zyn0217]

@cor3ntin I moved the check to CheckConstraintSatisfaction so we don't end up with many substitution failure notes. Can you check it out again?

[cor3ntin]

LGTM modulo comments

[cor3ntin]

Really?
This is... wild. Is that intentional?

[zyn0217]

This is brought by 6591149, which I think is inappropriate nowadays but we probably need to keep it for ... historical reasons?

[cor3ntin]

I think we should remove test that is unrelated to the issue.
Would you mind creating a new issue for this behavior? Clang is being peculiar https://compiler-explorer.com/z/TcPGjWMGM - I think we should reconsider

[zyn0217]

Yeah, I'd love to. Wait a minute.

[zyn0217]

#99296

[zyn0217]

(And thanks for the review.)