When converting an expression to type type, retain the resulting in…

…struction as well as the `TypeId`. (carbon-language#4355)

The `TypeId` is lossy, as it represents only the canonical type, and not
the specific computation that produced it.

toolchain/check/convert.cpp

@@ -853,7 +853,7 @@ static auto PerformBuiltinConversion(Context& context, SemIR::LocId loc_id,
            sem_ir.inst_blocks().Get(tuple_literal->elements_id)) {
         // TODO: This call recurses back into conversion. Switch to an
         // iterative approach.
-        type_ids.push_back(ExprAsType(context, loc_id, tuple_inst_id));
+        type_ids.push_back(ExprAsType(context, loc_id, tuple_inst_id).type_id);
       }
       auto tuple_type_id = context.GetTupleType(type_ids);
       return sem_ir.types().GetInstId(tuple_type_id);
@@ -1271,22 +1271,24 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
 }
 
 auto ExprAsType(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id)
-    -> SemIR::TypeId {
+    -> TypeExpr {
   auto type_inst_id =
       ConvertToValueOfType(context, loc_id, value_id, SemIR::TypeId::TypeType);
   if (type_inst_id == SemIR::InstId::BuiltinError) {
-    return SemIR::TypeId::Error;
+    return {.inst_id = type_inst_id, .type_id = SemIR::TypeId::Error};
   }
 
   auto type_const_id = context.constant_values().Get(type_inst_id);
   if (!type_const_id.is_constant()) {
     CARBON_DIAGNOSTIC(TypeExprEvaluationFailure, Error,
                       "cannot evaluate type expression");
     context.emitter().Emit(loc_id, TypeExprEvaluationFailure);
-    return SemIR::TypeId::Error;
+    return {.inst_id = SemIR::InstId::BuiltinError,
+            .type_id = SemIR::TypeId::Error};
   }
 
-  return context.GetTypeIdForTypeConstant(type_const_id);
+  return {.inst_id = type_inst_id,
+          .type_id = context.GetTypeIdForTypeConstant(type_const_id)};
 }
 
 }  // namespace Carbon::Check

toolchain/check/convert.h

@@ -119,9 +119,20 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
                      SemIR::SpecificId callee_specific_id)
     -> SemIR::InstBlockId;
 
+// A type that has been converted for use as a type expression.
+struct TypeExpr {
+  // The converted expression of type `type`, or `InstId::BuiltinError`.
+  SemIR::InstId inst_id;
+  // The corresponding type, or `TypeId::Error`.
+  SemIR::TypeId type_id;
+};
+
 // Converts an expression for use as a type.
+// TODO: Most of the callers of this function discard the `inst_id` and lose
+// track of the conversion. In most cases we should be retaining that as the
+// operand of some downstream instruction.
 auto ExprAsType(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id)
-    -> SemIR::TypeId;
+    -> TypeExpr;
 
 }  // namespace Carbon::Check
 

toolchain/check/handle_array.cpp

@@ -49,7 +49,8 @@ auto HandleParseNode(Context& context, Parse::ArrayExprId node_id) -> bool {
       node_id, {.type_id = SemIR::TypeId::TypeType,
                 .bound_id = bound_inst_id,
                 .element_type_id = ExprAsType(context, element_type_node_id,
-                                              element_type_inst_id)});
+                                              element_type_inst_id)
+                                       .type_id});
   return true;
 }
 

toolchain/check/handle_binding_pattern.cpp

@@ -14,7 +14,7 @@ namespace Carbon::Check {
 static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
                                     bool is_generic) -> bool {
   auto [type_node, parsed_type_id] = context.node_stack().PopExprWithNodeId();
-  auto cast_type_id = ExprAsType(context, type_node, parsed_type_id);
+  auto cast_type_id = ExprAsType(context, type_node, parsed_type_id).type_id;
 
   // TODO: Handle `_` bindings.
 

toolchain/check/handle_class.cpp

@@ -391,7 +391,8 @@ auto HandleParseNode(Context& context, Parse::AdaptDeclId node_id) -> bool {
     return true;
   }
 
-  auto adapted_type_id = ExprAsType(context, node_id, adapted_type_expr_id);
+  auto adapted_type_id =
+      ExprAsType(context, node_id, adapted_type_expr_id).type_id;
   adapted_type_id = context.AsCompleteType(adapted_type_id, [&] {
     CARBON_DIAGNOSTIC(IncompleteTypeInAdaptDecl, Error,
                       "adapted type `{0}` is an incomplete type",
@@ -467,7 +468,7 @@ static auto DiagnoseBaseIsFinal(Context& context, Parse::NodeId node_id,
 // Checks that the specified base type is valid.
 static auto CheckBaseType(Context& context, Parse::NodeId node_id,
                           SemIR::InstId base_expr_id) -> BaseInfo {
-  auto base_type_id = ExprAsType(context, node_id, base_expr_id);
+  auto base_type_id = ExprAsType(context, node_id, base_expr_id).type_id;
   base_type_id = context.AsCompleteType(base_type_id, [&] {
     CARBON_DIAGNOSTIC(IncompleteTypeInBaseDecl, Error,
                       "base `{0}` is an incomplete type", SemIR::TypeId);

toolchain/check/handle_function.cpp

@@ -43,7 +43,7 @@ auto HandleParseNode(Context& context, Parse::FunctionIntroducerId node_id)
 auto HandleParseNode(Context& context, Parse::ReturnTypeId node_id) -> bool {
   // Propagate the type expression.
   auto [type_node_id, type_inst_id] = context.node_stack().PopExprWithNodeId();
-  auto type_id = ExprAsType(context, type_node_id, type_inst_id);
+  auto type_id = ExprAsType(context, type_node_id, type_inst_id).type_id;
   // TODO: Use a dedicated instruction rather than VarStorage here.
   context.AddInstAndPush<SemIR::VarStorage>(
       node_id, {.type_id = type_id, .name_id = SemIR::NameId::ReturnSlot});

toolchain/check/handle_impl.cpp

@@ -55,15 +55,14 @@ auto HandleParseNode(Context& context, Parse::ImplForallId node_id) -> bool {
 
 auto HandleParseNode(Context& context, Parse::TypeImplAsId node_id) -> bool {
   auto [self_node, self_id] = context.node_stack().PopExprWithNodeId();
-  auto self_type_id = ExprAsType(context, self_node, self_id);
+  auto [self_inst_id, self_type_id] = ExprAsType(context, self_node, self_id);
   context.node_stack().Push(node_id, self_type_id);
 
   // Introduce `Self`. Note that we add this name lexically rather than adding
   // to the `NameScopeId` of the `impl`, because this happens before we enter
   // the `impl` scope or even identify which `impl` we're declaring.
   // TODO: Revisit this once #3714 is resolved.
-  context.AddNameToLookup(SemIR::NameId::SelfType,
-                          context.types().GetInstId(self_type_id));
+  context.AddNameToLookup(SemIR::NameId::SelfType, self_inst_id);
   return true;
 }
 
@@ -250,7 +249,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
 
   // Convert the constraint expression to a type.
   // TODO: Check that its constant value is a constraint.
-  auto constraint_type_id = ExprAsType(context, constraint_node, constraint_id);
+  auto constraint_type_id =
+      ExprAsType(context, constraint_node, constraint_id).type_id;
 
   // Process modifiers.
   // TODO: Should we somehow permit access specifiers on `impl`s?

toolchain/check/handle_operator.cpp

@@ -48,7 +48,7 @@ auto HandleParseNode(Context& context, Parse::InfixOperatorAsId node_id)
   auto [rhs_node, rhs_id] = context.node_stack().PopExprWithNodeId();
   auto [lhs_node, lhs_id] = context.node_stack().PopExprWithNodeId();
 
-  auto rhs_type_id = ExprAsType(context, rhs_node, rhs_id);
+  auto rhs_type_id = ExprAsType(context, rhs_node, rhs_id).type_id;
   context.node_stack().Push(
       node_id, ConvertForExplicitAs(context, node_id, lhs_id, rhs_type_id));
   return true;
@@ -213,7 +213,7 @@ auto HandleParseNode(Context& context, Parse::InfixOperatorStarEqualId node_id)
 auto HandleParseNode(Context& context, Parse::PostfixOperatorStarId node_id)
     -> bool {
   auto value_id = context.node_stack().PopExpr();
-  auto inner_type_id = ExprAsType(context, node_id, value_id);
+  auto inner_type_id = ExprAsType(context, node_id, value_id).type_id;
   context.AddInstAndPush<SemIR::PointerType>(
       node_id,
       {.type_id = SemIR::TypeId::TypeType, .pointee_id = inner_type_id});
@@ -266,7 +266,7 @@ auto HandleParseNode(Context& context, Parse::PrefixOperatorConstId node_id)
                       "additional effect");
     context.emitter().Emit(node_id, RepeatedConst);
   }
-  auto inner_type_id = ExprAsType(context, node_id, value_id);
+  auto inner_type_id = ExprAsType(context, node_id, value_id).type_id;
   context.AddInstAndPush<SemIR::ConstType>(
       node_id, {.type_id = SemIR::TypeId::TypeType, .inner_id = inner_type_id});
   return true;

toolchain/check/handle_struct.cpp

@@ -58,7 +58,7 @@ auto HandleParseNode(Context& context, Parse::StructFieldId node_id) -> bool {
 auto HandleParseNode(Context& context, Parse::StructTypeFieldId node_id)
     -> bool {
   auto [type_node, type_id] = context.node_stack().PopExprWithNodeId();
-  SemIR::TypeId cast_type_id = ExprAsType(context, type_node, type_id);
+  SemIR::TypeId cast_type_id = ExprAsType(context, type_node, type_id).type_id;
 
   auto [name_node, name_id] = context.node_stack().PopNameWithNodeId();
 

toolchain/check/handle_where.cpp

@@ -14,7 +14,7 @@ auto HandleParseNode(Context& context, Parse::WhereOperandId node_id) -> bool {
   // is being modified by the `where` operator. It would be `MyInterface` in
   // `MyInterface where .Member = i32`.
   auto [self_node, self_id] = context.node_stack().PopExprWithNodeId();
-  auto self_type_id = ExprAsType(context, self_node, self_id);
+  auto self_type_id = ExprAsType(context, self_node, self_id).type_id;
   // TODO: Validate that `self_type_id` represents a facet type. Only facet
   // types may have `where` restrictions.
 

toolchain/check/testdata/function/builtin/no_prelude/call_from_operator.carbon

@@ -178,13 +178,13 @@ var arr: [i32; 1 + 2] = (3, 4, 3 + 4);
 // CHECK:STDOUT:     %self.patt: i32 = binding_pattern self
 // CHECK:STDOUT:     %other.patt: i32 = binding_pattern other
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %Self.ref.loc7_15: type = name_ref Self, i32 [template = i32]
+// CHECK:STDOUT:     %Self.ref.loc7_15: type = name_ref Self, @impl.%.loc6_6.2 [template = i32]
 // CHECK:STDOUT:     %self.param: i32 = param self, runtime_param0
 // CHECK:STDOUT:     %self: i32 = bind_name self, %self.param
-// CHECK:STDOUT:     %Self.ref.loc7_28: type = name_ref Self, i32 [template = i32]
+// CHECK:STDOUT:     %Self.ref.loc7_28: type = name_ref Self, @impl.%.loc6_6.2 [template = i32]
 // CHECK:STDOUT:     %other.param: i32 = param other, runtime_param1
 // CHECK:STDOUT:     %other: i32 = bind_name other, %other.param
-// CHECK:STDOUT:     %Self.ref.loc7_37: type = name_ref Self, i32 [template = i32]
+// CHECK:STDOUT:     %Self.ref.loc7_37: type = name_ref Self, @impl.%.loc6_6.2 [template = i32]
 // CHECK:STDOUT:     %return: ref i32 = var <return slot>
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.loc6_22: <witness> = interface_witness (%Op.decl) [template = constants.%.3]

toolchain/check/testdata/impl/lookup/no_prelude/impl_forall.carbon

@@ -205,7 +205,7 @@ fn TestSpecific(a: A({})) -> {} {
 // CHECK:STDOUT:     %F.decl: %F.type.2 = fn_decl @F.2 [template = constants.%F.2] {
 // CHECK:STDOUT:       %self.patt: @F.2.%A (%A.3) = binding_pattern self
 // CHECK:STDOUT:     } {
-// CHECK:STDOUT:       %Self.ref: type = name_ref Self, constants.%A.3 [symbolic = %A (constants.%A.3)]
+// CHECK:STDOUT:       %Self.ref: type = name_ref Self, @impl.%A.loc10 [symbolic = %A (constants.%A.3)]
 // CHECK:STDOUT:       %self.param: @F.2.%A (%A.3) = param self, runtime_param0
 // CHECK:STDOUT:       %self: @F.2.%A (%A.3) = bind_name self, %self.param
 // CHECK:STDOUT:       %V.ref: type = name_ref V, @impl.%V.loc10 [symbolic = %V (constants.%V)]

toolchain/check/testdata/impl/no_prelude/import_self.carbon

@@ -155,13 +155,13 @@ fn F(x: (), y: ()) -> () {
 // CHECK:STDOUT:     %self.patt: %.1 = binding_pattern self
 // CHECK:STDOUT:     %other.patt: %.1 = binding_pattern other
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %Self.ref.loc7_15: type = name_ref Self, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:     %Self.ref.loc7_15: type = name_ref Self, @impl.%.loc6_7.2 [template = constants.%.1]
 // CHECK:STDOUT:     %self.param: %.1 = param self, runtime_param0
 // CHECK:STDOUT:     %self: %.1 = bind_name self, %self.param
-// CHECK:STDOUT:     %Self.ref.loc7_28: type = name_ref Self, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:     %Self.ref.loc7_28: type = name_ref Self, @impl.%.loc6_7.2 [template = constants.%.1]
 // CHECK:STDOUT:     %other.param: %.1 = param other, runtime_param1
 // CHECK:STDOUT:     %other: %.1 = bind_name other, %other.param
-// CHECK:STDOUT:     %Self.ref.loc7_37: type = name_ref Self, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:     %Self.ref.loc7_37: type = name_ref Self, @impl.%.loc6_7.2 [template = constants.%.1]
 // CHECK:STDOUT:     %return: ref %.1 = var <return slot>
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.loc6_16: <witness> = interface_witness (%Op.decl) [template = constants.%.3]

toolchain/check/testdata/impl/no_prelude/self_in_class.carbon

@@ -72,7 +72,7 @@ class A {
 // CHECK:STDOUT:
 // CHECK:STDOUT: impl @impl: %C as %.1 {
 // CHECK:STDOUT:   %Make.decl: %Make.type.2 = fn_decl @Make.2 [template = constants.%Make.2] {} {
-// CHECK:STDOUT:     %Self.ref: type = name_ref Self, constants.%C [template = constants.%C]
+// CHECK:STDOUT:     %Self.ref: type = name_ref Self, @impl.%C.ref [template = constants.%C]
 // CHECK:STDOUT:     %return: ref %C = var <return slot>
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.loc18: <witness> = interface_witness (%Make.decl) [template = constants.%.7]

toolchain/check/testdata/impl/no_prelude/self_in_signature.carbon

@@ -195,13 +195,13 @@ impl D as SelfNested {
 // CHECK:STDOUT:     %self.patt: %D = binding_pattern self
 // CHECK:STDOUT:     %x.patt: %D = binding_pattern x
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %Self.ref.loc24_14: type = name_ref Self, constants.%D [template = constants.%D]
+// CHECK:STDOUT:     %Self.ref.loc24_14: type = name_ref Self, @impl.2.%D.ref [template = constants.%D]
 // CHECK:STDOUT:     %self.param: %D = param self, runtime_param0
 // CHECK:STDOUT:     %self: %D = bind_name self, %self.param
-// CHECK:STDOUT:     %Self.ref.loc24_23: type = name_ref Self, constants.%D [template = constants.%D]
+// CHECK:STDOUT:     %Self.ref.loc24_23: type = name_ref Self, @impl.2.%D.ref [template = constants.%D]
 // CHECK:STDOUT:     %x.param: %D = param x, runtime_param1
 // CHECK:STDOUT:     %x: %D = bind_name x, %x.param
-// CHECK:STDOUT:     %Self.ref.loc24_32: type = name_ref Self, constants.%D [template = constants.%D]
+// CHECK:STDOUT:     %Self.ref.loc24_32: type = name_ref Self, @impl.2.%D.ref [template = constants.%D]
 // CHECK:STDOUT:     %return: ref %D = var <return slot>
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.loc23: <witness> = interface_witness (%F.decl) [template = constants.%.9]
@@ -237,9 +237,9 @@ impl D as SelfNested {
 // CHECK:STDOUT:   %F.decl: %F.type.6 = fn_decl @F.6 [template = constants.%F.6] {
 // CHECK:STDOUT:     %x.patt: %.23 = binding_pattern x
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %Self.ref.loc36_12: type = name_ref Self, constants.%D [template = constants.%D]
+// CHECK:STDOUT:     %Self.ref.loc36_12: type = name_ref Self, @impl.4.%D.ref [template = constants.%D]
 // CHECK:STDOUT:     %.loc36_16: type = ptr_type %D [template = constants.%.21]
-// CHECK:STDOUT:     %Self.ref.loc36_24: type = name_ref Self, constants.%D [template = constants.%D]
+// CHECK:STDOUT:     %Self.ref.loc36_24: type = name_ref Self, @impl.4.%D.ref [template = constants.%D]
 // CHECK:STDOUT:     %.loc36_35.1: %.2 = tuple_literal ()
 // CHECK:STDOUT:     %.loc36_35.2: type = converted %.loc36_35.1, constants.%.2 [template = constants.%.2]
 // CHECK:STDOUT:     %.loc36_36: type = struct_type {.x: %D, .y: %.2} [template = constants.%.22]