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

Completing a type no longer ignores facet types (#5004)

Make facet types complete like other types. This means that in the body
of an interface, the type of `Self` is incomplete. This involved fixing
an issue where eval of a specific_id that was already canonical was not
resolving the specific declaration, which could occur as part of
substituting into a facet type.

---------

Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Dana Jansens <danakj@orodu.net>
josh11b 1 год назад
Родитель
Сommit
3ebd098597
25 измененных файлов с 184 добавлено и 145 удалено
  1. 18 0
      toolchain/check/eval.cpp
  2. 2 4
      toolchain/check/generic.cpp
  3. 6 0
      toolchain/check/generic.h
  4. 7 1
      toolchain/check/handle_impl.cpp
  5. 7 1
      toolchain/check/name_lookup.cpp
  6. 1 1
      toolchain/check/testdata/builtin_conversions/no_prelude/convert_class_value_to_generic_facet_value_value.carbon
  7. 12 0
      toolchain/check/testdata/builtin_conversions/no_prelude/fail_convert_class_type_to_generic_facet_value.carbon
  8. 8 0
      toolchain/check/testdata/builtin_conversions/no_prelude/fail_todo_convert_facet_value_value_to_generic_facet_value_value.carbon
  9. 1 1
      toolchain/check/testdata/function/generic/call_method_on_generic_facet.carbon
  10. 1 1
      toolchain/check/testdata/impl/extend_impl_generic.carbon
  11. 2 2
      toolchain/check/testdata/impl/fail_extend_partially_defined_interface.carbon
  12. 4 4
      toolchain/check/testdata/impl/fail_extend_undefined_interface.carbon
  13. 26 32
      toolchain/check/testdata/impl/fail_self_type_mismatch.carbon
  14. 2 2
      toolchain/check/testdata/impl/lookup/no_prelude/impl_forall.carbon
  15. 2 2
      toolchain/check/testdata/impl/no_prelude/fail_extend_impl_scope.carbon
  16. 2 2
      toolchain/check/testdata/impl/no_prelude/fail_impl_as_scope.carbon
  17. 4 4
      toolchain/check/testdata/impl/no_prelude/fail_undefined_interface.carbon
  18. 18 8
      toolchain/check/testdata/interface/no_prelude/assoc_const_in_generic.carbon
  19. 2 2
      toolchain/check/testdata/interface/no_prelude/fail_lookup_undefined.carbon
  20. 4 0
      toolchain/check/testdata/interface/no_prelude/generic.carbon
  21. 1 1
      toolchain/check/testdata/where_expr/dot_self_index.carbon
  22. 49 57
      toolchain/check/type_completion.cpp
  23. 3 7
      toolchain/check/type_completion.h
  24. 2 1
      toolchain/diagnostics/diagnostic_kind.def
  25. 0 12
      toolchain/lower/testdata/interface/basic.carbon

+ 18 - 0
toolchain/check/eval.cpp

@@ -452,6 +452,24 @@ static auto GetConstantValue(EvalContext& eval_context,
   }
 
   if (args_id == specific.args_id) {
+    const auto& specific = eval_context.specifics().Get(specific_id);
+    // A constant specific_id should always have a resolved declaration. The
+    // specific_id from the instruction may coincidentally be canonical, and so
+    // constant evaluation gives the same value. In that case, we still need to
+    // ensure its declaration is resolved.
+    //
+    // However, don't resolve the declaration if the generic's eval block hasn't
+    // been set yet. This happens when building the eval block during import.
+    //
+    // TODO: Change importing of generic eval blocks to be less fragile and
+    // remove this `if` so we unconditionally call `ResolveSpecificDeclaration`.
+    if (!specific.decl_block_id.has_value() && eval_context.context()
+                                                   .generics()
+                                                   .Get(specific.generic_id)
+                                                   .decl_block_id.has_value()) {
+      ResolveSpecificDeclaration(eval_context.context(),
+                                 eval_context.fallback_loc(), specific_id);
+    }
     return specific_id;
   }
   return MakeSpecific(eval_context.context(), eval_context.fallback_loc(),

+ 2 - 4
toolchain/check/generic.cpp

@@ -21,8 +21,6 @@ namespace Carbon::Check {
 
 static auto MakeSelfSpecificId(Context& context, SemIR::GenericId generic_id)
     -> SemIR::SpecificId;
-static auto ResolveSpecificDeclaration(Context& context, SemIRLoc loc,
-                                       SemIR::SpecificId specific_id) -> void;
 
 auto StartGenericDecl(Context& context) -> void {
   context.generic_region_stack().Push();
@@ -414,8 +412,8 @@ auto FinishGenericDefinition(Context& context, SemIR::GenericId generic_id)
   context.generic_region_stack().Pop();
 }
 
-static auto ResolveSpecificDeclaration(Context& context, SemIRLoc loc,
-                                       SemIR::SpecificId specific_id) -> void {
+auto ResolveSpecificDeclaration(Context& context, SemIRLoc loc,
+                                SemIR::SpecificId specific_id) -> void {
   // If this is the first time we've formed this specific, evaluate its decl
   // block to form information about the specific.
   if (!context.specifics().Get(specific_id).decl_block_id.has_value()) {

+ 6 - 0
toolchain/check/generic.h

@@ -70,6 +70,12 @@ auto MakeSpecific(Context& context, SemIRLoc loc, SemIR::GenericId generic_id,
 auto MakeSelfSpecific(Context& context, SemIRLoc loc,
                       SemIR::GenericId generic_id) -> SemIR::SpecificId;
 
+// Resolve the declaration of the given specific, by evaluating the eval block
+// of the corresponding generic and storing a corresponding value block in the
+// specific.
+auto ResolveSpecificDeclaration(Context& context, SemIRLoc loc,
+                                SemIR::SpecificId specific_id) -> void;
+
 // Attempts to resolve the definition of the given specific, by evaluating the
 // eval block of the corresponding generic and storing a corresponding value
 // block in the specific. Returns false if a definition is not available.

+ 7 - 1
toolchain/check/handle_impl.cpp

@@ -322,7 +322,13 @@ static auto CheckConstraintIsInterface(Context& context,
 
   auto complete_id = RequireCompleteFacetType(
       context, facet_type_id, context.insts().GetLocId(impl.constraint_id),
-      *facet_type, FacetTypeImpl);
+      *facet_type, [&] {
+        CARBON_DIAGNOSTIC(ImplAsIncompleteFacetType, Error,
+                          "impl as incomplete facet type {0}", InstIdAsType);
+        return context.emitter().Build(impl.latest_decl_id(),
+                                       ImplAsIncompleteFacetType,
+                                       impl.constraint_id);
+      });
   if (!complete_id.has_value()) {
     return nullptr;
   }

+ 7 - 1
toolchain/check/name_lookup.cpp

@@ -258,7 +258,13 @@ auto AppendLookupScopesForConstant(Context& context, SemIR::LocId loc_id,
   if (auto base_as_facet_type = base.TryAs<SemIR::FacetType>()) {
     auto complete_id = RequireCompleteFacetType(
         context, context.types().GetTypeIdForTypeConstantId(base_const_id),
-        loc_id, *base_as_facet_type, FacetTypeMemberAccess);
+        loc_id, *base_as_facet_type, [&] {
+          CARBON_DIAGNOSTIC(QualifiedExprInIncompleteFacetTypeScope, Error,
+                            "member access into incomplete facet type {0}",
+                            InstIdAsType);
+          return context.emitter().Build(
+              loc_id, QualifiedExprInIncompleteFacetTypeScope, base_id);
+        });
     if (complete_id.has_value()) {
       const auto& resolved = context.complete_facet_types().Get(complete_id);
       for (const auto& interface : resolved.required_interfaces) {

+ 1 - 1
toolchain/check/testdata/builtin_conversions/no_prelude/convert_class_value_to_generic_facet_value_value.carbon

@@ -174,11 +174,11 @@ fn G() {
 // CHECK:STDOUT:   %CallGenericMethod2: %CallGenericMethod2.type = struct_value () [concrete]
 // CHECK:STDOUT:   %require_complete.7b2: <witness> = require_complete_type %U.as_type [symbolic]
 // CHECK:STDOUT:   %require_complete.4ae: <witness> = require_complete_type %T [symbolic]
-// CHECK:STDOUT:   %require_complete.02a: <witness> = require_complete_type %Generic.type.91ccba.2 [symbolic]
 // CHECK:STDOUT:   %F.type.f439a9.2: type = fn_type @F.1, @Generic(%T) [symbolic]
 // CHECK:STDOUT:   %F.8a2d67.2: %F.type.f439a9.2 = struct_value () [symbolic]
 // CHECK:STDOUT:   %Generic.assoc_type.de973d.2: type = assoc_entity_type %Generic.type.91ccba.2 [symbolic]
 // CHECK:STDOUT:   %assoc0.29ce53.2: %Generic.assoc_type.de973d.2 = assoc_entity element0, @Generic.%F.decl [symbolic]
+// CHECK:STDOUT:   %require_complete.02a: <witness> = require_complete_type %Generic.type.91ccba.2 [symbolic]
 // CHECK:STDOUT:   %U.as_wit: <witness> = facet_access_witness %U [symbolic]
 // CHECK:STDOUT:   %Generic.facet.2ea: %Generic.type.91ccba.2 = facet_value %U.as_type, %U.as_wit [symbolic]
 // CHECK:STDOUT:   %.da8: type = fn_type_with_self_type %F.type.f439a9.2, %Generic.facet.2ea [symbolic]

+ 12 - 0
toolchain/check/testdata/builtin_conversions/no_prelude/fail_convert_class_type_to_generic_facet_value.carbon

@@ -182,6 +182,10 @@ fn G() {
 // CHECK:STDOUT:   %G.type: type = fn_type @G [concrete]
 // CHECK:STDOUT:   %G: %G.type = struct_value () [concrete]
 // CHECK:STDOUT:   %Generic.type.c3b: type = facet_type <@Generic, @Generic(%WrongGenericParam)> [concrete]
+// CHECK:STDOUT:   %F.type.96d: type = fn_type @F.1, @Generic(%WrongGenericParam) [concrete]
+// CHECK:STDOUT:   %F.86b: %F.type.96d = struct_value () [concrete]
+// CHECK:STDOUT:   %Generic.assoc_type.967: type = assoc_entity_type %Generic.type.c3b [concrete]
+// CHECK:STDOUT:   %assoc0.c10: %Generic.assoc_type.967 = assoc_entity element0, @Generic.%F.decl [concrete]
 // CHECK:STDOUT:   %Dest: type = bind_symbolic_name Dest, 0 [symbolic]
 // CHECK:STDOUT:   %ImplicitAs.type.d62: type = facet_type <@ImplicitAs, @ImplicitAs(%Dest)> [symbolic]
 // CHECK:STDOUT:   %Self.519: %ImplicitAs.type.d62 = bind_symbolic_name Self, 1 [symbolic]
@@ -422,6 +426,14 @@ fn G() {
 // CHECK:STDOUT: specific @Generic(constants.%WrongGenericParam) {
 // CHECK:STDOUT:   %Scalar.loc4_19.2 => constants.%WrongGenericParam
 // CHECK:STDOUT:   %Scalar.patt.loc4_19.2 => constants.%WrongGenericParam
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %Generic.type => constants.%Generic.type.c3b
+// CHECK:STDOUT:   %Self.2 => constants.%Self.dee
+// CHECK:STDOUT:   %F.type => constants.%F.type.96d
+// CHECK:STDOUT:   %F => constants.%F.86b
+// CHECK:STDOUT:   %Generic.assoc_type => constants.%Generic.assoc_type.967
+// CHECK:STDOUT:   %assoc0.loc5_9.2 => constants.%assoc0.c10
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @ImplicitAs(constants.%Dest) {

+ 8 - 0
toolchain/check/testdata/builtin_conversions/no_prelude/fail_todo_convert_facet_value_value_to_generic_facet_value_value.carbon

@@ -590,6 +590,10 @@ fn F() {
 // CHECK:STDOUT: specific @Eats(constants.%Food.as_type.fae) {
 // CHECK:STDOUT:   %Food.loc12_16.2 => constants.%Food.as_type.fae
 // CHECK:STDOUT:   %Food.patt.loc12_16.2 => constants.%Food.as_type.fae
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %Eats.type => constants.%Eats.type.f54c3d.2
+// CHECK:STDOUT:   %Self.2 => constants.%Self.4eb
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @impl.2(constants.%T.fd4, constants.%Food.5fe) {
@@ -657,6 +661,10 @@ fn F() {
 // CHECK:STDOUT: specific @Eats(constants.%Grass) {
 // CHECK:STDOUT:   %Food.loc12_16.2 => constants.%Grass
 // CHECK:STDOUT:   %Food.patt.loc12_16.2 => constants.%Grass
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %Eats.type => constants.%Eats.type.1ae
+// CHECK:STDOUT:   %Self.2 => constants.%Self.4eb
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @impl.2(constants.%Animal.facet, constants.%Edible.facet) {

+ 1 - 1
toolchain/check/testdata/function/generic/call_method_on_generic_facet.carbon

@@ -78,11 +78,11 @@ fn G() {
 // CHECK:STDOUT:   %U.patt: %Generic.type.91ccba.2 = symbolic_binding_pattern U, 1 [symbolic]
 // CHECK:STDOUT:   %CallGenericMethod.type: type = fn_type @CallGenericMethod [concrete]
 // CHECK:STDOUT:   %CallGenericMethod: %CallGenericMethod.type = struct_value () [concrete]
-// CHECK:STDOUT:   %require_complete: <witness> = require_complete_type %Generic.type.91ccba.2 [symbolic]
 // CHECK:STDOUT:   %F.type.f439a9.2: type = fn_type @F.1, @Generic(%T) [symbolic]
 // CHECK:STDOUT:   %F.8a2d67.2: %F.type.f439a9.2 = struct_value () [symbolic]
 // CHECK:STDOUT:   %Generic.assoc_type.de973d.2: type = assoc_entity_type %Generic.type.91ccba.2 [symbolic]
 // CHECK:STDOUT:   %assoc0.29ce53.2: %Generic.assoc_type.de973d.2 = assoc_entity element0, @Generic.%F.decl [symbolic]
+// CHECK:STDOUT:   %require_complete: <witness> = require_complete_type %Generic.type.91ccba.2 [symbolic]
 // CHECK:STDOUT:   %U.as_type: type = facet_access_type %U [symbolic]
 // CHECK:STDOUT:   %U.as_wit: <witness> = facet_access_witness %U [symbolic]
 // CHECK:STDOUT:   %Generic.facet.2ea: %Generic.type.91ccba.2 = facet_value %U.as_type, %U.as_wit [symbolic]

+ 1 - 1
toolchain/check/testdata/impl/extend_impl_generic.carbon

@@ -328,11 +328,11 @@ class X(U:! type) {
 // CHECK:STDOUT:   %X.generic: %X.type = struct_value () [concrete]
 // CHECK:STDOUT:   %X: type = class_type @X, @X(%U) [symbolic]
 // CHECK:STDOUT:   %I.type.325e65.2: type = facet_type <@I, @I(%U)> [symbolic]
-// CHECK:STDOUT:   %require_complete.cfe: <witness> = require_complete_type %I.type.325e65.2 [symbolic]
 // CHECK:STDOUT:   %F.type.2aef59.2: type = fn_type @F.1, @I(%U) [symbolic]
 // CHECK:STDOUT:   %F.bb2dd4.2: %F.type.2aef59.2 = struct_value () [symbolic]
 // CHECK:STDOUT:   %I.assoc_type.955255.2: type = assoc_entity_type %I.type.325e65.2 [symbolic]
 // CHECK:STDOUT:   %assoc0.fef501.2: %I.assoc_type.955255.2 = assoc_entity element0, @I.%F.decl [symbolic]
+// CHECK:STDOUT:   %require_complete.cfe: <witness> = require_complete_type %I.type.325e65.2 [symbolic]
 // CHECK:STDOUT:   %impl_witness: <witness> = impl_witness (@impl.%F.decl), @impl(%U) [symbolic]
 // CHECK:STDOUT:   %F.type.e88: type = fn_type @F.2, @impl(%U) [symbolic]
 // CHECK:STDOUT:   %F.b02: %F.type.e88 = struct_value () [symbolic]

+ 2 - 2
toolchain/check/testdata/impl/fail_extend_partially_defined_interface.carbon

@@ -10,9 +10,9 @@
 
 interface I {
   class C {
-    // CHECK:STDERR: fail_extend_partially_defined_interface.carbon:[[@LINE+11]]:20: error: impl of undefined interface I [ResolveFacetTypeWithUndefinedInterface]
+    // CHECK:STDERR: fail_extend_partially_defined_interface.carbon:[[@LINE+11]]:5: error: impl as incomplete facet type `I` [ImplAsIncompleteFacetType]
     // CHECK:STDERR:     extend impl as I;
-    // CHECK:STDERR:                    ^
+    // CHECK:STDERR:     ^~~~~~~~~~~~~~~~~
     // CHECK:STDERR: fail_extend_partially_defined_interface.carbon:[[@LINE-5]]:1: note: interface is currently being defined [InterfaceUndefinedWithinDefinition]
     // CHECK:STDERR: interface I {
     // CHECK:STDERR: ^~~~~~~~~~~~~

+ 4 - 4
toolchain/check/testdata/impl/fail_extend_undefined_interface.carbon

@@ -11,9 +11,9 @@
 interface I;
 
 class C {
-  // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+7]]:18: error: impl of undefined interface I [ResolveFacetTypeWithUndefinedInterface]
+  // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+7]]:3: error: impl as incomplete facet type `I` [ImplAsIncompleteFacetType]
   // CHECK:STDERR:   extend impl as I;
-  // CHECK:STDERR:                  ^
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~
   // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-6]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
   // CHECK:STDERR: interface I;
   // CHECK:STDERR: ^~~~~~~~~~~~
@@ -22,7 +22,7 @@ class C {
 }
 
 fn F(c: C) {
-  // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+10]]:3: error: member access into undefined interface I [ResolveFacetTypeWithUndefinedInterface]
+  // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+10]]:3: error: member access into incomplete facet type `I` [QualifiedExprInIncompleteFacetTypeScope]
   // CHECK:STDERR:   C.F();
   // CHECK:STDERR:   ^~~
   // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-17]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
@@ -33,7 +33,7 @@ fn F(c: C) {
   // CHECK:STDERR:                  ^
   // CHECK:STDERR:
   C.F();
-  // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+14]]:3: error: member access into undefined interface I [ResolveFacetTypeWithUndefinedInterface]
+  // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE+14]]:3: error: member access into incomplete facet type `I` [QualifiedExprInIncompleteFacetTypeScope]
   // CHECK:STDERR:   c.F();
   // CHECK:STDERR:   ^~~
   // CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-28]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]

+ 26 - 32
toolchain/check/testdata/impl/fail_self_type_mismatch.carbon

@@ -11,17 +11,25 @@
 class C[T:! type](X:! T) {}
 
 interface I {
+  // This tests that inside the definition of `I`, `Self` has type `I`.
+
+  // Below, `C(Self)` deduces `T` to be the incomplete facet type `I`, which
+  // leads to the error.
+
+  // CHECK:STDERR: fail_self_type_mismatch.carbon:[[@LINE+10]]:11: error: forming value of incomplete type `I` [IncompleteTypeInValueConversion]
+  // CHECK:STDERR:   fn F(c: C(Self));
+  // CHECK:STDERR:           ^~~~~~~
+  // CHECK:STDERR: fail_self_type_mismatch.carbon:[[@LINE-9]]:1: note: interface is currently being defined [InterfaceUndefinedWithinDefinition]
+  // CHECK:STDERR: interface I {
+  // CHECK:STDERR: ^~~~~~~~~~~~~
+  // CHECK:STDERR: fail_self_type_mismatch.carbon:[[@LINE-14]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere]
+  // CHECK:STDERR: class C[T:! type](X:! T) {}
+  // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
   fn F(c: C(Self));
 }
 
 impl i32 as I {
-  // CHECK:STDERR: fail_self_type_mismatch.carbon:[[@LINE+7]]:8: error: type `C(i32)` of parameter 1 in redeclaration differs from previous parameter type `C(i32 as I)` [RedeclParamDiffersType]
-  // CHECK:STDERR:   fn F(c: C(i32));
-  // CHECK:STDERR:        ^~~~~~~~~
-  // CHECK:STDERR: fail_self_type_mismatch.carbon:[[@LINE-7]]:8: note: previous declaration's corresponding parameter here [RedeclParamPrevious]
-  // CHECK:STDERR:   fn F(c: C(Self));
-  // CHECK:STDERR:        ^~~~~~~~~~
-  // CHECK:STDERR:
   fn F(c: C(i32));
 }
 
@@ -39,7 +47,6 @@ impl i32 as I {
 // CHECK:STDOUT:   %complete_type.357: <witness> = complete_type_witness %empty_struct_type [concrete]
 // CHECK:STDOUT:   %I.type: type = facet_type <@I> [concrete]
 // CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic]
-// CHECK:STDOUT:   %C.dbb: type = class_type @C, @C(%I.type, %Self) [symbolic]
 // CHECK:STDOUT:   %F.type.cf0: type = fn_type @F.1 [concrete]
 // CHECK:STDOUT:   %F.bc6: %F.type.cf0 = struct_value () [concrete]
 // CHECK:STDOUT:   %I.assoc_type: type = assoc_entity_type %I.type [concrete]
@@ -51,7 +58,6 @@ impl i32 as I {
 // CHECK:STDOUT:   %F.type.066: type = fn_type @F.2 [concrete]
 // CHECK:STDOUT:   %F.9ec: %F.type.066 = struct_value () [concrete]
 // CHECK:STDOUT:   %I.facet: %I.type = facet_value %i32, %impl_witness [concrete]
-// CHECK:STDOUT:   %C.d7a: type = class_type @C, @C(%I.type, %I.facet) [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -93,16 +99,16 @@ impl i32 as I {
 // CHECK:STDOUT: interface @I {
 // CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
 // CHECK:STDOUT:   %F.decl: %F.type.cf0 = fn_decl @F.1 [concrete = constants.%F.bc6] {
-// CHECK:STDOUT:     %c.patt: @F.1.%C.loc14_17.1 (%C.dbb) = binding_pattern c
-// CHECK:STDOUT:     %c.param_patt: @F.1.%C.loc14_17.1 (%C.dbb) = value_param_pattern %c.patt, runtime_param0
+// CHECK:STDOUT:     %c.patt: <error> = binding_pattern c
+// CHECK:STDOUT:     %c.param_patt: <error> = value_param_pattern %c.patt, runtime_param0
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %c.param: @F.1.%C.loc14_17.1 (%C.dbb) = value_param runtime_param0
-// CHECK:STDOUT:     %.loc14: type = splice_block %C.loc14_17.2 [symbolic = %C.loc14_17.1 (constants.%C.dbb)] {
+// CHECK:STDOUT:     %c.param: <error> = value_param runtime_param0
+// CHECK:STDOUT:     %.loc29: type = splice_block %C [concrete = <error>] {
 // CHECK:STDOUT:       %C.ref: %C.type = name_ref C, file.%C.decl [concrete = constants.%C.generic]
 // CHECK:STDOUT:       %Self.ref: %I.type = name_ref Self, @I.%Self [symbolic = %Self (constants.%Self)]
-// CHECK:STDOUT:       %C.loc14_17.2: type = class_type @C, @C(constants.%I.type, constants.%Self) [symbolic = %C.loc14_17.1 (constants.%C.dbb)]
+// CHECK:STDOUT:       %C: type = class_type @C, @C(constants.%I.type, <error>) [concrete = <error>]
 // CHECK:STDOUT:     }
-// CHECK:STDOUT:     %c: @F.1.%C.loc14_17.1 (%C.dbb) = bind_name c, %c.param
+// CHECK:STDOUT:     %c: <error> = bind_name c, %c.param
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %assoc0: %I.assoc_type = assoc_entity element0, %F.decl [concrete = constants.%assoc0]
 // CHECK:STDOUT:
@@ -119,7 +125,7 @@ impl i32 as I {
 // CHECK:STDOUT:     %c.param_patt: %C.6fb = value_param_pattern %c.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %c.param: %C.6fb = value_param runtime_param0
-// CHECK:STDOUT:     %.loc25: type = splice_block %C [concrete = constants.%C.6fb] {
+// CHECK:STDOUT:     %.loc33: type = splice_block %C [concrete = constants.%C.6fb] {
 // CHECK:STDOUT:       %C.ref: %C.type = name_ref C, file.%C.decl [concrete = constants.%C.generic]
 // CHECK:STDOUT:       %int_32: Core.IntLiteral = int_value 32 [concrete = constants.%int_32]
 // CHECK:STDOUT:       %i32: type = class_type @Int, @Int(constants.%int_32) [concrete = constants.%i32]
@@ -153,9 +159,8 @@ impl i32 as I {
 // CHECK:STDOUT:
 // CHECK:STDOUT: generic fn @F.1(@I.%Self: %I.type) {
 // CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic = %Self (constants.%Self)]
-// CHECK:STDOUT:   %C.loc14_17.1: type = class_type @C, @C(constants.%I.type, %Self) [symbolic = %C.loc14_17.1 (constants.%C.dbb)]
 // CHECK:STDOUT:
-// CHECK:STDOUT:   fn(%c.param_patt: @F.1.%C.loc14_17.1 (%C.dbb));
+// CHECK:STDOUT:   fn(%c.param_patt: <error>);
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.2(%c.param_patt: %C.6fb);
@@ -167,20 +172,17 @@ impl i32 as I {
 // CHECK:STDOUT:   %X.patt.loc11_19.2 => constants.%X
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @C(constants.%I.type, constants.%Self) {
+// CHECK:STDOUT: specific @C(constants.%I.type, <error>) {
 // CHECK:STDOUT:   %T.loc11_9.2 => constants.%I.type
 // CHECK:STDOUT:   %T.patt.loc11_9.2 => constants.%I.type
-// CHECK:STDOUT:   %X.loc11_19.2 => constants.%Self
-// CHECK:STDOUT:   %X.patt.loc11_19.2 => constants.%Self
+// CHECK:STDOUT:   %X.loc11_19.2 => <error>
+// CHECK:STDOUT:   %X.patt.loc11_19.2 => <error>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @F.1(constants.%Self) {
 // CHECK:STDOUT:   %Self => constants.%Self
-// CHECK:STDOUT:   %C.loc14_17.1 => constants.%C.dbb
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @C(constants.%I.type, @F.1.%Self) {}
-// CHECK:STDOUT:
 // CHECK:STDOUT: specific @C(type, constants.%i32) {
 // CHECK:STDOUT:   %T.loc11_9.2 => type
 // CHECK:STDOUT:   %T.patt.loc11_9.2 => type
@@ -190,13 +192,5 @@ impl i32 as I {
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @F.1(constants.%I.facet) {
 // CHECK:STDOUT:   %Self => constants.%I.facet
-// CHECK:STDOUT:   %C.loc14_17.1 => constants.%C.d7a
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: specific @C(constants.%I.type, constants.%I.facet) {
-// CHECK:STDOUT:   %T.loc11_9.2 => constants.%I.type
-// CHECK:STDOUT:   %T.patt.loc11_9.2 => constants.%I.type
-// CHECK:STDOUT:   %X.loc11_19.2 => constants.%I.facet
-// CHECK:STDOUT:   %X.patt.loc11_19.2 => constants.%I.facet
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 2 - 2
toolchain/check/testdata/impl/lookup/no_prelude/impl_forall.carbon

@@ -59,11 +59,11 @@ fn TestSpecific(a: A({})) -> {} {
 // CHECK:STDOUT:   %V.patt: type = symbolic_binding_pattern V, 0 [symbolic]
 // CHECK:STDOUT:   %A.13025a.2: type = class_type @A, @A(%V) [symbolic]
 // CHECK:STDOUT:   %I.type.325e65.2: type = facet_type <@I, @I(%V)> [symbolic]
-// CHECK:STDOUT:   %require_complete.cfebb2.1: <witness> = require_complete_type %I.type.325e65.2 [symbolic]
 // CHECK:STDOUT:   %F.type.2aef59.2: type = fn_type @F.1, @I(%V) [symbolic]
 // CHECK:STDOUT:   %F.bb2dd4.2: %F.type.2aef59.2 = struct_value () [symbolic]
 // CHECK:STDOUT:   %I.assoc_type.955255.2: type = assoc_entity_type %I.type.325e65.2 [symbolic]
 // CHECK:STDOUT:   %assoc0.fef501.2: %I.assoc_type.955255.2 = assoc_entity element0, @I.%F.decl [symbolic]
+// CHECK:STDOUT:   %require_complete.cfebb2.1: <witness> = require_complete_type %I.type.325e65.2 [symbolic]
 // CHECK:STDOUT:   %impl_witness.ab3b51.1: <witness> = impl_witness (@impl.%F.decl), @impl(%V) [symbolic]
 // CHECK:STDOUT:   %F.type.0fea45.1: type = fn_type @F.2, @impl(%V) [symbolic]
 // CHECK:STDOUT:   %F.d6ae34.1: %F.type.0fea45.1 = struct_value () [symbolic]
@@ -84,11 +84,11 @@ fn TestSpecific(a: A({})) -> {} {
 // CHECK:STDOUT:   %complete_type.84bb3d.3: <witness> = complete_type_witness %struct_type.n.848971.3 [symbolic]
 // CHECK:STDOUT:   %require_complete.5b8aee.2: <witness> = require_complete_type %A.13025a.3 [symbolic]
 // CHECK:STDOUT:   %I.type.325e65.3: type = facet_type <@I, @I(%W)> [symbolic]
-// CHECK:STDOUT:   %require_complete.cfebb2.2: <witness> = require_complete_type %I.type.325e65.3 [symbolic]
 // CHECK:STDOUT:   %F.type.2aef59.3: type = fn_type @F.1, @I(%W) [symbolic]
 // CHECK:STDOUT:   %F.bb2dd4.3: %F.type.2aef59.3 = struct_value () [symbolic]
 // CHECK:STDOUT:   %I.assoc_type.955255.3: type = assoc_entity_type %I.type.325e65.3 [symbolic]
 // CHECK:STDOUT:   %assoc0.fef501.3: %I.assoc_type.955255.3 = assoc_entity element0, @I.%F.decl [symbolic]
+// CHECK:STDOUT:   %require_complete.cfebb2.2: <witness> = require_complete_type %I.type.325e65.3 [symbolic]
 // CHECK:STDOUT:   %impl_witness.ab3b51.2: <witness> = impl_witness (@impl.%F.decl), @impl(%W) [symbolic]
 // CHECK:STDOUT:   %F.type.0fea45.2: type = fn_type @F.2, @impl(%W) [symbolic]
 // CHECK:STDOUT:   %F.d6ae34.2: %F.type.0fea45.2 = struct_value () [symbolic]

+ 2 - 2
toolchain/check/testdata/impl/no_prelude/fail_extend_impl_scope.carbon

@@ -42,9 +42,9 @@ interface Z {
   // CHECK:STDERR:   extend impl as Z {
   // CHECK:STDERR:               ^~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_extend_impl_self_interface.carbon:[[@LINE+7]]:18: error: impl of undefined interface Z [ResolveFacetTypeWithUndefinedInterface]
+  // CHECK:STDERR: fail_extend_impl_self_interface.carbon:[[@LINE+7]]:3: error: impl as incomplete facet type `Z` [ImplAsIncompleteFacetType]
   // CHECK:STDERR:   extend impl as Z {
-  // CHECK:STDERR:                  ^
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~
   // CHECK:STDERR: fail_extend_impl_self_interface.carbon:[[@LINE-10]]:1: note: interface is currently being defined [InterfaceUndefinedWithinDefinition]
   // CHECK:STDERR: interface Z {
   // CHECK:STDERR: ^~~~~~~~~~~~~

+ 2 - 2
toolchain/check/testdata/impl/no_prelude/fail_impl_as_scope.carbon

@@ -49,9 +49,9 @@ interface Z {
    // CHECK:STDERR:    impl as Z {
    // CHECK:STDERR:         ^~
    // CHECK:STDERR:
-   // CHECK:STDERR: fail_impl_as_self_interface.carbon:[[@LINE+7]]:12: error: impl of undefined interface Z [ResolveFacetTypeWithUndefinedInterface]
+   // CHECK:STDERR: fail_impl_as_self_interface.carbon:[[@LINE+7]]:4: error: impl as incomplete facet type `Z` [ImplAsIncompleteFacetType]
    // CHECK:STDERR:    impl as Z {
-   // CHECK:STDERR:            ^
+   // CHECK:STDERR:    ^~~~~~~~~~~
    // CHECK:STDERR: fail_impl_as_self_interface.carbon:[[@LINE-10]]:1: note: interface is currently being defined [InterfaceUndefinedWithinDefinition]
    // CHECK:STDERR: interface Z {
    // CHECK:STDERR: ^~~~~~~~~~~~~

+ 4 - 4
toolchain/check/testdata/impl/no_prelude/fail_undefined_interface.carbon

@@ -13,9 +13,9 @@
 library "[[@TEST_NAME]]";
 
 interface I;
-// CHECK:STDERR: fail_empty_struct.carbon:[[@LINE+7]]:12: error: impl of undefined interface I [ResolveFacetTypeWithUndefinedInterface]
+// CHECK:STDERR: fail_empty_struct.carbon:[[@LINE+7]]:1: error: impl as incomplete facet type `I` [ImplAsIncompleteFacetType]
 // CHECK:STDERR: impl {} as I {}
-// CHECK:STDERR:            ^
+// CHECK:STDERR: ^~~~~~~~~~~~~~
 // CHECK:STDERR: fail_empty_struct.carbon:[[@LINE-4]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
 // CHECK:STDERR: interface I;
 // CHECK:STDERR: ^~~~~~~~~~~~
@@ -28,9 +28,9 @@ library "[[@TEST_NAME]]";
 
 interface J;
 class C {}
-// CHECK:STDERR: fail_class.carbon:[[@LINE+7]]:11: error: impl of undefined interface J [ResolveFacetTypeWithUndefinedInterface]
+// CHECK:STDERR: fail_class.carbon:[[@LINE+7]]:1: error: impl as incomplete facet type `J` [ImplAsIncompleteFacetType]
 // CHECK:STDERR: impl C as J {}
-// CHECK:STDERR:           ^
+// CHECK:STDERR: ^~~~~~~~~~~~~
 // CHECK:STDERR: fail_class.carbon:[[@LINE-5]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
 // CHECK:STDERR: interface J;
 // CHECK:STDERR: ^~~~~~~~~~~~

+ 18 - 8
toolchain/check/testdata/interface/no_prelude/assoc_const_in_generic.carbon

@@ -35,8 +35,8 @@ fn H() {
 // CHECK:STDOUT:   %Self: %I.type.325 = bind_symbolic_name Self, 1 [symbolic]
 // CHECK:STDOUT:   %U: type = bind_symbolic_name U, 2 [symbolic]
 // CHECK:STDOUT:   %U.patt: type = symbolic_binding_pattern U, 2 [symbolic]
-// CHECK:STDOUT:   %F.type: type = fn_type @F, @I(%T) [symbolic]
-// CHECK:STDOUT:   %F: %F.type = struct_value () [symbolic]
+// CHECK:STDOUT:   %F.type.2ae: type = fn_type @F, @I(%T) [symbolic]
+// CHECK:STDOUT:   %F.bb2: %F.type.2ae = struct_value () [symbolic]
 // CHECK:STDOUT:   %I.assoc_type.955: type = assoc_entity_type %I.type.325 [symbolic]
 // CHECK:STDOUT:   %assoc0.fef: %I.assoc_type.955 = assoc_entity element0, @I.%F.decl [symbolic]
 // CHECK:STDOUT:   %G.type: type = fn_type @G [concrete]
@@ -48,9 +48,11 @@ fn H() {
 // CHECK:STDOUT:   %empty_struct_type: type = struct_type {} [concrete]
 // CHECK:STDOUT:   %G.specific_fn: <specific function> = specific_function %G, @G(%empty_struct_type) [concrete]
 // CHECK:STDOUT:   %I.type.885: type = facet_type <@I, @I(%empty_struct_type)> [concrete]
-// CHECK:STDOUT:   %complete_type.788: <witness> = complete_type_witness %I.type.885 [concrete]
+// CHECK:STDOUT:   %F.type.684: type = fn_type @F, @I(%empty_struct_type) [concrete]
+// CHECK:STDOUT:   %F.a8d: %F.type.684 = struct_value () [concrete]
 // CHECK:STDOUT:   %I.assoc_type.67f: type = assoc_entity_type %I.type.885 [concrete]
 // CHECK:STDOUT:   %assoc0.639: %I.assoc_type.67f = assoc_entity element0, @I.%F.decl [concrete]
+// CHECK:STDOUT:   %complete_type.788: <witness> = complete_type_witness %I.type.885 [concrete]
 // CHECK:STDOUT:   %complete_type.973: <witness> = complete_type_witness %I.assoc_type.67f [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -84,14 +86,14 @@ fn H() {
 // CHECK:STDOUT: !definition:
 // CHECK:STDOUT:   %I.type: type = facet_type <@I, @I(%T.loc11_13.2)> [symbolic = %I.type (constants.%I.type.325)]
 // CHECK:STDOUT:   %Self.2: %I.type.325 = bind_symbolic_name Self, 1 [symbolic = %Self.2 (constants.%Self)]
-// CHECK:STDOUT:   %F.type: type = fn_type @F, @I(%T.loc11_13.2) [symbolic = %F.type (constants.%F.type)]
-// CHECK:STDOUT:   %F: @I.%F.type (%F.type) = struct_value () [symbolic = %F (constants.%F)]
+// CHECK:STDOUT:   %F.type: type = fn_type @F, @I(%T.loc11_13.2) [symbolic = %F.type (constants.%F.type.2ae)]
+// CHECK:STDOUT:   %F: @I.%F.type (%F.type.2ae) = struct_value () [symbolic = %F (constants.%F.bb2)]
 // CHECK:STDOUT:   %I.assoc_type: type = assoc_entity_type @I.%I.type (%I.type.325) [symbolic = %I.assoc_type (constants.%I.assoc_type.955)]
 // CHECK:STDOUT:   %assoc0.loc12_22.2: @I.%I.assoc_type (%I.assoc_type.955) = assoc_entity element0, %F.decl [symbolic = %assoc0.loc12_22.2 (constants.%assoc0.fef)]
 // CHECK:STDOUT:
 // CHECK:STDOUT:   interface {
 // CHECK:STDOUT:     %Self.1: @I.%I.type (%I.type.325) = bind_symbolic_name Self, 1 [symbolic = %Self.2 (constants.%Self)]
-// CHECK:STDOUT:     %F.decl: @I.%F.type (%F.type) = fn_decl @F [symbolic = @I.%F (constants.%F)] {
+// CHECK:STDOUT:     %F.decl: @I.%F.type (%F.type.2ae) = fn_decl @F [symbolic = @I.%F (constants.%F.bb2)] {
 // CHECK:STDOUT:       %U.patt.loc12_8.2: type = symbolic_binding_pattern U, 2 [symbolic = %U.patt.loc12_8.1 (constants.%U.patt)]
 // CHECK:STDOUT:       %U.param_patt: type = value_param_pattern %U.patt.loc12_8.2, runtime_param<none> [symbolic = %U.patt.loc12_8.1 (constants.%U.patt)]
 // CHECK:STDOUT:       %return.patt: @F.%U.loc12_8.1 (%U) = return_slot_pattern
@@ -158,8 +160,8 @@ fn H() {
 // CHECK:STDOUT: !definition:
 // CHECK:STDOUT:   %I.type => constants.%I.type.325
 // CHECK:STDOUT:   %Self.2 => constants.%Self
-// CHECK:STDOUT:   %F.type => constants.%F.type
-// CHECK:STDOUT:   %F => constants.%F
+// CHECK:STDOUT:   %F.type => constants.%F.type.2ae
+// CHECK:STDOUT:   %F => constants.%F.bb2
 // CHECK:STDOUT:   %I.assoc_type => constants.%I.assoc_type.955
 // CHECK:STDOUT:   %assoc0.loc12_22.2 => constants.%assoc0.fef
 // CHECK:STDOUT: }
@@ -193,5 +195,13 @@ fn H() {
 // CHECK:STDOUT: specific @I(constants.%empty_struct_type) {
 // CHECK:STDOUT:   %T.loc11_13.2 => constants.%empty_struct_type
 // CHECK:STDOUT:   %T.patt.loc11_13.2 => constants.%empty_struct_type
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %I.type => constants.%I.type.885
+// CHECK:STDOUT:   %Self.2 => constants.%Self
+// CHECK:STDOUT:   %F.type => constants.%F.type.684
+// CHECK:STDOUT:   %F => constants.%F.a8d
+// CHECK:STDOUT:   %I.assoc_type => constants.%I.assoc_type.67f
+// CHECK:STDOUT:   %assoc0.loc12_22.2 => constants.%assoc0.639
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 2 - 2
toolchain/check/testdata/interface/no_prelude/fail_lookup_undefined.carbon

@@ -20,7 +20,7 @@ interface Undefined;
 fn Undefined.F();
 
 fn Test() {
-  // CHECK:STDERR: fail_lookup_undefined.carbon:[[@LINE+7]]:3: error: member access into undefined interface Undefined [ResolveFacetTypeWithUndefinedInterface]
+  // CHECK:STDERR: fail_lookup_undefined.carbon:[[@LINE+7]]:3: error: member access into incomplete facet type `Undefined` [QualifiedExprInIncompleteFacetTypeScope]
   // CHECK:STDERR:   Undefined.G();
   // CHECK:STDERR:   ^~~~~~~~~~~
   // CHECK:STDERR: fail_lookup_undefined.carbon:[[@LINE-15]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
@@ -31,7 +31,7 @@ fn Test() {
 }
 
 interface BeingDefined {
-  // CHECK:STDERR: fail_lookup_undefined.carbon:[[@LINE+7]]:13: error: member access into undefined interface BeingDefined [ResolveFacetTypeWithUndefinedInterface]
+  // CHECK:STDERR: fail_lookup_undefined.carbon:[[@LINE+7]]:13: error: member access into incomplete facet type `BeingDefined` [QualifiedExprInIncompleteFacetTypeScope]
   // CHECK:STDERR:   fn H() -> BeingDefined.T;
   // CHECK:STDERR:             ^~~~~~~~~~~~~~
   // CHECK:STDERR: fail_lookup_undefined.carbon:[[@LINE-4]]:1: note: interface is currently being defined [InterfaceUndefinedWithinDefinition]

+ 4 - 0
toolchain/check/testdata/interface/no_prelude/generic.carbon

@@ -518,6 +518,10 @@ fn G(T:! Generic(B)) {
 // CHECK:STDOUT: specific @Generic(constants.%A) {
 // CHECK:STDOUT:   %T.loc4_19.2 => constants.%A
 // CHECK:STDOUT:   %T.patt.loc4_19.2 => constants.%A
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %Generic.type => constants.%Generic.type.c7c
+// CHECK:STDOUT:   %Self.2 => constants.%Self
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @F(constants.%T.a53) {

+ 1 - 1
toolchain/check/testdata/where_expr/dot_self_index.carbon

@@ -37,9 +37,9 @@ fn G(U: Empty(i32) where .A = i32*) {
 // CHECK:STDOUT:   %T.patt: type = symbolic_binding_pattern T, 0 [symbolic]
 // CHECK:STDOUT:   %Empty.type.3e5fde.2: type = facet_type <@Empty, @Empty(%T)> [symbolic]
 // CHECK:STDOUT:   %.Self.c95: %Empty.type.3e5fde.2 = bind_symbolic_name .Self [symbolic]
-// CHECK:STDOUT:   %require_complete.22f: <witness> = require_complete_type %Empty.type.3e5fde.2 [symbolic]
 // CHECK:STDOUT:   %Empty.assoc_type.54c14f.2: type = assoc_entity_type %Empty.type.3e5fde.2 [symbolic]
 // CHECK:STDOUT:   %assoc0.68f673.2: %Empty.assoc_type.54c14f.2 = assoc_entity element0, @Empty.%A [symbolic]
+// CHECK:STDOUT:   %require_complete.22f: <witness> = require_complete_type %Empty.type.3e5fde.2 [symbolic]
 // CHECK:STDOUT:   %.Self.as_type.a75: type = facet_access_type %.Self.c95 [symbolic]
 // CHECK:STDOUT:   %.Self.as_wit.da4: <witness> = facet_access_witness %.Self.c95 [symbolic]
 // CHECK:STDOUT:   %Empty.facet.bea: %Empty.type.3e5fde.2 = facet_value %.Self.as_type.a75, %.Self.as_wit.da4 [symbolic]

+ 49 - 57
toolchain/check/type_completion.cpp

@@ -283,6 +283,52 @@ auto TypeCompleter::AddNestedIncompleteTypes(SemIR::Inst type_inst) -> bool {
       Push(inst.inner_id);
       break;
     }
+    case CARBON_KIND(SemIR::FacetType inst): {
+      if (context_.complete_facet_types()
+              .TryGetId(inst.facet_type_id)
+              .has_value()) {
+        break;
+      }
+      const auto& facet_type_info =
+          context_.facet_types().Get(inst.facet_type_id);
+
+      SemIR::CompleteFacetType result;
+      result.required_interfaces.reserve(
+          facet_type_info.impls_constraints.size());
+      // Every mentioned interface needs to be defined.
+      for (auto impl_interface : facet_type_info.impls_constraints) {
+        // TODO: expand named constraints
+        auto interface_id = impl_interface.interface_id;
+        const auto& interface = context_.interfaces().Get(interface_id);
+        if (!interface.is_defined()) {
+          if (diagnoser_) {
+            auto builder = diagnoser_();
+            NoteUndefinedInterface(context_, interface_id, builder);
+            builder.Emit();
+          }
+          return false;
+        }
+
+        if (impl_interface.specific_id.has_value()) {
+          ResolveSpecificDefinition(context_, loc_, impl_interface.specific_id);
+        }
+        result.required_interfaces.push_back(
+            {.interface_id = interface_id,
+             .specific_id = impl_interface.specific_id});
+      }
+      // TODO: Sort and deduplicate result.required_interfaces. For now, we have
+      // at most one.
+      CARBON_CHECK(result.required_interfaces.size() <= 1);
+
+      // TODO: Distinguish interfaces that are required but would not be
+      // implemented, such as those from `where .Self impls I`.
+      result.num_to_impl = result.required_interfaces.size();
+
+      // TODO: Process other kinds of requirements.
+      context_.complete_facet_types().Add(inst.facet_type_id, result);
+      break;
+    }
+
     default:
       break;
   }
@@ -574,70 +620,16 @@ auto RequireConcreteType(Context& context, SemIR::TypeId type_id,
   return true;
 }
 
-static auto AddCompleteFacetType(Context& context, SemIR::LocId loc_id,
-                                 SemIR::FacetTypeId facet_type_id,
-                                 FacetTypeContext context_for_diagnostics)
-    -> SemIR::CompleteFacetTypeId {
-  const auto& facet_type_info = context.facet_types().Get(facet_type_id);
-
-  SemIR::CompleteFacetType result;
-  result.required_interfaces.reserve(facet_type_info.impls_constraints.size());
-  // Every mentioned interface needs to be defined.
-  for (auto impl_interface : facet_type_info.impls_constraints) {
-    // TODO: expand named constraints
-    auto interface_id = impl_interface.interface_id;
-    const auto& interface = context.interfaces().Get(interface_id);
-    if (!interface.is_defined()) {
-      CARBON_DIAGNOSTIC(
-          ResolveFacetTypeWithUndefinedInterface, Error,
-          "{0:=0:member access into|=1:impl of} undefined interface {1}",
-          IntAsSelect, SemIR::NameId);
-      auto builder = context.emitter().Build(
-          loc_id, ResolveFacetTypeWithUndefinedInterface,
-          static_cast<int>(context_for_diagnostics), interface.name_id);
-      NoteUndefinedInterface(context, interface_id, builder);
-      builder.Emit();
-      return SemIR::CompleteFacetTypeId::None;
-    }
-
-    if (impl_interface.specific_id.has_value()) {
-      ResolveSpecificDefinition(context, loc_id, impl_interface.specific_id);
-    }
-    result.required_interfaces.push_back(
-        {.interface_id = interface_id,
-         .specific_id = impl_interface.specific_id});
-  }
-  // TODO: Sort and deduplicate result.required_interfaces. For now, we have at
-  // most one.
-  CARBON_CHECK(result.required_interfaces.size() <= 1);
-
-  // TODO: Distinguish interfaces that are required but would not be
-  // implemented, such as those from `where .Self impls I`.
-  result.num_to_impl = result.required_interfaces.size();
-  return context.complete_facet_types().Add(facet_type_id, result);
-}
-
-// TODO: RequireCompleteType should do these checks, this should just return
-// additional information.
 auto RequireCompleteFacetType(Context& context, SemIR::TypeId type_id,
                               SemIR::LocId loc_id,
                               const SemIR::FacetType& facet_type,
-                              FacetTypeContext context_for_diagnostics)
+                              Context::BuildDiagnosticFn diagnoser)
     -> SemIR::CompleteFacetTypeId {
-  if (!RequireCompleteType(
-          context, type_id, loc_id, [&]() -> Context::DiagnosticBuilder {
-            CARBON_FATAL("Unreachable, facet types are always complete.");
-          })) {
+  if (!RequireCompleteType(context, type_id, loc_id, diagnoser)) {
     return SemIR::CompleteFacetTypeId::None;
   }
 
-  auto complete_id =
-      context.complete_facet_types().TryGetId(facet_type.facet_type_id);
-  if (!complete_id.has_value()) {
-    complete_id = AddCompleteFacetType(
-        context, loc_id, facet_type.facet_type_id, context_for_diagnostics);
-  }
-  return complete_id;
+  return context.complete_facet_types().TryGetId(facet_type.facet_type_id);
 }
 
 auto AsCompleteType(Context& context, SemIR::TypeId type_id,

+ 3 - 7
toolchain/check/type_completion.h

@@ -48,16 +48,12 @@ auto RequireConcreteType(Context& context, SemIR::TypeId type_id,
                          Context::BuildDiagnosticFn diagnoser,
                          Context::BuildDiagnosticFn abstract_diagnoser) -> bool;
 
-// Like `RequireCompleteType`, but also require the facet type to be fully
-// defined with known members. If it uses some incomplete interface, diagnoses
-// the problem and returns None.
-// TODO: Get rid of this enum and use the `RequireCompleteType` diagnostic
-// mechanism instead.
-enum FacetTypeContext { FacetTypeMemberAccess, FacetTypeImpl };
+// Like `RequireCompleteType`, but only for facet types. If it uses some
+// incomplete interface, diagnoses the problem and returns `None`.
 auto RequireCompleteFacetType(Context& context, SemIR::TypeId type_id,
                               SemIR::LocId loc_id,
                               const SemIR::FacetType& facet_type,
-                              FacetTypeContext context_for_diagnostics)
+                              Context::BuildDiagnosticFn diagnoser)
     -> SemIR::CompleteFacetTypeId;
 
 // Returns the type `type_id` if it is a complete type, or produces an

+ 2 - 1
toolchain/diagnostics/diagnostic_kind.def

@@ -284,6 +284,7 @@ CARBON_DIAGNOSTIC_KIND(ExtendImplOutsideClass)
 CARBON_DIAGNOSTIC_KIND(ExtendImplSelfAs)
 CARBON_DIAGNOSTIC_KIND(ExtendImplSelfAsDefault)
 CARBON_DIAGNOSTIC_KIND(ImplAccessMemberBeforeComplete)
+CARBON_DIAGNOSTIC_KIND(ImplAsIncompleteFacetType)
 CARBON_DIAGNOSTIC_KIND(ImplAsNonFacetType)
 CARBON_DIAGNOSTIC_KIND(ImplAsOutsideClass)
 CARBON_DIAGNOSTIC_KIND(ImplAssociatedConstantNeedsValue)
@@ -411,6 +412,7 @@ CARBON_DIAGNOSTIC_KIND(UnexpectedDeclNameParams)
 CARBON_DIAGNOSTIC_KIND(QualifiedNameInNonScope)
 CARBON_DIAGNOSTIC_KIND(QualifiedNameNonScopeEntity)
 CARBON_DIAGNOSTIC_KIND(QualifiedExprInIncompleteClassScope)
+CARBON_DIAGNOSTIC_KIND(QualifiedExprInIncompleteFacetTypeScope)
 CARBON_DIAGNOSTIC_KIND(QualifiedExprUnsupported)
 CARBON_DIAGNOSTIC_KIND(QualifiedExprNameNotFound)
 CARBON_DIAGNOSTIC_KIND(UseOfNonExprAsValue)
@@ -448,7 +450,6 @@ CARBON_DIAGNOSTIC_KIND(WhereOnNonFacetType)
 CARBON_DIAGNOSTIC_KIND(AssociatedConstantNotConstantAfterConversion)
 CARBON_DIAGNOSTIC_KIND(AssociatedConstantWithDifferentValues)
 CARBON_DIAGNOSTIC_KIND(RewriteForAssociatedFunction)
-CARBON_DIAGNOSTIC_KIND(ResolveFacetTypeWithUndefinedInterface)
 
 // Pattern matching diagnostics.
 CARBON_DIAGNOSTIC_KIND(TuplePatternSizeDoesntMatchLiteral)

+ 0 - 12
toolchain/lower/testdata/interface/basic.carbon

@@ -16,11 +16,6 @@ interface I {
 // you can't pass a facet around at runtime, so make sure it works.
 fn F(T: I) -> I { return T; }
 
-interface J;
-
-// Declared-but-not-defined interfaces are still complete types.
-fn G(T: J) {}
-
 // CHECK:STDOUT: ; ModuleID = 'basic.carbon'
 // CHECK:STDOUT: source_filename = "basic.carbon"
 // CHECK:STDOUT:
@@ -29,11 +24,6 @@ fn G(T: J) {}
 // CHECK:STDOUT:   ret void, !dbg !7
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: define void @_CG.Main() !dbg !8 {
-// CHECK:STDOUT: entry:
-// CHECK:STDOUT:   ret void, !dbg !9
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
 // CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
 // CHECK:STDOUT: !llvm.dbg.cu = !{!2}
 // CHECK:STDOUT:
@@ -45,5 +35,3 @@ fn G(T: J) {}
 // CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
 // CHECK:STDOUT: !6 = !{}
 // CHECK:STDOUT: !7 = !DILocation(line: 17, column: 19, scope: !4)
-// CHECK:STDOUT: !8 = distinct !DISubprogram(name: "G", linkageName: "_CG.Main", scope: null, file: !3, line: 22, type: !5, spFlags: DISPFlagDefinition, unit: !2)
-// CHECK:STDOUT: !9 = !DILocation(line: 22, column: 1, scope: !8)