Check for a symbolic value only if normal value lookup fails. (#1326)

This causes compile-time evaluation to give the same result as runtime
evaluation wherever possible. In particular, referencing a generic
parameter at compile time will resolve to the actual value if we're
evaluating a call to the enclosing function so a value for the
parameter is available in the call frame.

Clean up recently-added support for `SymbolicWitness`es using this.

explorer/ast/impl_binding.h

@@ -53,7 +53,11 @@ class ImplBinding : public AstNode {
     return std::nullopt;
   }
   auto symbolic_identity() const -> std::optional<Nonnull<const Value*>> {
-    return std::nullopt;
+    return symbolic_identity_;
+  }
+  void set_symbolic_identity(Nonnull<const Value*> value) {
+    CARBON_CHECK(!symbolic_identity_.has_value());
+    symbolic_identity_ = value;
   }
 
   // The static type of the impl. Cannot be called before typechecking.
@@ -81,6 +85,7 @@ class ImplBinding : public AstNode {
  private:
   Nonnull<const GenericBinding*> type_var_;
   Nonnull<const Value*> iface_;
+  std::optional<Nonnull<const Value*>> symbolic_identity_;
   std::optional<Nonnull<const Value*>> static_type_;
   std::optional<Nonnull<const ImplBinding*>> original_;
 };

explorer/interpreter/action_stack.cpp

@@ -52,11 +52,9 @@ void ActionStack::Initialize(ValueNodeView value_node,
 auto ActionStack::ValueOfNode(ValueNodeView value_node,
                               SourceLocation source_loc) const
     -> ErrorOr<Nonnull<const Value*>> {
-  std::optional<const Value*> value = (phase_ == Phase::CompileTime)
-                                          ? value_node.symbolic_identity()
-                                          : value_node.constant_value();
-  if (value.has_value()) {
-    return *value;
+  std::optional<const Value*> constant_value = value_node.constant_value();
+  if (constant_value.has_value()) {
+    return *constant_value;
   }
   for (const std::unique_ptr<Action>& action : todo_) {
     // TODO: have static name resolution identify the scope of value_node
@@ -77,6 +75,20 @@ auto ActionStack::ValueOfNode(ValueNodeView value_node,
       return *result;
     }
   }
+  // We don't know the value of this node, but at compile time we may still be
+  // able to form a symbolic value for it. For example, in
+  //
+  //   fn F[T:! Type](x: T) {}
+  //
+  // ... we don't know the value of `T` but can still symbolically evaluate it
+  // to a `VariableType`. At runtime we need actual values.
+  if (phase_ == Phase::CompileTime) {
+    std::optional<const Value*> symbolic_identity =
+        value_node.symbolic_identity();
+    if (symbolic_identity.has_value()) {
+      return *symbolic_identity;
+    }
+  }
   // TODO: Move these errors to compile time and explain them more clearly.
   return RuntimeError(source_loc)
          << "could not find `" << value_node.base() << "`";

explorer/interpreter/interpreter.cpp

@@ -918,16 +918,9 @@ auto Interpreter::StepExp() -> ErrorOr<Success> {
       CARBON_CHECK(act.pos() == 0);
       const auto& ident = cast<IdentifierExpression>(exp);
       // { {x :: C, E, F} :: S, H} -> { {H(E(x)) :: C, E, F} :: S, H}
-      auto value_or_error =
-          todo_.ValueOfNode(ident.value_node(), ident.source_loc());
-      if (!value_or_error.ok() && phase() == Phase::CompileTime &&
-          isa<ImplBinding>(ident.value_node().base())) {
-        // The `ImplBinding` might not be in scope. If so, just remember the
-        // expression from which it was derived.
-        return todo_.FinishAction(arena_->New<SymbolicWitness>(&exp));
-      }
-      CARBON_ASSIGN_OR_RETURN(Nonnull<const Value*> value,
-                              std::move(value_or_error));
+      CARBON_ASSIGN_OR_RETURN(
+          Nonnull<const Value*> value,
+          todo_.ValueOfNode(ident.value_node(), ident.source_loc()));
       if (const auto* lvalue = dyn_cast<LValue>(value)) {
         CARBON_ASSIGN_OR_RETURN(
             value, heap_.Read(lvalue->address(), exp.source_loc()));

explorer/interpreter/type_checker.cpp

@@ -2348,6 +2348,8 @@ auto TypeChecker::TypeCheckPattern(
       if (isa<InterfaceType, ConstraintType>(type)) {
         Nonnull<ImplBinding*> impl_binding =
             arena_->New<ImplBinding>(binding.source_loc(), &binding, type);
+        impl_binding->set_symbolic_identity(
+            arena_->New<SymbolicWitness>(CreateImplReference(impl_binding)));
         binding.set_impl_binding(impl_binding);
         BringImplIntoScope(impl_binding, impl_scope);
       }

explorer/testdata/generic_class/instantiation_at_compile_time.carbon

@@ -0,0 +1,24 @@
+// 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
+//
+// RUN: %{explorer} %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s
+// RUN: %{explorer} --parser_debug --trace_file=- %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes %s
+// AUTOUPDATE: %{explorer} %s
+// CHECK: result: 0
+
+package ExplorerTest api;
+
+class C(T:! Type) {}
+
+fn F(T:! Type) -> Type {
+  return C(T);
+}
+
+fn Main() -> i32 {
+  var v: F(i32) = {};
+  var w: C(i32) = v;
+  return 0;
+}