Allow facet types to be combined (#5026)

The resulting facet type has its complete facet type canonicalized by
sorting and deduplicating the `required_interfaces`.

Impl lookup now uses the complete facet type. It continues to diagnose
with a TODO if it sees a complete facet type with 0 or more than 1
interface in it. Impl lookup to convert from a type to a facet value
hits this diagnosis.

Member lookup by name works on a facet type with more than one interface
because the name knows which interface to look for from the name, and
AppendLookupScopesForConstant() looks through all interfaces on the
complete facet type already to get the correct scope for the name.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

toolchain/check/impl_lookup.cpp

@@ -5,6 +5,7 @@
 #include "toolchain/check/impl_lookup.h"
 
 #include "toolchain/check/deduce.h"
+#include "toolchain/check/diagnostic_helpers.h"
 #include "toolchain/check/generic.h"
 #include "toolchain/check/import_ref.h"
 #include "toolchain/check/type_completion.h"
@@ -164,23 +165,39 @@ static auto FindAndDiagnoseImplLookupCycle(
 static auto GetInterfaceIdFromConstantId(Context& context, SemIR::LocId loc_id,
                                          SemIR::ConstantId interface_const_id)
     -> SemIR::InterfaceId {
+  // The `interface_const_id` is a constant value for some facet type. We do
+  // this long chain of steps to go from that constant value to the
+  // `FacetTypeId` found on the `FacetType` instruction of this constant value,
+  // and finally to the `CompleteFacetType`.
   auto facet_type_inst_id =
       context.constant_values().GetInstId(interface_const_id);
-  auto facet_type_id =
-      context.insts().GetAs<SemIR::FacetType>(facet_type_inst_id).facet_type_id;
-  const auto& facet_type_info = context.facet_types().Get(facet_type_id);
-  if (facet_type_info.impls_constraints.empty()) {
+  auto facet_type_inst =
+      context.insts().GetAs<SemIR::FacetType>(facet_type_inst_id);
+  auto facet_type_id = facet_type_inst.facet_type_id;
+  auto complete_facet_type_id =
+      context.complete_facet_types().TryGetId(facet_type_id);
+  // The facet type will already be completed before coming here. If we're
+  // converting from a concrete type to a facet type, the conversion step
+  // requires everything to be complete before doing impl lookup.
+  CARBON_CHECK(complete_facet_type_id.has_value());
+  const auto& complete_facet_type =
+      context.complete_facet_types().Get(complete_facet_type_id);
+
+  if (complete_facet_type.required_interfaces.empty()) {
+    // This should never happen - a FacetType either requires or is bounded by
+    // some `.Self impls` clause. Otherwise you would just have `type` (aka
+    // `TypeType` in the toolchain implementation) which is not a facet type.
     context.TODO(loc_id,
                  "impl lookup for a FacetType with no interface (using "
                  "`where .Self impls ...` instead?)");
     return SemIR::InterfaceId::None;
   }
-  if (facet_type_info.impls_constraints.size() > 1) {
+  if (complete_facet_type.required_interfaces.size() > 1) {
     context.TODO(loc_id,
                  "impl lookup for a FacetType with more than one interface");
     return SemIR::InterfaceId::None;
   }
-  return facet_type_info.impls_constraints[0].interface_id;
+  return complete_facet_type.required_interfaces[0].interface_id;
 }
 
 static auto GetWitnessIdForImpl(Context& context, SemIR::LocId loc_id,

toolchain/check/testdata/facet/no_prelude/combine.carbon

@@ -17,6 +17,10 @@ interface As(Dest:! type) {
   fn Convert[self: Self]() -> Dest;
 }
 
+interface ImplicitAs(Dest:! type) {
+  fn Convert[self: Self]() -> Dest;
+}
+
 interface BitAnd {
   fn Op[self: Self](other: Self) -> Self;
 }
@@ -75,28 +79,54 @@ fn F() {
   (({} as C) as (C as (A & A))).(A.G)();
 }
 
-// --- fail_todo_combine.carbon
+// --- fail_name_collision.carbon
 library "[[@TEST_NAME]]";
 
 import Core;
 
-interface A {}
+interface A {
+  fn G();
+}
 interface B {
   fn G();
 }
 
 class C {}
-impl C as A {}
+impl C as A {
+  fn G();
+}
 impl C as B {
   fn G() {}
 }
 
 fn F() {
-  // CHECK:STDERR: fail_todo_combine.carbon:[[@LINE+4]]:14: error: semantics TODO: `FacetType with more than one required interface. Sort and deduplicate` [SemanticsTodo]
+  // TODO: This error message is wrong here, we are not using `extend`.
+
+  // CHECK:STDERR: fail_name_collision.carbon:[[@LINE+4]]:14: error: ambiguous use of name `G` found in multiple extended scopes [NameAmbiguousDueToExtend]
   // CHECK:STDERR:   ({} as C).((A & B).G)();
   // CHECK:STDERR:              ^~~~~~~~~
   // CHECK:STDERR:
   ({} as C).((A & B).G)();
+}
+
+// --- fail_todo_combine.carbon
+library "[[@TEST_NAME]]";
+
+import Core;
+
+interface A {}
+interface B {
+  fn G();
+}
+
+class C {}
+impl C as A {}
+impl C as B {
+  fn G() {}
+}
+
+fn F() {
+  ({} as C).((A & B).G)();
 
   // CHECK:STDERR: fail_todo_combine.carbon:[[@LINE+11]]:18: error: semantics TODO: `impl lookup for a FacetType with more than one interface` [SemanticsTodo]
   // CHECK:STDERR:   (({} as C) as (C as (A & B))).((A & B).G)();
@@ -124,3 +154,50 @@ fn F() {
   // CHECK:STDERR:
   (({} as C) as (C as (A & B))).((B).G)();
 }
+
+// --- fail_todo_generic_combine.carbon
+
+library "[[@TEST_NAME]]";
+
+import Core;
+
+interface A {
+}
+interface B {
+  fn G();
+}
+
+fn G[T:! A & B](t: T) {
+  // CHECK:STDERR: fail_todo_generic_combine.carbon:[[@LINE+4]]:3: error: cannot access member of interface `B` in type `T` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   t.(B.G)();
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  t.(B.G)();
+}
+
+class C {}
+impl C as A {}
+impl C as B {
+  fn G() {}
+}
+
+fn F() {
+  // CHECK:STDERR: fail_todo_generic_combine.carbon:[[@LINE+17]]:3: error: semantics TODO: `impl lookup for a FacetType with more than one interface` [SemanticsTodo]
+  // CHECK:STDERR:   G({} as C);
+  // CHECK:STDERR:   ^~~~~~~~~~
+  // CHECK:STDERR: fail_todo_generic_combine.carbon:[[@LINE-18]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere]
+  // CHECK:STDERR: fn G[T:! A & B](t: T) {
+  // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_generic_combine.carbon:[[@LINE+10]]:3: error: cannot implicitly convert from `type` to `A & B` [ImplicitAsConversionFailure]
+  // CHECK:STDERR:   G({} as C);
+  // CHECK:STDERR:   ^~~~~~~~~~
+  // CHECK:STDERR: fail_todo_generic_combine.carbon:[[@LINE+7]]:3: note: type `type` does not implement interface `Core.ImplicitAs(A & B)` [MissingImplInMemberAccessNote]
+  // CHECK:STDERR:   G({} as C);
+  // CHECK:STDERR:   ^~~~~~~~~~
+  // CHECK:STDERR: fail_todo_generic_combine.carbon:[[@LINE-28]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere]
+  // CHECK:STDERR: fn G[T:! A & B](t: T) {
+  // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  G({} as C);
+}

toolchain/check/testdata/facet/no_prelude/fail_incomplete.carbon

@@ -0,0 +1,83 @@
+// 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
+//
+// EXTRA-ARGS: --no-dump-sem-ir
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/facet/no_prelude/fail_incomplete.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/facet/no_prelude/fail_incomplete.carbon
+
+// --- core.carbon
+package Core;
+
+interface As(Dest:! type) {
+  fn Convert[self: Self]() -> Dest;
+}
+
+interface BitAnd {
+  fn Op[self: Self](other: Self) -> Self;
+}
+
+impl forall [T:! type] T as BitAnd {
+  fn Op[self: Self](other: Self) -> Self = "type.and";
+}
+
+// --- fail_incomplete.carbon
+
+library "[[@TEST_NAME]]";
+
+import Core;
+
+interface A;
+interface B {}
+class C {}
+
+fn G[T:! A](t: T) {}
+fn H[T:! A & B](t: T) {}
+
+fn F() {
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE+7]]:17: error: invalid use of incomplete type `A` [IncompleteTypeInConversion]
+  // CHECK:STDERR:   ({} as C) as (C as A);
+  // CHECK:STDERR:                 ^~~~~~
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE-11]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
+  // CHECK:STDERR: interface A;
+  // CHECK:STDERR: ^~~~~~~~~~~~
+  // CHECK:STDERR:
+  ({} as C) as (C as A);
+
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE+7]]:17: error: invalid use of incomplete type `A & B` [IncompleteTypeInConversion]
+  // CHECK:STDERR:   ({} as C) as (C as (A & B));
+  // CHECK:STDERR:                 ^~~~~~~~~~~~
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE-20]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
+  // CHECK:STDERR: interface A;
+  // CHECK:STDERR: ^~~~~~~~~~~~
+  // CHECK:STDERR:
+  ({} as C) as (C as (A & B));
+
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE+10]]:3: error: forming value of incomplete type `A` [IncompleteTypeInValueConversion]
+  // CHECK:STDERR:   G({} as C);
+  // CHECK:STDERR:   ^~~~~~~~~~
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE-29]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
+  // CHECK:STDERR: interface A;
+  // CHECK:STDERR: ^~~~~~~~~~~~
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE-28]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere]
+  // CHECK:STDERR: fn G[T:! A](t: T) {}
+  // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  G({} as C);
+
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE+10]]:3: error: forming value of incomplete type `A & B` [IncompleteTypeInValueConversion]
+  // CHECK:STDERR:   H({} as C);
+  // CHECK:STDERR:   ^~~~~~~~~~
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE-41]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
+  // CHECK:STDERR: interface A;
+  // CHECK:STDERR: ^~~~~~~~~~~~
+  // CHECK:STDERR: fail_incomplete.carbon:[[@LINE-39]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere]
+  // CHECK:STDERR: fn H[T:! A & B](t: T) {}
+  // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  H({} as C);
+}

toolchain/check/type_completion.cpp

@@ -316,13 +316,7 @@ auto TypeCompleter::AddNestedIncompleteTypes(SemIR::Inst type_inst) -> bool {
             {.interface_id = interface_id,
              .specific_id = impl_interface.specific_id});
       }
-      // TODO: Sort and deduplicate result.required_interfaces. For now, we have
-      // at most one.
-      if (result.required_interfaces.size() > 1) {
-        context_.TODO(loc_,
-                      "FacetType with more than one required interface. Sort "
-                      "and deduplicate");
-      }
+      result.CanonicalizeRequiredInterfaces();
 
       // TODO: Distinguish interfaces that are required but would not be
       // implemented, such as those from `where .Self impls I`.

toolchain/sem_ir/facet_type_info.cpp

@@ -26,6 +26,14 @@ static auto RewriteLess(const FacetTypeInfo::RewriteConstraint& lhs,
          std::tie(rhs.lhs_const_id.index, rhs.rhs_const_id.index);
 }
 
+// Canonically ordered by the numerical ids.
+static auto RequiredLess(const CompleteFacetType::RequiredInterface& lhs,
+                         const CompleteFacetType::RequiredInterface& rhs)
+    -> bool {
+  return std::tie(lhs.interface_id.index, lhs.specific_id.index) <
+         std::tie(rhs.interface_id.index, rhs.specific_id.index);
+}
+
 auto FacetTypeInfo::Canonicalize() -> void {
   SortAndDeduplicate(impls_constraints, ImplsLess);
   SortAndDeduplicate(rewrite_constraints, RewriteLess);
@@ -61,4 +69,8 @@ auto FacetTypeInfo::Print(llvm::raw_ostream& out) const -> void {
   out << "}";
 }
 
+auto CompleteFacetType::CanonicalizeRequiredInterfaces() -> void {
+  SortAndDeduplicate(required_interfaces, RequiredLess);
+}
+
 }  // namespace Carbon::SemIR

toolchain/sem_ir/facet_type_info.h

@@ -114,6 +114,10 @@ struct CompleteFacetType {
   int num_to_impl;
 
   // TODO: Which interfaces to perform name lookup into.
+
+  // Sorts and deduplicates `required_interfaces`. Call after building the sets
+  // of interfaces, and then don't mutate them value afterwards.
+  auto CanonicalizeRequiredInterfaces() -> void;
 };
 
 // See common/hashing.h.