Преглед изворни кода

Impl lookup allowed for incomplete facet types (#5132)

New planned direction is to not require completeness. Updated comments
to reflect that some care will be needed once named constraints are
supported. Long term plan is [discussed in
#5089](https://github.com/carbon-language/carbon-lang/pull/5089#discussion_r1985908453):

> We have some options. In our last conversation, it sounded like it
would be beneficial for named constraints to have their own witnesses,
with entries in declaration order. This would allow accesses to the
named constraint while it was being defined. So there would be something
of a hierarchy in a facet type witness, with a named constraint taking a
single slot in a facet type witness, independent of its definition.

Co-authored-by: Josh L <josh11b@users.noreply.github.com>
josh11b пре 1 година
родитељ
комит
fb3721df9a

+ 25 - 30
toolchain/check/impl_lookup.cpp

@@ -173,16 +173,15 @@ static auto GetInterfacesFromConstantId(
   const auto& facet_type_info =
       context.facet_types().Get(facet_type_inst.facet_type_id);
   has_other_requirements = facet_type_info.other_requirements;
-  // TODO: Once we add support for named constraints, we will need to change
-  // this to return the same interfaces as in the complete facet type.
+  // TODO: This needs to match the order of witnesses for the facet type, which
+  // will need to be maintained once we add support for named constraints.
   return facet_type_info.impls_constraints;
 }
 
-static auto GetWitnessIdForImpl(
-    Context& context, SemIR::LocId loc_id,
-    SemIR::ConstantId query_self_const_id,
-    const SemIR::CompleteFacetType::RequiredInterface& interface,
-    SemIR::ImplId impl_id) -> SemIR::InstId {
+static auto GetWitnessIdForImpl(Context& context, SemIR::LocId loc_id,
+                                SemIR::ConstantId query_self_const_id,
+                                const SemIR::SpecificInterface& interface,
+                                SemIR::ImplId impl_id) -> SemIR::InstId {
   // The impl may have generic arguments, in which case we need to deduce them
   // to find what they are given the specific type and interface query. We use
   // that specific to map values in the impl to the deduced values.
@@ -275,26 +274,22 @@ static auto FindWitnessInFacet(
   SemIR::TypeId facet_type_id = context.insts().Get(facet_inst_id).type_id();
   if (auto facet_type_inst =
           context.types().TryGetAs<SemIR::FacetType>(facet_type_id)) {
-    auto complete_facet_type_id = RequireCompleteFacetType(
-        context, facet_type_id, loc_id, *facet_type_inst,
-        [&]() -> DiagnosticBuilder {
-          context.TODO(loc_id, "impl lookup on incomplete facet type");
-          return context.emitter().BuildSuppressed();
-        });
-    if (complete_facet_type_id.has_value()) {
-      const auto& complete_facet_type =
-          context.complete_facet_types().Get(complete_facet_type_id);
-      for (auto [index, interface] :
-           llvm::enumerate(complete_facet_type.required_interfaces)) {
-        if (interface == specific_interface) {
-          return GetOrAddInst(
-              context, loc_id,
-              SemIR::FacetAccessWitness{
-                  .type_id = GetSingletonType(
-                      context, SemIR::WitnessType::SingletonInstId),
-                  .facet_value_inst_id = facet_inst_id,
-                  .index = SemIR::ElementIndex(index)});
-        }
+    const auto& facet_type_info =
+        context.facet_types().Get(facet_type_inst->facet_type_id);
+    // TODO: This depends on the index into `impls_constraints` matching
+    // the index into the facet type witness. This will have to be maintained
+    // even for facet types that include named constraints, once that is
+    // supported.
+    for (auto [index, interface] :
+         llvm::enumerate(facet_type_info.impls_constraints)) {
+      if (interface == specific_interface) {
+        return GetOrAddInst(
+            context, loc_id,
+            SemIR::FacetAccessWitness{
+                .type_id = GetSingletonType(
+                    context, SemIR::WitnessType::SingletonInstId),
+                .facet_value_inst_id = facet_inst_id,
+                .index = SemIR::ElementIndex(index)});
       }
     }
   }
@@ -415,9 +410,9 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
       .query_self_const_id = query_self_const_id,
       .query_facet_type_const_id = query_facet_type_const_id,
   });
-  // We need to find a witness for each interface in `interfaces`. We return
-  // them in the same order as they are found in the `CompleteFacetType`, which
-  // is the same order as in `interfaces` here.
+  // We need to find a witness for each interface in `interfaces`. Every
+  // consumer of a facet type needs to agree on the order of interfaces used for
+  // its witnesses.
   for (const auto& interface : interfaces) {
     // TODO: Since both `interfaces` and `query_self_const_id` are sorted lists,
     // do an O(N+M) merge instead of O(N*M) nested loops.

+ 7 - 15
toolchain/check/testdata/interface/no_prelude/fail_assoc_const_alias.carbon

@@ -26,10 +26,6 @@ interface I {
 
 interface J {
   alias U = I.T;
-  // CHECK:STDERR: fail_alias_to_different_interface.carbon:[[@LINE+11]]:13: error: semantics TODO: `impl lookup on incomplete facet type` [SemanticsTodo]
-  // CHECK:STDERR:   fn F() -> U;
-  // CHECK:STDERR:             ^
-  // CHECK:STDERR:
   // CHECK:STDERR: fail_alias_to_different_interface.carbon:[[@LINE+7]]:13: error: cannot implicitly convert type `Self` that implements `J` into type implementing `I` [ConversionFailureFacetToFacet]
   // CHECK:STDERR:   fn F() -> U;
   // CHECK:STDERR:             ^
@@ -55,10 +51,6 @@ impl forall [V:! J2] V as I2 where .T2 = () {}
 
 interface J2 {
   alias U2 = I2.T2;
-  // CHECK:STDERR: fail_todo_alias_to_different_interface_with_requires.carbon:[[@LINE+11]]:14: error: semantics TODO: `impl lookup on incomplete facet type` [SemanticsTodo]
-  // CHECK:STDERR:   fn F2() -> U2;
-  // CHECK:STDERR:              ^~
-  // CHECK:STDERR:
   // CHECK:STDERR: fail_todo_alias_to_different_interface_with_requires.carbon:[[@LINE+7]]:14: error: cannot implicitly convert type `Self` that implements `J2` into type implementing `I2` [ConversionFailureFacetToFacet]
   // CHECK:STDERR:   fn F2() -> U2;
   // CHECK:STDERR:              ^~
@@ -277,7 +269,7 @@ interface C {
 // CHECK:STDOUT:     %return.patt: <error> = return_slot_pattern
 // CHECK:STDOUT:     %return.param_patt: <error> = out_param_pattern %return.patt, call_param0
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %.loc22: %I.type = converted @J.%Self, <error> [concrete = <error>]
+// CHECK:STDOUT:     %.loc18: %I.type = converted @J.%Self, <error> [concrete = <error>]
 // CHECK:STDOUT:     %U.ref: <error> = name_ref U, <error> [concrete = <error>]
 // CHECK:STDOUT:     %return.param: ref <error> = out_param call_param0
 // CHECK:STDOUT:     %return: ref <error> = return_slot %return.param
@@ -475,10 +467,10 @@ interface C {
 // CHECK:STDOUT:     %return.patt: <error> = return_slot_pattern
 // CHECK:STDOUT:     %return.param_patt: <error> = out_param_pattern %return.patt, call_param0
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %Self.as_type.loc26_14.2: type = facet_access_type constants.%Self.541 [symbolic = %Self.as_type.loc26_14.1 (constants.%Self.as_type.f5c)]
-// CHECK:STDOUT:     %.loc26_14.1: type = converted constants.%Self.541, %Self.as_type.loc26_14.2 [symbolic = %Self.as_type.loc26_14.1 (constants.%Self.as_type.f5c)]
-// CHECK:STDOUT:     %.loc26_14.2: %J2.type = converted %.loc26_14.1, constants.%Self.541 [symbolic = %Self (constants.%Self.541)]
-// CHECK:STDOUT:     %.loc26_14.3: %I2.type = converted @J2.%Self, <error> [concrete = <error>]
+// CHECK:STDOUT:     %Self.as_type.loc22_14.2: type = facet_access_type constants.%Self.541 [symbolic = %Self.as_type.loc22_14.1 (constants.%Self.as_type.f5c)]
+// CHECK:STDOUT:     %.loc22_14.1: type = converted constants.%Self.541, %Self.as_type.loc22_14.2 [symbolic = %Self.as_type.loc22_14.1 (constants.%Self.as_type.f5c)]
+// CHECK:STDOUT:     %.loc22_14.2: %J2.type = converted %.loc22_14.1, constants.%Self.541 [symbolic = %Self (constants.%Self.541)]
+// CHECK:STDOUT:     %.loc22_14.3: %I2.type = converted @J2.%Self, <error> [concrete = <error>]
 // CHECK:STDOUT:     %U2.ref: <error> = name_ref U2, <error> [concrete = <error>]
 // CHECK:STDOUT:     %return.param: ref <error> = out_param call_param0
 // CHECK:STDOUT:     %return: ref <error> = return_slot %return.param
@@ -542,7 +534,7 @@ interface C {
 // CHECK:STDOUT:
 // CHECK:STDOUT: generic fn @F2(@J2.%Self: %J2.type) {
 // CHECK:STDOUT:   %Self: %J2.type = bind_symbolic_name Self, 0 [symbolic = %Self (constants.%Self.541)]
-// CHECK:STDOUT:   %Self.as_type.loc26_14.1: type = facet_access_type %Self [symbolic = %Self.as_type.loc26_14.1 (constants.%Self.as_type.f5c)]
+// CHECK:STDOUT:   %Self.as_type.loc22_14.1: type = facet_access_type %Self [symbolic = %Self.as_type.loc22_14.1 (constants.%Self.as_type.f5c)]
 // CHECK:STDOUT:
 // CHECK:STDOUT:   fn() -> <error>;
 // CHECK:STDOUT: }
@@ -598,7 +590,7 @@ interface C {
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @F2(constants.%Self.541) {
 // CHECK:STDOUT:   %Self => constants.%Self.541
-// CHECK:STDOUT:   %Self.as_type.loc26_14.1 => constants.%Self.as_type.f5c
+// CHECK:STDOUT:   %Self.as_type.loc22_14.1 => constants.%Self.as_type.f5c
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_call_method_alias.carbon

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

@@ -510,10 +510,6 @@ fn G(T:! Generic(B)) {
 // CHECK:STDOUT: specific @Generic(constants.%B) {
 // CHECK:STDOUT:   %T.loc4_19.2 => constants.%B
 // CHECK:STDOUT:   %T.patt.loc4_19.2 => constants.%B
-// CHECK:STDOUT:
-// CHECK:STDOUT: !definition:
-// CHECK:STDOUT:   %Generic.type => constants.%Generic.type.4ce
-// CHECK:STDOUT:   %Self.2 => constants.%Self
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @G(constants.%T.bae) {