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

... and only look at equivalence of substituted expressions, not results
of constraint satisfaction.
This is required by the standard when matching redeclarations.

Fixes llvm#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. Introduce a new function
that does not evaluate constraint and use it when matching declarations.

Also change implementation of `SubstConstraintExpr` to call `SubstExpr`
directly so it's obvious they behave in the same way and add a FIXME to
call out that we might need to revamp this approach in the future.

Author: ilya-biryukov

clang/include/clang/Sema/Sema.h

@@ -10263,11 +10263,13 @@ class Sema final {
     ~ConstraintEvalRAII() { TI.setEvaluateConstraints(OldValue); }
   };
 
-  // Unlike the above, this evaluates constraints, which should only happen at
-  // 'constraint checking' time.
+  // Must be used instead of SubstExpr at 'constraint checking' time.
   ExprResult
   SubstConstraintExpr(Expr *E,
                       const MultiLevelTemplateArgumentList &TemplateArgs);
+  // Unlike the above, this does not evaluates constraints.
+  ExprResult SubstConstraintExprWithoutSatisfaction(
+      Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs);
 
   /// Substitute the given template arguments into a list of
   /// expressions, expanding pack expansions if required.

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

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,
@@ -797,8 +796,8 @@ SubstituteConstraintExpression(Sema &S,
   std::optional<Sema::CXXThisScopeRAII> ThisScope;
   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);
+  ExprResult SubstConstr = S.SubstConstraintExprWithoutSatisfaction(
+      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;

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -1190,6 +1190,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:
@@ -2499,6 +2500,17 @@ 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;
+    if (TransConstraint.get()->isInstantiationDependent())
+      return new (SemaRef.Context)
+          concepts::NestedRequirement(TransConstraint.get());
+    ConstraintSatisfaction Satisfaction;
+    return new (SemaRef.Context) concepts::NestedRequirement(
+        SemaRef.Context, TransConstraint.get(), Satisfaction);
+  }
 
   ExprResult TransConstraint;
   ConstraintSatisfaction Satisfaction;
@@ -4093,13 +4105,19 @@ Sema::SubstExpr(Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
 ExprResult
 Sema::SubstConstraintExpr(Expr *E,
                           const MultiLevelTemplateArgumentList &TemplateArgs) {
+  // FIXME: should call SubstExpr directly if this function is equivalent or
+  //        should it be different?
+  return SubstExpr(E, TemplateArgs);
+}
+
+ExprResult Sema::SubstConstraintExprWithoutSatisfaction(
+    Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
   if (!E)
     return E;
 
-  // This is where we need to make sure we 'know' constraint checking needs to
-  // happen.
   TemplateInstantiator Instantiator(*this, TemplateArgs, SourceLocation(),
                                     DeclarationName());
+  Instantiator.setEvaluateConstraints(false);
   return Instantiator.TransformExpr(E);
 }
 

clang/test/SemaTemplate/concepts-out-of-line-def.cpp

@@ -504,3 +504,35 @@ 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>; };
+      void bar(decltype(requires { requires is_not_same_v<T, int>; }));
+};
+
+template <class T>
+void X<T>::foo() requires requires { requires something_interesting<T>; } {}
+// expected-error@-1{{definition of 'foo' does not match any declaration}}
+// expected-note@*{{}}
+
+template <class T>
+void X<T>::foo() requires requires { requires is_not_same_v<T, int>; } {} // ok
+
+template <class T>
+void X<T>::bar(decltype(requires { requires something_interesting<T>; })) {}
+// expected-error@-1{{definition of 'bar' does not match any declaration}}
+
+template <class T>
+void X<T>::bar(decltype(requires { requires is_not_same_v<T, int>; })) {}
+} // namespace GH74314