Implement the termination algorithm for impl selection described in #2458 (#2602)

Detect when evaluating an impl recursively tries to evaluate the same impl for the same or a more complex set of parameters.

In order to perform the check after we have tested that the type structure matches and before we check that constraints are recursively satisfied, argument deduction is extended to check structural matching properties earlier.

This requires us to separate match failures into two kinds: hard failures that produce errors that should never be swallowed, and soft failures such as a missing impl that lead us to merely discard an impl as a candidate. A flag has been added to `ImplScope` and `ArgumentDeduction` to specify whether soft failures should produce an error message or not.

explorer/interpreter/impl_scope.cpp

@@ -65,18 +65,17 @@ void ImplScope::AddParent(Nonnull<const ImplScope*> parent) {
   parent_scopes_.push_back(parent);
 }
 
-// Checks that `a_evaluated == b_evaluated` for the purpose of an equality
-// constraint. Produces an error if not.
-static auto CheckEqualOrDiagnose(SourceLocation source_loc,
-                                 Nonnull<const Value*> a_written,
-                                 Nonnull<const Value*> a_evaluated,
-                                 Nonnull<const Value*> b_written,
-                                 Nonnull<const Value*> b_evaluated,
-                                 Nonnull<const EqualityContext*> equality_ctx)
-    -> ErrorOr<Success> {
-  if (ValueEqual(a_evaluated, b_evaluated, equality_ctx)) {
-    return Success();
-  }
+// Diagnose that `a_evaluated != b_evaluated` for the purpose of an equality
+// constraint.
+static auto DiagnoseUnequalValues(SourceLocation source_loc,
+                                  Nonnull<const Value*> a_written,
+                                  Nonnull<const Value*> a_evaluated,
+                                  Nonnull<const Value*> b_written,
+                                  Nonnull<const Value*> b_evaluated,
+                                  Nonnull<const EqualityContext*> equality_ctx)
+    -> Error {
+  CARBON_CHECK(!ValueEqual(a_evaluated, b_evaluated, equality_ctx))
+      << "expected unequal values";
   auto error = ProgramError(source_loc);
   error << "constraint requires that " << *a_written;
   if (!ValueEqual(a_written, a_evaluated, std::nullopt)) {
@@ -96,10 +95,26 @@ auto ImplScope::Resolve(Nonnull<const Value*> constraint_type,
                         const TypeChecker& type_checker,
                         const Bindings& bindings) const
     -> ErrorOr<Nonnull<const Witness*>> {
+  CARBON_ASSIGN_OR_RETURN(
+      std::optional<Nonnull<const Witness*>> witness,
+      TryResolve(constraint_type, impl_type, source_loc, type_checker, bindings,
+                 /*diagnose_missing_impl=*/true));
+  CARBON_CHECK(witness) << "should have diagnosed missing impl";
+  return *witness;
+}
+
+auto ImplScope::TryResolve(Nonnull<const Value*> constraint_type,
+                           Nonnull<const Value*> impl_type,
+                           SourceLocation source_loc,
+                           const TypeChecker& type_checker,
+                           const Bindings& bindings,
+                           bool diagnose_missing_impl) const
+    -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
   if (const auto* iface_type = dyn_cast<InterfaceType>(constraint_type)) {
     iface_type =
         cast<InterfaceType>(type_checker.Substitute(bindings, iface_type));
-    return ResolveInterface(iface_type, impl_type, source_loc, type_checker);
+    return TryResolveInterface(iface_type, impl_type, source_loc, type_checker,
+                               diagnose_missing_impl);
   }
   if (const auto* constraint = dyn_cast<ConstraintType>(constraint_type)) {
     std::vector<Nonnull<const Witness*>> witnesses;
@@ -120,12 +135,16 @@ auto ImplScope::Resolve(Nonnull<const Value*> constraint_type,
       Bindings local_bindings = bindings;
       local_bindings.Add(constraint->self_binding(), impl_type, witness);
       CARBON_ASSIGN_OR_RETURN(
-          Nonnull<const Witness*> result,
-          ResolveInterface(cast<InterfaceType>(type_checker.Substitute(
-                               local_bindings, impl.interface)),
-                           type_checker.Substitute(local_bindings, impl.type),
-                           source_loc, type_checker));
-      witnesses.push_back(result);
+          std::optional<Nonnull<const Witness*>> result,
+          TryResolveInterface(
+              cast<InterfaceType>(
+                  type_checker.Substitute(local_bindings, impl.interface)),
+              type_checker.Substitute(local_bindings, impl.type), source_loc,
+              type_checker, diagnose_missing_impl));
+      if (!result) {
+        return {std::nullopt};
+      }
+      witnesses.push_back(*result);
     }
 
     // Check that all intrinsic, equality, and rewrite constraints
@@ -155,6 +174,9 @@ auto ImplScope::Resolve(Nonnull<const Value*> constraint_type,
               type_checker.Substitute(local_bindings, argument));
         }
         if (!type_checker.IsIntrinsicConstraintSatisfied(converted, *this)) {
+          if (!diagnose_missing_impl) {
+            return {std::nullopt};
+          }
           return ProgramError(source_loc)
                  << "constraint requires that " << converted;
         }
@@ -166,9 +188,13 @@ auto ImplScope::Resolve(Nonnull<const Value*> constraint_type,
         for (; it != equal.values.end(); ++it) {
           Nonnull<const Value*> current =
               type_checker.Substitute(local_bindings, *it);
-          CARBON_RETURN_IF_ERROR(
-              CheckEqualOrDiagnose(source_loc, equal.values.front(), first, *it,
-                                   current, &equality_ctx));
+          if (!ValueEqual(first, current, &equality_ctx)) {
+            if (!diagnose_missing_impl) {
+              return {std::nullopt};
+            }
+            return DiagnoseUnequalValues(source_loc, equal.values.front(),
+                                         first, *it, current, &equality_ctx);
+          }
         }
       }
       for (const auto& rewrite : rewrites) {
@@ -176,12 +202,17 @@ auto ImplScope::Resolve(Nonnull<const Value*> constraint_type,
             type_checker.Substitute(local_bindings, rewrite.constant);
         Nonnull<const Value*> value = type_checker.Substitute(
             local_bindings, rewrite.converted_replacement);
-        CARBON_RETURN_IF_ERROR(CheckEqualOrDiagnose(
-            source_loc, rewrite.constant, constant,
-            rewrite.converted_replacement, value, &equality_ctx));
+        if (!ValueEqual(constant, value, &equality_ctx)) {
+          if (!diagnose_missing_impl) {
+            return {std::nullopt};
+          }
+          return DiagnoseUnequalValues(source_loc, rewrite.constant, constant,
+                                       rewrite.converted_replacement, value,
+                                       &equality_ctx);
+        }
       }
     }
-    return type_checker.MakeConstraintWitness(std::move(witnesses));
+    return {type_checker.MakeConstraintWitness(std::move(witnesses))};
   }
   CARBON_FATAL() << "expected a constraint, not " << *constraint_type;
 }
@@ -202,19 +233,21 @@ auto ImplScope::VisitEqualValues(
   return true;
 }
 
-auto ImplScope::ResolveInterface(Nonnull<const InterfaceType*> iface_type,
-                                 Nonnull<const Value*> type,
-                                 SourceLocation source_loc,
-                                 const TypeChecker& type_checker) const
-    -> ErrorOr<Nonnull<const Witness*>> {
+auto ImplScope::TryResolveInterface(Nonnull<const InterfaceType*> iface_type,
+                                    Nonnull<const Value*> type,
+                                    SourceLocation source_loc,
+                                    const TypeChecker& type_checker,
+                                    bool diagnose_missing_impl) const
+    -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
   CARBON_ASSIGN_OR_RETURN(
       std::optional<Nonnull<const Witness*>> result,
-      TryResolve(iface_type, type, source_loc, *this, type_checker));
-  if (!result.has_value()) {
+      TryResolveInterfaceRecursively(iface_type, type, source_loc, *this,
+                                     type_checker));
+  if (!result.has_value() && diagnose_missing_impl) {
     return ProgramError(source_loc) << "could not find implementation of "
                                     << *iface_type << " for " << *type;
   }
-  return *result;
+  return result;
 }
 
 // Combines the results of two impl lookups. In the event of a tie, arbitrarily
@@ -273,36 +306,36 @@ static auto CombineResults(Nonnull<const InterfaceType*> iface_type,
          << "ambiguous implementations of " << *iface_type << " for " << *type;
 }
 
-auto ImplScope::TryResolve(Nonnull<const InterfaceType*> iface_type,
-                           Nonnull<const Value*> type,
-                           SourceLocation source_loc,
-                           const ImplScope& original_scope,
-                           const TypeChecker& type_checker) const
+auto ImplScope::TryResolveInterfaceRecursively(
+    Nonnull<const InterfaceType*> iface_type, Nonnull<const Value*> type,
+    SourceLocation source_loc, const ImplScope& original_scope,
+    const TypeChecker& type_checker) const
     -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
   CARBON_ASSIGN_OR_RETURN(
       std::optional<Nonnull<const Witness*>> result,
-      ResolveHere(iface_type, type, source_loc, original_scope, type_checker));
+      TryResolveInterfaceHere(iface_type, type, source_loc, original_scope,
+                              type_checker));
   for (Nonnull<const ImplScope*> parent : parent_scopes_) {
     CARBON_ASSIGN_OR_RETURN(
         std::optional<Nonnull<const Witness*>> parent_result,
-        parent->TryResolve(iface_type, type, source_loc, original_scope,
-                           type_checker));
+        parent->TryResolveInterfaceRecursively(iface_type, type, source_loc,
+                                               original_scope, type_checker));
     CARBON_ASSIGN_OR_RETURN(result, CombineResults(iface_type, type, source_loc,
                                                    result, parent_result));
   }
   return result;
 }
 
-auto ImplScope::ResolveHere(Nonnull<const InterfaceType*> iface_type,
-                            Nonnull<const Value*> impl_type,
-                            SourceLocation source_loc,
-                            const ImplScope& original_scope,
-                            const TypeChecker& type_checker) const
+auto ImplScope::TryResolveInterfaceHere(
+    Nonnull<const InterfaceType*> iface_type, Nonnull<const Value*> impl_type,
+    SourceLocation source_loc, const ImplScope& original_scope,
+    const TypeChecker& type_checker) const
     -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
   std::optional<Nonnull<const Witness*>> result = std::nullopt;
   for (const Impl& impl : impls_) {
-    std::optional<Nonnull<const Witness*>> m = type_checker.MatchImpl(
-        *iface_type, impl_type, impl, original_scope, source_loc);
+    CARBON_ASSIGN_OR_RETURN(std::optional<Nonnull<const Witness*>> m,
+                            type_checker.MatchImpl(*iface_type, impl_type, impl,
+                                                   original_scope, source_loc));
     CARBON_ASSIGN_OR_RETURN(
         result, CombineResults(iface_type, impl_type, source_loc, result, m));
   }