[Concepts] Fix an assert when trying to form a recovery expr on a

concept

When we failed the lookup of the function, we tried to form a
RecoveryExpr that caused us to recursively re-check the same constraint,
which caused us to try to double-insert the satisfaction into the cache.

This patch makes us just return the inner-cached version instead. We DO
end up double-evaluating thanks to the recovery-expr, but there isn't a
good way around that.

Author: Erich Keane

clang/docs/ReleaseNotes.rst

@@ -260,6 +260,10 @@ Bug Fixes
 - Fix template arguments of pointer and reference not taking the type as
   part of their identity.
   `Issue 47136 <https://github.com/llvm/llvm-project/issues/47136>`_
+- Fix a crash when trying to form a recovery expression on a call inside a
+  constraint, which re-evaluated the same constraint.
+  `Issue 53213 <https://github.com/llvm/llvm-project/issues/53213>`_
+  `Issue 45736 <https://github.com/llvm/llvm-project/issues/45736>`_
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaConcept.cpp

@@ -408,13 +408,29 @@ bool Sema::CheckConstraintSatisfaction(
     OutSatisfaction = *Cached;
     return false;
   }
+
   auto Satisfaction =
       std::make_unique<ConstraintSatisfaction>(Template, FlattenedArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
                                     ConvertedConstraints, TemplateArgsLists,
                                     TemplateIDRange, *Satisfaction)) {
     return true;
   }
+
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+    // The evaluation of this constraint resulted in us trying to re-evaluate it
+    // recursively. This isn't really possible, except we try to form a
+    // RecoveryExpr as a part of the evaluation.  If this is the case, just
+    // return the 'cached' version (which will have the same result), and save
+    // ourselves the extra-insert. If it ever becomes possible to legitimately
+    // recursively check a constraint, we should skip checking the 'inner' one
+    // above, and replace the cached version with this one, as it would be more
+    // specific.
+    OutSatisfaction = *Cached;
+    return false;
+  }
+
+  // Else we can simply add this satisfaction to the list.
   OutSatisfaction = *Satisfaction;
   // We cannot use InsertPos here because CheckConstraintSatisfaction might have
   // invalidated it.

clang/test/SemaTemplate/concepts-GH53213.cpp

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+namespace GH53213 {
+template<typename T>
+concept c = requires(T t) { f(t); }; // #CDEF
+
+auto f(c auto); // #FDEF
+
+void g() {
+  f(0);
+  // expected-error@-1{{no matching function for call to 'f'}}
+  // expected-note@#FDEF{{constraints not satisfied}}
+  // expected-note@#FDEF{{because 'int' does not satisfy 'c'}}
+  // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}}
+}
+} // namespace GH53213 
+
+namespace GH45736 {
+struct constrained;
+
+template<typename T>
+  struct type {
+  };
+template<typename T>
+  constexpr bool f(type<T>) {
+      return true;
+  }
+
+template<typename T>
+  concept matches = f(type<T>());
+
+
+struct constrained {
+    template<typename U> requires matches<U>
+        explicit constrained(U value) {
+            }
+};
+
+bool f(constrained const &) {
+    return true;
+}
+
+struct outer {
+    constrained state;
+};
+
+bool f(outer const & x) {
+    return f(x.state);
+}
+} // namespace GH45736