Add cache of template instantiations. (#3243)

Also some drive-by comment and trace improvements.

Closes #2951

explorer/interpreter/interpreter.cpp

@@ -516,49 +516,62 @@ auto Interpreter::InstantiateType(Nonnull<const Value*> type,
                            << source_loc << ")\n";
   }
 
+  const Value* value = nullptr;
   switch (type->kind()) {
     case Value::Kind::VariableType: {
       CARBON_ASSIGN_OR_RETURN(
-          Nonnull<const Value*> value,
+          value,
           todo_.ValueOfNode(&cast<VariableType>(*type).binding(), source_loc));
       if (const auto* location = dyn_cast<LocationValue>(value)) {
         CARBON_ASSIGN_OR_RETURN(value,
                                 heap_.Read(location->address(), source_loc));
       }
-      return value;
+      break;
     }
     case Value::Kind::InterfaceType: {
       const auto& interface_type = cast<InterfaceType>(*type);
       CARBON_ASSIGN_OR_RETURN(
           Nonnull<const Bindings*> bindings,
           InstantiateBindings(&interface_type.bindings(), source_loc));
-      return arena_->New<InterfaceType>(&interface_type.declaration(),
-                                        bindings);
+      value =
+          arena_->New<InterfaceType>(&interface_type.declaration(), bindings);
+      break;
     }
     case Value::Kind::NamedConstraintType: {
       const auto& constraint_type = cast<NamedConstraintType>(*type);
       CARBON_ASSIGN_OR_RETURN(
           Nonnull<const Bindings*> bindings,
           InstantiateBindings(&constraint_type.bindings(), source_loc));
-      return arena_->New<NamedConstraintType>(&constraint_type.declaration(),
-                                              bindings);
+      value = arena_->New<NamedConstraintType>(&constraint_type.declaration(),
+                                               bindings);
+      break;
     }
     case Value::Kind::ChoiceType: {
       const auto& choice_type = cast<ChoiceType>(*type);
       CARBON_ASSIGN_OR_RETURN(
           Nonnull<const Bindings*> bindings,
           InstantiateBindings(&choice_type.bindings(), source_loc));
-      return arena_->New<ChoiceType>(&choice_type.declaration(), bindings);
+      value = arena_->New<ChoiceType>(&choice_type.declaration(), bindings);
+      break;
     }
     case Value::Kind::AssociatedConstant: {
       CARBON_ASSIGN_OR_RETURN(
           Nonnull<const Value*> type_value,
           EvalAssociatedConstant(cast<AssociatedConstant>(type), source_loc));
-      return type_value;
+      value = type_value;
+      break;
     }
     default:
-      return type;
+      value = type;
+      break;
+  }
+
+  if (trace_stream_->is_enabled()) {
+    trace_stream_->End() << "instantiated type `" << *type << "` as `" << *value
+                         << "` (" << source_loc << ")\n";
   }
+
+  return value;
 }
 
 auto Interpreter::InstantiateBindings(Nonnull<const Bindings*> bindings,

explorer/interpreter/type_checker.cpp

@@ -2176,13 +2176,10 @@ class TypeChecker::SubstituteTransform
     const auto* declaration = &witness->declaration();
     if (!IsTemplateSaturated(witness->bindings()) &&
         IsTemplateSaturated(*bindings)) {
-      CARBON_ASSIGN_OR_RETURN(
-          CARBON_PROTECT_COMMAS(auto [new_decl, new_bindings]),
-          type_checker_->InstantiateImplDeclaration(declaration, bindings));
-      declaration = new_decl;
-      bindings = new_bindings;
+      return type_checker_->InstantiateImplDeclaration(declaration, bindings);
+    } else {
+      return type_checker_->arena_->New<ImplWitness>(declaration, bindings);
     }
-    return type_checker_->arena_->New<ImplWitness>(declaration, bindings);
   }
 
   // For an associated constant, look for a rewrite.
@@ -5805,6 +5802,11 @@ auto TypeChecker::DeclareImplDeclaration(Nonnull<ImplDeclaration*> impl_decl,
                            << impl_decl->source_loc() << ")\n";
   }
 
+  // We need to eagerly typecheck portions of the impl in terms of the generic
+  // parameters, and then typecheck it again at instantiation time in terms of
+  // the actual arguments. The AST doesn't allow type information to be mutated,
+  // So in order to do that, we need to preserve a clone that doesn't have type
+  // information attached.
   if (!IsTemplateSaturated(impl_decl->deduced_parameters())) {
     CloneContext context(arena_);
     TemplateInfo template_info = {.pattern = context.Clone(impl_decl)};
@@ -6502,25 +6504,31 @@ auto TypeChecker::FindCollectedMembers(Nonnull<const Declaration*> decl)
 }
 
 auto TypeChecker::InstantiateImplDeclaration(
-    Nonnull<const ImplDeclaration*> old_impl,
+    Nonnull<const ImplDeclaration*> pattern,
     Nonnull<const Bindings*> bindings) const
-    -> ErrorOr<std::pair<Nonnull<ImplDeclaration*>, Nonnull<Bindings*>>> {
+    -> ErrorOr<Nonnull<const ImplWitness*>> {
   CARBON_CHECK(IsTemplateSaturated(*bindings));
 
   if (trace_stream_->is_enabled()) {
-    trace_stream_->Start() << "instantiating `" << PrintAsID(*old_impl) << "` ("
-                           << old_impl->source_loc() << ")\n";
+    trace_stream_->Start() << "instantiating `" << PrintAsID(*pattern) << "` ("
+                           << pattern->source_loc() << ")\n";
     *trace_stream_ << *bindings << "\n";
   }
 
-  SetFileContext set_file_context(*trace_stream_, old_impl->source_loc());
+  SetFileContext set_file_context(*trace_stream_, pattern->source_loc());
 
-  auto it = templates_.find(old_impl);
+  auto it = templates_.find(pattern);
   CARBON_CHECK(it != templates_.end());
   const TemplateInfo& info = it->second;
 
-  // TODO: Only instantiate each declaration once for each set of template
-  // arguments.
+  if (auto instantiation = info.instantiations.find(bindings);
+      instantiation != info.instantiations.end()) {
+    if (trace_stream_->is_enabled()) {
+      *trace_stream_ << "reusing cached instantiation\n";
+    }
+    return instantiation->second;
+  }
+
   CloneContext context(arena_);
   Nonnull<ImplDeclaration*> impl =
       context.Clone(cast<ImplDeclaration>(info.pattern));
@@ -6605,7 +6613,10 @@ auto TypeChecker::InstantiateImplDeclaration(
       /*is_template_instantiation=*/true));
   CARBON_RETURN_IF_ERROR(type_checker->TypeCheckImplDeclaration(impl, scope));
 
-  return std::pair{impl, arena_->New<Bindings>(std::move(new_bindings))};
+  auto* result = arena_->New<ImplWitness>(
+      impl, arena_->New<Bindings>(std::move(new_bindings)));
+  CARBON_CHECK(info.instantiations.insert({bindings, result}).second);
+  return result;
 }
 
 auto TypeChecker::InterpExp(Nonnull<const Expression*> e)

explorer/interpreter/type_checker.h

@@ -558,7 +558,7 @@ class TypeChecker {
   // template bindings.
   auto InstantiateImplDeclaration(Nonnull<const ImplDeclaration*> pattern,
                                   Nonnull<const Bindings*> bindings) const
-      -> ErrorOr<std::pair<Nonnull<ImplDeclaration*>, Nonnull<Bindings*>>>;
+      -> ErrorOr<Nonnull<const ImplWitness*>>;
 
   // Wraps the interpreter's InterpExp, forwarding TypeChecker members as
   // arguments.
@@ -598,8 +598,20 @@ class TypeChecker {
     // A mapping from the bindings of the type-checked pattern to the bindings
     // of the original.
     std::map<const GenericBinding*, const GenericBinding*> param_map;
-    // TODO: Keep track of the instantiations we've already performed and don't
-    // do them again.
+
+    // Comparator for pointers to Bindings.
+    struct BindingPtrCompare {
+      auto operator()(Nonnull<const Bindings*> lhs,
+                      Nonnull<const Bindings*> rhs) const {
+        return std::tie(lhs->args(), lhs->witnesses()) <
+               std::tie(rhs->args(), rhs->witnesses());
+      }
+    };
+
+    // Cache of instantiations of this template.
+    mutable std::map<Nonnull<const Bindings*>, Nonnull<const ImplWitness*>,
+                     BindingPtrCompare>
+        instantiations;
   };
 
   // Map from template declarations to extra information we use to type-check

explorer/testdata/template/instantiations_are_cached.carbon

@@ -0,0 +1,32 @@
+// 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
+
+package ExplorerTest api;
+
+interface I { fn F(); }
+
+impl () as I {
+  fn F() {}
+}
+
+impl forall [template T:! type] (T,) as I {
+  fn F() {
+    if (true) {
+      T.(I.F)();
+    } else {
+      T.(I.F)();
+    }
+  }
+}
+
+fn Main() -> i32 {
+  // This would cause an exponential number of template instantiations if we
+  // didn't cache them.
+  ((((((((((((((((((((((((((((((((),),),),),),),),),),),),),),),),),),),),),),),),),),),),),),)).(I.F)();
+	return 0;
+}
+
+// CHECK:STDOUT: result: 0

explorer/testdata/trace/phase_execution.carbon

@@ -101,6 +101,8 @@ fn Main() -> i32 {
 // CHECK:STDOUT: ->> step TypeInstantiationAction pos: 0 `interface As(T = class T)` (phase_execution.carbon:10) --->
 // CHECK:STDOUT: ->> instantiating type `interface As(T = class T)` (phase_execution.carbon:10)
 // CHECK:STDOUT: ->> instantiating type `class T` (phase_execution.carbon:10)
+// CHECK:STDOUT: <<- instantiated type `class T` as `class T` (phase_execution.carbon:10)
+// CHECK:STDOUT: <<- instantiated type `interface As(T = class T)` as `interface As(T = class T)` (phase_execution.carbon:10)
 // CHECK:STDOUT: <[] stack-pop:  TypeInstantiationAction pos: 0 `interface As(T = class T)` (phase_execution.carbon:10)
 // CHECK:STDOUT: ->> step ExpressionAction pos: 3 `{}.(interface ImplicitAs(T = class T).Convert)` results: [`{}`, `witness for impl T as ImplicitAs(U)`, `interface As(T = class T)`]  (phase_execution.carbon:10) --->
 // CHECK:STDOUT: ->> step ExpressionAction pos: 4 `{}.(interface ImplicitAs(T = class T).Convert)` results: [`{}`, `witness for impl T as ImplicitAs(U)`, `interface As(T = class T)`]  (phase_execution.carbon:10) --->
@@ -150,6 +152,7 @@ fn Main() -> i32 {
 // CHECK:STDOUT: <[] stack-pop:  TypeInstantiationAction pos: 0 `class T` (prelude.carbon:{{\d+}})
 // CHECK:STDOUT: ->> step ExpressionAction pos: 2 `self` results: [`{}`, `class T`]  (prelude.carbon:{{\d+}}) --->
 // CHECK:STDOUT: ->> instantiating type `class T` (prelude.carbon:{{\d+}})
+// CHECK:STDOUT: <<- instantiated type `class T` as `class T` (prelude.carbon:{{\d+}})
 // CHECK:STDOUT: <[] stack-pop:  ExpressionAction pos: 2 `self` results: [`{}`, `class T`]  (prelude.carbon:{{\d+}})
 // CHECK:STDOUT: ->> step ValueExpressionAction pos: 1 `__intrinsic_implicit_as_convert(self, U)` results: [`T{}`]  (prelude.carbon:{{\d+}}) --->
 // CHECK:STDOUT: <[] stack-pop:  ValueExpressionAction pos: 1 `__intrinsic_implicit_as_convert(self, U)` results: [`T{}`]  (prelude.carbon:{{\d+}})