Просмотр исходного кода

Call GetConstantFacetTypeInfo on fully constructed FacetTypeInfo in WhereExpr and BitAnd (#5647)

The `BitAnd` operation combines two `FacetTypeInfo` structures by
concatenating their lists, but did not apply the current specific to the
instructions in the `FacetTypeInfo` as it forgot to go through
`GetContantFacetTypeInfo`.

`WhereExpr` handling duplicates a lot of the logic in
`GetConstantFacetTypeInfo` by calling `GetConstantValue` on things,
instead of calling `GetConstantFacetTypeInfo` on the `FacetTypeInfo` it
constructs. This meant it also needed to call `GetConstantFacetTypeInfo`
on the base facet type, and on any `impls`-requirement facet types
before merging their values together into a single `FacetTypeInfo`.

Instead, make `WhereExpr` more like `BitAnd`, and have it concatenate
things together as-is to construct a `FacetTypeInfo`. Then call
`GetConstantFacetTypeInfo` to canonicalize it and return a constant
value referring to it.

In `GetConstantFacetTypeInfo` we fix a crasher by propagating errors
inserted into the `FacetTypeInfo` out to the `Phase` so that the
resulting instruction depending on the `FacetTypeInfo` is not resolved
to a constant value with errors inside it. A test is added for this,
which was crashing on import of the `FacetType` with an error within
from the imported `impl` decl.

This refactoring gives us three benefits:
* There's now only a single place that does
`ResolveRewriteConstraintsAndCanonicalize`, which is inside
`GetConstantFacetTypeInfo`. This makes the inputs/behaviour of
`ResolveRewriteConstraintsAndCanonicalize` more consistent.
* There's now only a single place that updates the instructions in
`FacetTypeInfo` constraints with new constant values, so that changes
that rely on observing and interacting with that code only need to be
written in a single place. This will avoid duplicating logic in
https://github.com/carbon-language/carbon-lang/pull/5644.
* This will make it easier to move `WhereExpr` handling to a
`EvalConstantInst` function, as it no longer directly depends on
`GetConstantValue()` from `eval.cpp`.
Dana Jansens 10 месяцев назад
Родитель
Сommit
3689a3b3e4

+ 54 - 42
toolchain/check/eval.cpp

@@ -580,12 +580,11 @@ static auto GetConstantValue(EvalContext& eval_context,
            GetConstantValue(eval_context, interface.specific_id, phase)});
 }
 
-// Like `GetConstantValue` but does a `FacetTypeId` -> `FacetTypeInfo`
-// conversion. Does not perform canonicalization.
+// Like `GetConstantValue` but for a `FacetTypeInfo`.
 static auto GetConstantFacetTypeInfo(EvalContext& eval_context,
-                                     SemIR::FacetTypeId facet_type_id,
+                                     SemIR::LocId loc_id,
+                                     const SemIR::FacetTypeInfo& orig,
                                      Phase* phase) -> SemIR::FacetTypeInfo {
-  const auto& orig = eval_context.facet_types().Get(facet_type_id);
   SemIR::FacetTypeInfo info;
   info.extend_constraints.reserve(orig.extend_constraints.size());
   for (const auto& interface : orig.extend_constraints) {
@@ -612,16 +611,23 @@ static auto GetConstantFacetTypeInfo(EvalContext& eval_context,
   }
   // TODO: Process other requirements.
   info.other_requirements = orig.other_requirements;
+
+  if (!ResolveRewriteConstraintsAndCanonicalize(eval_context.context(), loc_id,
+                                                info)) {
+    // TODO: Should ResolveRewriteConstraintsAndCanonicalize() move into
+    // eval.cpp so it can set the Phase directly?
+    *phase = Phase::UnknownDueToError;
+  }
+
   return info;
 }
 
 static auto GetConstantValue(EvalContext& eval_context,
                              SemIR::FacetTypeId facet_type_id, Phase* phase)
     -> SemIR::FacetTypeId {
-  SemIR::FacetTypeInfo info =
-      GetConstantFacetTypeInfo(eval_context, facet_type_id, phase);
-  ResolveRewriteConstraintsAndCanonicalize(eval_context.context(),
-                                           SemIR::LocId::None, info);
+  SemIR::FacetTypeInfo info = GetConstantFacetTypeInfo(
+      eval_context, SemIR::LocId::None,
+      eval_context.facet_types().Get(facet_type_id), phase);
   // TODO: Return `facet_type_id` if we can detect nothing has changed.
   return eval_context.facet_types().Add(info);
 }
@@ -1537,11 +1543,16 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
         return context.types().GetConstantId(
             context.types().GetTypeIdForTypeInstId(arg_ids[0]));
       }
-      auto info = SemIR::FacetTypeInfo::Combine(
+      auto combined_info = SemIR::FacetTypeInfo::Combine(
           context.facet_types().Get(lhs_facet_type_id),
           context.facet_types().Get(rhs_facet_type_id));
-      ResolveRewriteConstraintsAndCanonicalize(context, loc_id, info);
-      return MakeFacetTypeResult(eval_context.context(), info, phase);
+      if (!ResolveRewriteConstraintsAndCanonicalize(eval_context.context(),
+                                                    loc_id, combined_info)) {
+        // TODO: Should ResolveRewriteConstraintsAndCanonicalize() move into
+        // eval.cpp so it can set the Phase directly?
+        phase = Phase::UnknownDueToError;
+      }
+      return MakeFacetTypeResult(eval_context.context(), combined_info, phase);
     }
 
     case SemIR::BuiltinFunctionKind::IntLiteralMakeType: {
@@ -1969,7 +1980,7 @@ static auto IsPeriodSelf(EvalContext& eval_context, SemIR::ConstantId const_id)
   return false;
 }
 
-// TODO: Convert this to an EvalConstantInst instruction. This will require
+// TODO: Convert this to an EvalConstantInst function. This will require
 // providing a `GetConstantValue` overload for a requirement block.
 template <>
 auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
@@ -1983,11 +1994,11 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
   SemIR::Inst base_facet_inst =
       eval_context.types().GetAsInst(base_facet_type_id);
   SemIR::FacetTypeInfo info = {.other_requirements = false};
+
   // `where` provides that the base facet is an error, `type`, or a facet
   // type.
   if (auto facet_type = base_facet_inst.TryAs<SemIR::FacetType>()) {
-    info = GetConstantFacetTypeInfo(eval_context, facet_type->facet_type_id,
-                                    &phase);
+    info = eval_context.facet_types().Get(facet_type->facet_type_id);
   } else if (base_facet_type_id == SemIR::ErrorInst::TypeId) {
     return SemIR::ErrorInst::ConstantId;
   } else {
@@ -1995,35 +2006,33 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
                  "Unexpected type_id: {0}, inst: {1}", base_facet_type_id,
                  base_facet_inst);
   }
+
+  // Add the constraints from the `WhereExpr` instruction into `info`.
   if (typed_inst.requirements_id.has_value()) {
     auto insts = eval_context.inst_blocks().Get(typed_inst.requirements_id);
     for (auto inst_id : insts) {
       if (auto rewrite =
               eval_context.insts().TryGetAs<SemIR::RequirementRewrite>(
                   inst_id)) {
-        // `where` requirements using `.Self` should not be considered
-        // symbolic.
-        auto lhs_id = GetConstantValueIgnoringPeriodSelf(
-            eval_context, rewrite->lhs_id, &phase);
-        auto rhs_id = GetConstantValueIgnoringPeriodSelf(
-            eval_context, rewrite->rhs_id, &phase);
         info.rewrite_constraints.push_back(
-            {.lhs_id = lhs_id, .rhs_id = rhs_id});
-      } else if (auto impls =
-                     eval_context.insts().TryGetAs<SemIR::RequirementImpls>(
-                         inst_id)) {
-        SemIR::ConstantId lhs = eval_context.GetConstantValue(impls->lhs_id);
-        SemIR::ConstantId rhs = eval_context.GetConstantValue(impls->rhs_id);
-        if (rhs != SemIR::ErrorInst::ConstantId &&
-            IsPeriodSelf(eval_context, lhs)) {
-          auto rhs_inst_id = eval_context.constant_values().GetInstId(rhs);
-          if (rhs_inst_id == SemIR::TypeType::TypeInstId) {
+            {.lhs_id = rewrite->lhs_id, .rhs_id = rewrite->rhs_id});
+        continue;
+      }
+
+      if (auto impls =
+              eval_context.insts().TryGetAs<SemIR::RequirementImpls>(inst_id)) {
+        if (impls->rhs_id != SemIR::ErrorInst::InstId &&
+            IsPeriodSelf(eval_context,
+                         eval_context.constant_values().Get(impls->lhs_id))) {
+          if (impls->rhs_id == SemIR::TypeType::TypeInstId) {
             // `.Self impls type` -> nothing to do.
+            continue;
           } else {
-            auto facet_type =
-                eval_context.insts().GetAs<SemIR::FacetType>(rhs_inst_id);
-            SemIR::FacetTypeInfo more_info = GetConstantFacetTypeInfo(
-                eval_context, facet_type.facet_type_id, &phase);
+            auto facet_type = eval_context.insts().GetAs<SemIR::FacetType>(
+                eval_context.constant_values().GetConstantInstId(
+                    impls->rhs_id));
+            const auto& more_info =
+                eval_context.facet_types().Get(facet_type.facet_type_id);
             // The way to prevent lookup into the interface requirements of a
             // facet type is to put it to the right of a `.Self impls`, which we
             // accomplish by putting them into `self_impls_constraints`.
@@ -2036,19 +2045,22 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
                                more_info.rewrite_constraints);
             info.other_requirements |= more_info.other_requirements;
           }
-        } else {
-          // TODO: Handle `impls` constraints beyond `.Self impls`.
-          info.other_requirements = true;
+          continue;
         }
-      } else {
-        // TODO: Handle other requirements
+
+        // TODO: Handle `impls` constraints beyond `.Self impls`.
         info.other_requirements = true;
+        continue;
       }
+
+      // TODO: Handle other requirements
+      info.other_requirements = true;
     }
   }
-  ResolveRewriteConstraintsAndCanonicalize(eval_context.context(),
-                                           SemIR::LocId(inst_id), info);
-  return MakeFacetTypeResult(eval_context.context(), info, phase);
+
+  auto const_info = GetConstantFacetTypeInfo(
+      eval_context, SemIR::LocId(inst_id), info, &phase);
+  return MakeFacetTypeResult(eval_context.context(), const_info, phase);
 }
 
 // Implementation for `TryEvalInst`, wrapping `Context` with `EvalContext`.

+ 7 - 2
toolchain/check/facet_type.cpp

@@ -402,13 +402,15 @@ class SubstImplWitnessAccessCallbacks : public SubstInstCallbacks {
 
 auto ResolveRewriteConstraintsAndCanonicalize(
     Context& context, SemIR::LocId loc_id,
-    SemIR::FacetTypeInfo& facet_type_info) -> void {
+    SemIR::FacetTypeInfo& facet_type_info) -> bool {
   // This operation sorts and dedupes the rewrite constraints. They are sorted
   // primarily by the `lhs_id`, then by the `rhs_id`.
   facet_type_info.Canonicalize();
 
+  bool success = true;
+
   if (facet_type_info.rewrite_constraints.empty()) {
-    return;
+    return success;
   }
 
   for (size_t i = 0; i < facet_type_info.rewrite_constraints.size() - 1; ++i) {
@@ -452,6 +454,7 @@ auto ResolveRewriteConstraintsAndCanonicalize(
       }
       constraint.rhs_id = SemIR::ErrorInst::InstId;
       next.rhs_id = SemIR::ErrorInst::InstId;
+      success = false;
     }
   }
 
@@ -495,6 +498,8 @@ auto ResolveRewriteConstraintsAndCanonicalize(
   // Canonicalize again, as we may have inserted errors into the rewrite
   // constraints, and these could change sorting order and need to be deduped.
   facet_type_info.Canonicalize();
+
+  return success;
 }
 
 }  // namespace Carbon::Check

+ 3 - 1
toolchain/check/facet_type.h

@@ -68,10 +68,12 @@ auto IsPeriodSelf(Context& context, SemIR::InstId inst_id) -> bool;
 
 // Perform rewrite constraint resolution for a facet type and canonicalize the
 // FacetTypeInfo. The FacetTypeInfo must not be modified after this operation.
+//
+// Returns false if an error was inserted into `facet_type`.
 auto ResolveRewriteConstraintsAndCanonicalize(Context& context,
                                               SemIR::LocId loc_id,
                                               SemIR::FacetTypeInfo& facet_type)
-    -> void;
+    -> bool;
 
 }  // namespace Carbon::Check
 

+ 4 - 22
toolchain/check/testdata/impl/impl_assoc_const.carbon

@@ -120,17 +120,10 @@ library "[[@TEST_NAME]]";
 
 interface L2 { let W2:! type; let X2:! type; }
 
-// CHECK:STDERR: fail_two_different_first_associated.carbon:[[@LINE+11]]:12: error: associated constant `.(L2.W2)` given two different values `()` and `.(L2.X2)` [AssociatedConstantWithDifferentValues]
+// CHECK:STDERR: fail_two_different_first_associated.carbon:[[@LINE+4]]:12: error: associated constant `.(L2.W2)` given two different values `()` and `.(L2.X2)` [AssociatedConstantWithDifferentValues]
 // CHECK:STDERR: impl () as L2 where .W2 = .X2 and .W2 = () {}
 // CHECK:STDERR:            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_two_different_first_associated.carbon:[[@LINE+7]]:1: error: associated constant X2 not given a value in impl of interface L2 [ImplAssociatedConstantNeedsValue]
-// CHECK:STDERR: impl () as L2 where .W2 = .X2 and .W2 = () {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_two_different_first_associated.carbon:[[@LINE-9]]:35: note: associated constant declared here [AssociatedConstantHere]
-// CHECK:STDERR: interface L2 { let W2:! type; let X2:! type; }
-// CHECK:STDERR:                                   ^~~~~~~~~
-// CHECK:STDERR:
 impl () as L2 where .W2 = .X2 and .W2 = () {}
 
 // --- fail_two_different_second_associated.carbon
@@ -138,17 +131,10 @@ library "[[@TEST_NAME]]";
 
 interface L2 { let W2:! type; let X2:! type; }
 
-// CHECK:STDERR: fail_two_different_second_associated.carbon:[[@LINE+11]]:12: error: associated constant `.(L2.W2)` given two different values `()` and `.(L2.X2)` [AssociatedConstantWithDifferentValues]
+// CHECK:STDERR: fail_two_different_second_associated.carbon:[[@LINE+4]]:12: error: associated constant `.(L2.W2)` given two different values `()` and `.(L2.X2)` [AssociatedConstantWithDifferentValues]
 // CHECK:STDERR: impl () as L2 where .W2 = () and .W2 = .X2 {}
 // CHECK:STDERR:            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_two_different_second_associated.carbon:[[@LINE+7]]:1: error: associated constant X2 not given a value in impl of interface L2 [ImplAssociatedConstantNeedsValue]
-// CHECK:STDERR: impl () as L2 where .W2 = () and .W2 = .X2 {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_two_different_second_associated.carbon:[[@LINE-9]]:35: note: associated constant declared here [AssociatedConstantHere]
-// CHECK:STDERR: interface L2 { let W2:! type; let X2:! type; }
-// CHECK:STDERR:                                   ^~~~~~~~~
-// CHECK:STDERR:
 impl () as L2 where .W2 = () and .W2 = .X2 {}
 
 // --- fail_two_different_first_bad.carbon
@@ -349,8 +335,6 @@ impl CD as IF where .F = 0 {
 // CHECK:STDOUT:   %tuple.type.e5a: type = tuple_type (%empty_struct_type, %empty_tuple.type, %empty_tuple.type) [concrete]
 // CHECK:STDOUT:   %tuple.type.d7e: type = tuple_type (%empty_struct_type, %empty_struct_type, %empty_tuple.type) [concrete]
 // CHECK:STDOUT:   %tuple.type.bd8: type = tuple_type (%empty_struct_type, %empty_tuple.type, %empty_struct_type) [concrete]
-// CHECK:STDOUT:   %L_where.type: type = facet_type <@L where %impl.elem0 = <error>> [concrete]
-// CHECK:STDOUT:   %L.impl_witness: <witness> = impl_witness file.%L.impl_witness_table [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -411,20 +395,18 @@ impl CD as IF where .F = 0 {
 // CHECK:STDOUT:     %.loc13_102.3: type = converted %.loc13_97, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type]
 // CHECK:STDOUT:     %.loc13_102.4: type = converted %.loc13_101, constants.%empty_struct_type [concrete = constants.%empty_struct_type]
 // CHECK:STDOUT:     %.loc13_102.5: type = converted %.loc13_102.1, constants.%tuple.type.bd8 [concrete = constants.%tuple.type.bd8]
-// CHECK:STDOUT:     %.loc13_14: type = where_expr %.Self [concrete = constants.%L_where.type] {
+// CHECK:STDOUT:     %.loc13_14: type = where_expr %.Self [concrete = <error>] {
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc13_20, %.loc13_36.5
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc13_42, %.loc13_58.5
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc13_64, %.loc13_80.5
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc13_86, %.loc13_102.5
 // CHECK:STDOUT:     }
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %L.impl_witness_table = impl_witness_table (<error>), @impl [concrete]
-// CHECK:STDOUT:   %L.impl_witness: <witness> = impl_witness %L.impl_witness_table [concrete = constants.%L.impl_witness]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: impl @impl: %.loc13_7.2 as %.loc13_14 {
 // CHECK:STDOUT: !members:
-// CHECK:STDOUT:   witness = file.%L.impl_witness
+// CHECK:STDOUT:   witness = <error>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_cycle.carbon

+ 4 - 8
toolchain/check/testdata/impl/impl_assoc_const_with_prelude.carbon

@@ -298,8 +298,6 @@ impl () as I where .X = {.a = true, .b = (1, 2)} and .X = {.a = false, .b = (3,
 // CHECK:STDOUT:   %int_4.940: %i32 = int_value 4 [concrete]
 // CHECK:STDOUT:   %tuple.ffd: %tuple.type.d07 = tuple_value (%int_3.822, %int_4.940) [concrete]
 // CHECK:STDOUT:   %struct.68c: %struct_type.a.b.fe2 = struct_value (%false, %tuple.ffd) [concrete]
-// CHECK:STDOUT:   %I_where.type: type = facet_type <@I where %impl.elem0 = <error>> [concrete]
-// CHECK:STDOUT:   %I.impl_witness: <witness> = impl_witness file.%I.impl_witness_table [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -324,7 +322,7 @@ impl () as I where .X = {.a = true, .b = (1, 2)} and .X = {.a = false, .b = (3,
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %I.decl: type = interface_decl @I [concrete = constants.%I.type] {} {}
-// CHECK:STDOUT:   impl_decl @impl.fa8 [concrete] {} {
+// CHECK:STDOUT:   impl_decl @impl.de5 [concrete] {} {
 // CHECK:STDOUT:     %.loc10_7.1: %empty_tuple.type = tuple_literal ()
 // CHECK:STDOUT:     %.loc10_7.2: type = converted %.loc10_7.1, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type]
 // CHECK:STDOUT:     %I.ref: type = name_ref I, file.%I.decl [concrete = constants.%I.type]
@@ -385,13 +383,11 @@ impl () as I where .X = {.a = true, .b = (1, 2)} and .X = {.a = false, .b = (3,
 // CHECK:STDOUT:     %.loc10_83.2: %tuple.type.d07 = converted %.loc10_82.1, %tuple.loc10_82 [concrete = constants.%tuple.ffd]
 // CHECK:STDOUT:     %struct.loc10_83: %struct_type.a.b.fe2 = struct_value (%false, %.loc10_83.2) [concrete = constants.%struct.68c]
 // CHECK:STDOUT:     %.loc10_83.3: %struct_type.a.b.fe2 = converted %.loc10_83.1, %struct.loc10_83 [concrete = constants.%struct.68c]
-// CHECK:STDOUT:     %.loc10_14: type = where_expr %.Self [concrete = constants.%I_where.type] {
+// CHECK:STDOUT:     %.loc10_14: type = where_expr %.Self [concrete = <error>] {
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc10_20, %.loc10_48.3
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc10_54, %.loc10_83.3
 // CHECK:STDOUT:     }
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %I.impl_witness_table = impl_witness_table (<error>), @impl.fa8 [concrete]
-// CHECK:STDOUT:   %I.impl_witness: <witness> = impl_witness %I.impl_witness_table [concrete = constants.%I.impl_witness]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: interface @I {
@@ -410,9 +406,9 @@ impl () as I where .X = {.a = true, .b = (1, 2)} and .X = {.a = false, .b = (3,
 // CHECK:STDOUT:   assoc_const X:! %struct_type.a.b.fe2;
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: impl @impl.fa8: %.loc10_7.2 as %.loc10_14 {
+// CHECK:STDOUT: impl @impl.de5: %.loc10_7.2 as %.loc10_14 {
 // CHECK:STDOUT: !members:
-// CHECK:STDOUT:   witness = file.%I.impl_witness
+// CHECK:STDOUT:   witness = <error>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @X(constants.%Self.826) {}

+ 3 - 7
toolchain/check/testdata/impl/import_interface_assoc_const.carbon

@@ -993,8 +993,6 @@ impl CD as IF where .F = 0 {
 // CHECK:STDOUT:   %I.facet: %I.type = facet_value %.Self.as_type, (%I.lookup_impl_witness) [symbolic_self]
 // CHECK:STDOUT:   %impl.elem0: type = impl_witness_access %I.lookup_impl_witness, element0 [symbolic_self]
 // CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
-// CHECK:STDOUT:   %I_where.type: type = facet_type <@I where %impl.elem0 = <error>> [concrete]
-// CHECK:STDOUT:   %I.impl_witness: <witness> = impl_witness file.%I.impl_witness_table [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -1003,7 +1001,7 @@ impl CD as IF where .F = 0 {
 // CHECK:STDOUT:   %Main.NonType = import_ref Main//interface, NonType, unloaded
 // CHECK:STDOUT:   %Main.import_ref.e5d = import_ref Main//interface, inst17 [no loc], unloaded
 // CHECK:STDOUT:   %Main.import_ref.4fb: %I.assoc_type = import_ref Main//interface, loc3_20, loaded [concrete = constants.%assoc0]
-// CHECK:STDOUT:   %Main.T: type = import_ref Main//interface, T, loaded [concrete = %T]
+// CHECK:STDOUT:   %Main.T = import_ref Main//interface, T, unloaded
 // CHECK:STDOUT:   %Main.import_ref.652: type = import_ref Main//interface, loc3_20, loaded [concrete = %T]
 // CHECK:STDOUT:   %T: type = assoc_const_decl @T [concrete] {}
 // CHECK:STDOUT:   %Main.import_ref.5dd: %I.type = import_ref Main//interface, inst17 [no loc], loaded [symbolic = constants.%Self]
@@ -1036,13 +1034,11 @@ impl CD as IF where .F = 0 {
 // CHECK:STDOUT:     %impl.elem0.loc10_32: type = impl_witness_access constants.%I.lookup_impl_witness, element0 [symbolic_self = constants.%impl.elem0]
 // CHECK:STDOUT:     %.loc10_38.1: %empty_tuple.type = tuple_literal ()
 // CHECK:STDOUT:     %.loc10_38.2: type = converted %.loc10_38.1, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type]
-// CHECK:STDOUT:     %.loc10_14: type = where_expr %.Self [concrete = constants.%I_where.type] {
+// CHECK:STDOUT:     %.loc10_14: type = where_expr %.Self [concrete = <error>] {
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc10_20, %.loc10_26.2
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc10_32, %.loc10_38.2
 // CHECK:STDOUT:     }
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %I.impl_witness_table = impl_witness_table (<error>), @impl [concrete]
-// CHECK:STDOUT:   %I.impl_witness: <witness> = impl_witness %I.impl_witness_table [concrete = constants.%I.impl_witness]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: interface @I [from "interface.carbon"] {
@@ -1058,7 +1054,7 @@ impl CD as IF where .F = 0 {
 // CHECK:STDOUT:
 // CHECK:STDOUT: impl @impl: %C7.ref as %.loc10_14 {
 // CHECK:STDOUT: !members:
-// CHECK:STDOUT:   witness = file.%I.impl_witness
+// CHECK:STDOUT:   witness = <error>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C7 {

+ 43 - 0
toolchain/check/testdata/impl/lookup/import_error.carbon

@@ -0,0 +1,43 @@
+// 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
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/facet_types.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/lookup/import_error.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/lookup/import_error.carbon
+
+// --- fail_error_interface_z.carbon
+library "[[@TEST_NAME]]";
+
+interface Z {
+  let X:! type;
+}
+
+// The value of .X is an error.
+// CHECK:STDERR: fail_error_interface_z.carbon:[[@LINE+4]]:35: error: associated constant `.(Z.X)` given two different values `{}` and `()` [AssociatedConstantWithDifferentValues]
+// CHECK:STDERR: final impl forall [T:! type] T as Z where .X = {} and .X = () {}
+// CHECK:STDERR:                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+final impl forall [T:! type] T as Z where .X = {} and .X = () {}
+
+// --- fail_import_error.carbon
+library "[[@TEST_NAME]]";
+
+import library "error_interface_z";
+
+fn F[U:! type](T:! Z) {
+  // The value of `.X` is an error. It should not crash though.
+  //
+  // CHECK:STDERR: fail_import_error.carbon:[[@LINE+7]]:16: error: cannot implicitly convert expression of type `()` to `T.(Z.X)` [ConversionFailure]
+  // CHECK:STDERR:   let a: T.X = ();
+  // CHECK:STDERR:                ^~
+  // CHECK:STDERR: fail_import_error.carbon:[[@LINE+4]]:16: note: type `()` does not implement interface `Core.ImplicitAs(T.(Z.X))` [MissingImplInMemberAccessNote]
+  // CHECK:STDERR:   let a: T.X = ();
+  // CHECK:STDERR:                ^~
+  // CHECK:STDERR:
+  let a: T.X = ();
+}