[clang] [Sema] When checking for constraint equivalence, do not calculate satisfaction (PR #74490)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Ilya Biryukov (ilya-biryukov)

<details>
<summary>Changes</summary>

... and only look at equivalence of substituted expressions, not results of constraint satisfaction.

Fixes #<!-- -->74314.

There is already some existing machinery for that in `TemplateInstantiator` and `Sema` exposed separate functions for substituting expressions with intention to do that:
- `Sema::SubstExpr` should not evaluate constraints.
- `Sema::SubstConstraintExpr` should.

However, both functions used to be equivalent. This commit changes the former to actually avoid calculating constraint satisfaction and uses it in the code path that matches definition and declaration.

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


4 Files Affected:

- (modified) clang/include/clang/Sema/Template.h (+1) 
- (modified) clang/lib/Sema/SemaConcept.cpp (+8-7) 
- (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+8) 
- (modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+21) 


``````````diff
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 2a553054a0ce5..ce44aca797b0f 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -564,6 +564,7 @@ enum class TemplateSubstitutionKind : char {
     const MultiLevelTemplateArgumentList &TemplateArgs;
     Sema::LateInstantiatedAttrVec* LateAttrs = nullptr;
     LocalInstantiationScope *StartingScope = nullptr;
+    // Whether to evaluate the C++20 constraints or simply substitute into them.
     bool EvaluateConstraints = true;
 
     /// A list of out-of-line class template partial
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 719c6aab74e01..be3541573377b 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -771,10 +771,9 @@ namespace {
   };
 } // namespace
 
-static const Expr *
-SubstituteConstraintExpression(Sema &S,
-                               const Sema::TemplateCompareNewDeclInfo &DeclInfo,
-                               const Expr *ConstrExpr) {
+static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
+    Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo,
+    const Expr *ConstrExpr) {
   MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
       DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
       /*Innermost=*/nullptr,
@@ -798,7 +797,7 @@ SubstituteConstraintExpression(Sema &S,
   if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
     ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
   ExprResult SubstConstr =
-      S.SubstConstraintExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
+      S.SubstExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
   if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
     return nullptr;
   return SubstConstr.get();
@@ -814,12 +813,14 @@ bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
   if (Old && !New.isInvalid() && !New.ContainsDecl(Old) &&
       Old->getLexicalDeclContext() != New.getLexicalDeclContext()) {
     if (const Expr *SubstConstr =
-            SubstituteConstraintExpression(*this, Old, OldConstr))
+            SubstituteConstraintExpressionWithoutSatisfaction(*this, Old,
+                                                              OldConstr))
       OldConstr = SubstConstr;
     else
       return false;
     if (const Expr *SubstConstr =
-            SubstituteConstraintExpression(*this, New, NewConstr))
+            SubstituteConstraintExpressionWithoutSatisfaction(*this, New,
+                                                              NewConstr))
       NewConstr = SubstConstr;
     else
       return false;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 6ad70109c8cee..971a4c99add0c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1186,6 +1186,7 @@ namespace {
     const MultiLevelTemplateArgumentList &TemplateArgs;
     SourceLocation Loc;
     DeclarationName Entity;
+    // Whether to evaluate the C++20 constraints or simply substitute into them.
     bool EvaluateConstraints = true;
 
   public:
@@ -2489,6 +2490,12 @@ TemplateInstantiator::TransformNestedRequirement(
       Req->getConstraintExpr()->getBeginLoc(), Req,
       Sema::InstantiatingTemplate::ConstraintsCheck{},
       Req->getConstraintExpr()->getSourceRange());
+  if (!getEvaluateConstraints()) {
+    ExprResult TransConstraint = TransformExpr(Req->getConstraintExpr());
+    if (TransConstraint.isInvalid() || !TransConstraint.get())
+      return nullptr;
+    return RebuildNestedRequirement(TransConstraint.get());
+  }
 
   ExprResult TransConstraint;
   ConstraintSatisfaction Satisfaction;
@@ -4077,6 +4084,7 @@ Sema::SubstExpr(Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
   TemplateInstantiator Instantiator(*this, TemplateArgs,
                                     SourceLocation(),
                                     DeclarationName());
+  Instantiator.setEvaluateConstraints(false);
   return Instantiator.TransformExpr(E);
 }
 
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index f134394615fb2..7e79eeb6d279a 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -504,3 +504,24 @@ struct bar {
 bar<int> x;
 } // namespace GH61763
 
+
+namespace GH74314 {
+template <class T, class U> constexpr bool is_same_v = __is_same(T, U);
+template <class T, class U> constexpr bool is_not_same_v = !__is_same(T, U);
+
+template <class Result>
+concept something_interesting = requires {
+      true;
+      requires is_same_v<int, Result>;
+};
+
+template <class T>
+struct X {
+      void foo() requires requires { requires is_not_same_v<T, int>; };
+};
+
+template <class T>
+void X<T>::foo() requires requires { requires something_interesting<T>; } {}
+// expected-error at -1{{definition of 'foo' does not match any declaration}}
+// expected-note@*{{}}
+} // namespace GH74314
\ No newline at end of file

``````````

</details>


https://github.com/llvm/llvm-project/pull/74490