Bring constraints into scope when a generic calls a template. (#2878)

When a template has an argument that involves a generic parameter, we're supposed to delay instantiation until we know the concrete value, but explorer is not set up to do that yet, so for now we instead instantiate the template with the symbolic argument. When that happens, bring the constraints on the generic parameter into scope so they can be used inside the template instantiation.

This requires adding a new search over a value for the generic parameters that appear within it; a `VisitNestedValues` visitor is added to visit all the `Value`s nested with a value, and also convert an existing place where we were doing the same thing in a way that was incorrect (but harmlessly incorrect for now) to use it.

This is needed by #2881, which needs implementations of `ImplicitAs` for nested types when instantiating a builtin impl of `ImplicitAs` for an aggregate type.

Co-authored-by: Geoff Romer <gromer@google.com>

explorer/ast/pattern.h

@@ -106,8 +106,8 @@ class Pattern : public AstNode {
 };
 
 // Call the given `visitor` on all patterns nested within the given pattern,
-// including `pattern` itself. Aborts and returns `false` if `visitor` returns
-// `false`, otherwise returns `true`.
+// including `pattern` itself, in a preorder traversal. Aborts and returns
+// `false` if `visitor` returns `false`, otherwise returns `true`.
 auto VisitNestedPatterns(const Pattern& pattern,
                          llvm::function_ref<bool(const Pattern&)> visitor)
     -> bool;

explorer/ast/value.cpp

@@ -13,6 +13,7 @@
 #include "explorer/ast/declaration.h"
 #include "explorer/ast/element.h"
 #include "explorer/ast/element_path.h"
+#include "explorer/ast/value_transform.h"
 #include "explorer/common/arena.h"
 #include "explorer/common/error_builders.h"
 #include "llvm/ADT/STLExtras.h"
@@ -27,6 +28,86 @@ using llvm::dyn_cast;
 using llvm::dyn_cast_or_null;
 using llvm::isa;
 
+namespace {
+// A visitor that walks the Value*s nested within a value.
+struct NestedValueVisitor {
+  template <typename T>
+  auto VisitParts(const T& decomposable) -> bool {
+    return decomposable.Decompose(
+        [&](const auto&... parts) { return (Visit(parts) && ...); });
+  }
+
+  auto Visit(Nonnull<const Value*> value) -> bool {
+    if (!callback(value)) {
+      return false;
+    }
+
+    return value->Visit<bool>(
+        [&](const auto* derived_value) { return VisitParts(*derived_value); });
+  }
+
+  auto Visit(Nonnull<const Bindings*> bindings) -> bool {
+    for (auto [binding, value] : bindings->args()) {
+      if (!Visit(value)) {
+        return false;
+      }
+    }
+    for (auto [binding, value] : bindings->witnesses()) {
+      if (!Visit(value)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  template <typename T>
+  auto Visit(const std::vector<T>& vec) -> bool {
+    for (auto& v : vec) {
+      if (!Visit(v)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  template <typename T>
+  auto Visit(const std::optional<T>& opt) -> bool {
+    return !opt || Visit(*opt);
+  }
+
+  template <typename T,
+            typename = std::enable_if_t<IsRecursivelyTransformable<T>>>
+  auto Visit(Nonnull<const T*> value) -> bool {
+    return VisitParts(*value);
+  }
+  template <typename T,
+            typename = std::enable_if_t<IsRecursivelyTransformable<T>>>
+  auto Visit(const T& value) -> bool {
+    return VisitParts(value);
+  }
+
+  // Other value components can't refer to a value.
+  auto Visit(Nonnull<const AstNode*>) -> bool { return true; }
+  auto Visit(ValueNodeView) -> bool { return true; }
+  auto Visit(int) -> bool { return true; }
+  auto Visit(Address) -> bool { return true; }
+  auto Visit(const std::string&) -> bool { return true; }
+  auto Visit(Nonnull<const NominalClassValue**>) -> bool {
+    // This is the pointer to the most-derived value within a class value,
+    // which is not "within" this value, so we shouldn't visit it.
+    return true;
+  }
+  auto Visit(const VTable&) -> bool { return true; }
+
+  llvm::function_ref<bool(const Value*)> callback;
+};
+}  // namespace
+
+auto VisitNestedValues(Nonnull<const Value*> value,
+                       llvm::function_ref<bool(const Value*)> visitor) -> bool {
+  return NestedValueVisitor{.callback = visitor}.Visit(value);
+}
+
 auto StructValue::FindField(std::string_view name) const
     -> std::optional<Nonnull<const Value*>> {
   for (const NamedValue& element : elements_) {

explorer/ast/value.h

@@ -122,6 +122,12 @@ auto ValueEqual(Nonnull<const Value*> v1, Nonnull<const Value*> v2,
                 std::optional<Nonnull<const EqualityContext*>> equality_ctx)
     -> bool;
 
+// Call the given `visitor` on all values nested within the given value,
+// including `value` itself, in a preorder traversal. Aborts and returns
+// `false` if `visitor` returns `false`, otherwise returns `true`.
+auto VisitNestedValues(Nonnull<const Value*> value,
+                       llvm::function_ref<bool(const Value*)> visitor) -> bool;
+
 // An integer value.
 class IntValue : public Value {
  public:

explorer/interpreter/type_checker.cpp

@@ -454,27 +454,17 @@ static auto ExpectResolvedBindingType(const BindingPattern& binding,
 }
 
 // Returns whether the given value is template-dependent, that is, if it
-// depends on any template paramaeter.
-static auto IsTemplateDependent(Nonnull<const Value*> value) -> bool {
-  // A VariableType is template dependent if it names a template binding.
-  if (const auto* var_type = dyn_cast<VariableType>(value)) {
-    return var_type->binding().binding_kind() ==
-           GenericBinding::BindingKind::Template;
-  }
-
-  static constexpr auto is_dependent_value = [](auto&& x) -> bool {
-    if constexpr (std::is_convertible_v<decltype(x), const Value*>) {
-      return IsTemplateDependent(x);
-    }
-    return false;
-  };
-
-  // Any other value is template dependent if any part of it is.
-  return value->Visit<bool>([](auto* derived_value) {
-    return derived_value->Decompose([](auto&&... parts) {
-      return (is_dependent_value(decltype(parts)(parts)) || ...);
-    });
-  });
+// depends on any template parameter.
+static auto DependsOnTemplateParameter(Nonnull<const Value*> value) -> bool {
+  bool mentions_no_template_parameters =
+      VisitNestedValues(value, [](Nonnull<const Value*> nested) -> bool {
+        if (const auto* var_type = dyn_cast<VariableType>(nested)) {
+          return var_type->binding().binding_kind() !=
+                 GenericBinding::BindingKind::Template;
+        }
+        return true;
+      });
+  return !mentions_no_template_parameters;
 }
 
 // Returns whether all template parameters in `bindings` are saturated: that
@@ -483,7 +473,7 @@ static auto IsTemplateDependent(Nonnull<const Value*> value) -> bool {
 static auto IsTemplateSaturated(const Bindings& bindings) -> bool {
   for (auto [binding, value] : bindings.args()) {
     if (binding->binding_kind() == GenericBinding::BindingKind::Template &&
-        IsTemplateDependent(value)) {
+        DependsOnTemplateParameter(value)) {
       return false;
     }
   }
@@ -6407,6 +6397,12 @@ auto TypeChecker::InstantiateImplDeclaration(
     }
   }
 
+  // TODO: Make this method non-const and remove the const-cast here. The
+  // requirement to perform template instantiation unfortunately means that a
+  // lot of type-checking stops being free of side-effects, so this means
+  // removing `const` throughout most of the type-checker.
+  auto* type_checker = const_cast<TypeChecker*>(this);
+
   // TODO: It's probably not correct to use the top-level impl scope here. It's
   // not obvious what we should use, though -- which impls are in scope in
   // template instantiation?
@@ -6414,11 +6410,34 @@ auto TypeChecker::InstantiateImplDeclaration(
       << "can't perform template instantiation with no top-level scope";
   ImplScope scope(*top_level_impl_scope_);
 
-  // TODO: Remove the const-cast here. The requirement to perform template
-  // instantiation unfortunately means that a lot of type-checking stops being
-  // free of side-effects, so this means removing `const` throughout most of
-  // the type-checker.
-  auto* type_checker = const_cast<TypeChecker*>(this);
+  // Bring all impls from any checked generic bindings in the template
+  // arguments into scope.
+  //
+  // TODO: There shouldn't be any checked generic bindings in the template
+  // arguments by the time we come to perform an instantiation, but in order
+  // for that to work, we need to defer instantiating templates until we know
+  // the values of checked generic parameters, such as by performing
+  // monomorphization for checked generics (see #2153 for details). However,
+  // explorer doesn't yet support that.
+  //
+  // As a workaround for the lack of support for #2153, we can instantiate
+  // templates with the argument equal to a generic parameter. When we do so,
+  // the constraints on that generic parameter need to be in scope in the
+  // instantiation. This is imperfect: it misses constraints on the binding
+  // that come from anywhere other than its type.
+  for (auto [param, value] : bindings->args()) {
+    if (param->binding_kind() != GenericBinding::BindingKind::Template) {
+      continue;
+    }
+    VisitNestedValues(value, [&](Nonnull<const Value*> nested) -> bool {
+      if (auto* var_type = dyn_cast<VariableType>(nested)) {
+        if (auto impl_binding = var_type->binding().impl_binding()) {
+          type_checker->BringImplBindingIntoScope(*impl_binding, scope);
+        }
+      }
+      return true;
+    });
+  }
 
   // Type-check the new impl.
   //

explorer/testdata/template/instantiate_from_generic.carbon

@@ -0,0 +1,34 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// CHECK:STDOUT: 1
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+interface I { fn F[self: Self](); }
+
+fn CheckTimplsI[T:! I](x: T) { x.F(); }
+
+interface J { fn F[self: Self](); }
+
+impl forall [template T:! I] T as J {
+  fn F[self: Self]() { CheckTimplsI(self); }
+}
+
+// Ensure that the instantiated `impl T as J` is type-checked in an impl scope
+// where `T impls I` is available, so that its call to `CheckTimplsI` is valid.
+fn UseTemplatedImplFromGeneric[T:! I](x: T) { x.(J.F)(); }
+
+impl i32 as I {
+  fn F[self: i32]() {
+    Print("{0}", self);
+  }
+}
+
+fn Main() -> i32 {
+  UseTemplatedImplFromGeneric(1);
+  return 0;
+}