Browse Source

Make identifying a facet type an operation on a (self+facet type) pair (#6592)

Identifying a facet type takes both a self and facet type as a pair, and
then encode the self into the IdentifiedFacetType. This makes a
constraint that requires some _other_ type implements an interface
visible in the IdentifiedFacetType. And it will help to enable facet
types with `where T impls Z` for `T` that is not `.Self` in the future.

IdentifiedFacetTypes are now stored in a CanonicalValueStore instead of
a RelationalValueStore as they key is the combination of self and
(declared) facet type together now.

When the self-type is a facet value (has type FacetType) this is most
straightforward. But when it's a type we need to construct a FacetValue
to construct a specific for a require decl, to replace the generic
binding of the symbolic `Self`, which has type FacetType. To do so, we
make a FacetValue with an empty FacetType (equivalent to TypeType). This
prevents any looking for witnesses through the FacetType, which matches
what you can get from a type directly, requiring witnesses to come from
finding an `impl` decl.

Add additional InstNamer logic for such empty facet types so they print
as `<typename>.type.facet` if possible instead of as just `facet_value`.
Dana Jansens 3 months ago
parent
commit
32aa7cb1fa

+ 1 - 1
toolchain/check/context.h

@@ -325,7 +325,7 @@ class Context {
   auto facet_types() -> SemIR::FacetTypeInfoStore& {
     return sem_ir().facet_types();
   }
-  auto identified_facet_types() -> SemIR::File::IdentifiedFacetTypeStore& {
+  auto identified_facet_types() -> SemIR::IdentifiedFacetTypeStore& {
     return sem_ir().identified_facet_types();
   }
   auto impls() -> SemIR::ImplStore& { return sem_ir().impls(); }

+ 2 - 1
toolchain/check/convert.cpp

@@ -1327,7 +1327,8 @@ static auto PerformBuiltinConversion(
         // Note that `FacetValue`'s type is the same `FacetType` that was used
         // to construct the set of witnesses, ie. the query to
         // `LookupImplWitness()`. This ensures that the witnesses are in the
-        // same order as the `required_interfaces()` in the `FacetValue`'s type.
+        // same order as the `required_impls()` in the `IdentifiedFacetType` of
+        // the `FacetValue`'s type.
         return AddInst<SemIR::FacetValue>(
             context, loc_id,
             {.type_id = target.type_id,

+ 22 - 0
toolchain/check/facet_type.cpp

@@ -424,4 +424,26 @@ auto MakePeriodSelfFacetValue(Context& context, SemIR::TypeId self_type_id)
   return inst_id;
 }
 
+auto GetEmptyFacetType(Context& context) -> SemIR::TypeId {
+  SemIR::FacetTypeId facet_type_id =
+      context.facet_types().Add(SemIR::FacetTypeInfo{});
+  auto const_id = EvalOrAddInst<SemIR::FacetType>(
+      context, SemIR::LocId::None,
+      {.type_id = SemIR::TypeType::TypeId, .facet_type_id = facet_type_id});
+  return context.types().GetTypeIdForTypeConstantId(const_id);
+}
+
+auto GetConstantFacetValueForType(Context& context,
+                                  SemIR::TypeInstId type_inst_id)
+    -> SemIR::ConstantId {
+  // We use an empty facet type because values of type `type` do not provide any
+  // witnesses of their own.
+  auto type_facet_type = GetEmptyFacetType(context);
+  return EvalOrAddInst<SemIR::FacetValue>(
+      context, SemIR::LocId::None,
+      {.type_id = type_facet_type,
+       .type_inst_id = type_inst_id,
+       .witnesses_block_id = SemIR::InstBlockId::Empty});
+}
+
 }  // namespace Carbon::Check

+ 13 - 0
toolchain/check/facet_type.h

@@ -72,6 +72,19 @@ auto ResolveFacetTypeRewriteConstraints(
 auto MakePeriodSelfFacetValue(Context& context, SemIR::TypeId self_type_id)
     -> SemIR::InstId;
 
+// Get a FacetType instruction for an empty FacetType. This is the facet
+// equivalent to TypeType.
+//
+// TODO: We vaguely plan to replace TypeType with this FacetType in the future,
+// though that's a big change.
+auto GetEmptyFacetType(Context& context) -> SemIR::TypeId;
+
+// Make a facet value for a type value, which has an empty FacetType as its
+// type. Returns a constant value, whose instruction payload is a FacetValue.
+auto GetConstantFacetValueForType(Context& context,
+                                  SemIR::TypeInstId type_inst_id)
+    -> SemIR::ConstantId;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_FACET_TYPE_H_

+ 3 - 7
toolchain/check/handle_binding_pattern.cpp

@@ -302,7 +302,8 @@ auto HandleParseNode(Context& context, Parse::VarBindingPatternId node_id)
 }
 
 auto HandleParseNode(Context& context,
-                     Parse::CompileTimeBindingPatternStartId node_id) -> bool {
+                     Parse::CompileTimeBindingPatternStartId /*node_id*/)
+    -> bool {
   // Make a scope to contain the `.Self` facet value for use in the type of the
   // compile time binding. This is popped when handling the
   // CompileTimeBindingPatternId.
@@ -312,12 +313,7 @@ auto HandleParseNode(Context& context,
   // `FacetAccessType` when used in a type position, such as in `U:! I(.Self)`.
   // This allows substitution with other facet values without requiring an
   // additional `FacetAccessType` to be inserted.
-  SemIR::FacetTypeId facet_type_id =
-      context.facet_types().Add(SemIR::FacetTypeInfo{});
-  auto const_id = EvalOrAddInst<SemIR::FacetType>(
-      context, node_id,
-      {.type_id = SemIR::TypeType::TypeId, .facet_type_id = facet_type_id});
-  auto type_id = context.types().GetTypeIdForTypeConstantId(const_id);
+  auto type_id = GetEmptyFacetType(context);
 
   MakePeriodSelfFacetValue(context, type_id);
   return true;

+ 2 - 2
toolchain/check/handle_impl.cpp

@@ -228,8 +228,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id)
 
   // This requires that the facet type is identified. It returns None if an
   // error was diagnosed.
-  auto specific_interface = CheckConstraintIsInterface(context, impl_decl_id,
-                                                       constraint_type_inst_id);
+  auto specific_interface = CheckConstraintIsInterface(
+      context, impl_decl_id, self_type_inst_id, constraint_type_inst_id);
 
   auto impl_id = SemIR::ImplId::None;
   {

+ 20 - 26
toolchain/check/handle_require.cpp

@@ -109,11 +109,6 @@ auto HandleParseNode(Context& context, Parse::RequireTypeImplsId node_id)
 static auto TypeStructureReferencesSelf(
     Context& context, SemIR::TypeInstId inst_id,
     const SemIR::IdentifiedFacetType& identified_facet_type) -> bool {
-  if (inst_id == SemIR::ErrorInst::TypeInstId) {
-    // Don't generate more diagnostics.
-    return true;
-  }
-
   auto find_self = [&](SemIR::TypeIterator& type_iter) -> bool {
     while (true) {
       auto step = type_iter.Next();
@@ -147,11 +142,11 @@ static auto TypeStructureReferencesSelf(
     }
   }
 
-  if (identified_facet_type.required_interfaces().empty()) {
+  if (identified_facet_type.required_impls().empty()) {
     return false;
   }
 
-  for (auto specific_interface : identified_facet_type.required_interfaces()) {
+  for (auto [_, specific_interface] : identified_facet_type.required_impls()) {
     SemIR::TypeIterator type_iter(&context.sem_ir());
     type_iter.Add(specific_interface);
     if (!find_self(type_iter)) {
@@ -175,26 +170,35 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
                             SemIR::InstId constraint_inst_id,
                             SemIR::InstId scope_inst_id)
     -> std::optional<ValidateRequireResult> {
+  auto self_constant_value_id = context.constant_values().Get(self_inst_id);
   auto constraint_constant_value_id =
       context.constant_values().Get(constraint_inst_id);
+
+  if (self_constant_value_id == SemIR::ErrorInst::ConstantId ||
+      constraint_constant_value_id == SemIR::ErrorInst::ConstantId ||
+      scope_inst_id == SemIR::ErrorInst::InstId) {
+    // An error was already diagnosed, don't diagnose another. We can't build a
+    // useful `require` with an error, it couldn't do anything.
+    return std::nullopt;
+  }
+
   auto constraint_type_id =
       SemIR::TypeId::ForTypeConstant(constraint_constant_value_id);
   auto constraint_facet_type =
       context.types().TryGetAs<SemIR::FacetType>(constraint_type_id);
   if (!constraint_facet_type) {
-    if (constraint_constant_value_id != SemIR::ErrorInst::ConstantId) {
-      CARBON_DIAGNOSTIC(
-          RequireImplsMissingFacetType, Error,
-          "`require` declaration constrained by a non-facet type; "
-          "expected an `interface` or `constraint` name after `impls`");
-      context.emitter().Emit(constraint_inst_id, RequireImplsMissingFacetType);
-    }
+    CARBON_DIAGNOSTIC(
+        RequireImplsMissingFacetType, Error,
+        "`require` declaration constrained by a non-facet type; "
+        "expected an `interface` or `constraint` name after `impls`");
+    context.emitter().Emit(constraint_inst_id, RequireImplsMissingFacetType);
     // Can't continue without a constraint to use.
     return std::nullopt;
   }
 
   auto identified_facet_type_id = RequireIdentifiedFacetType(
-      context, SemIR::LocId(constraint_inst_id), *constraint_facet_type, [&] {
+      context, SemIR::LocId(constraint_inst_id), self_constant_value_id,
+      *constraint_facet_type, [&] {
         CARBON_DIAGNOSTIC(
             RequireImplsUnidentifiedFacetType, Error,
             "facet type {0} cannot be identified in `require` declaration",
@@ -220,16 +224,6 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
     return std::nullopt;
   }
 
-  if (scope_inst_id == SemIR::ErrorInst::InstId) {
-    // `require` is in the wrong scope.
-    return std::nullopt;
-  }
-  if (self_inst_id == SemIR::ErrorInst::InstId ||
-      constraint_inst_id == SemIR::ErrorInst::InstId) {
-    // Can't build a useful `require` with an error, it couldn't do anything.
-    return std::nullopt;
-  }
-
   return ValidateRequireResult{.constraint_type_id = constraint_type_id,
                                .identified_facet_type = &identified};
 }
@@ -265,7 +259,7 @@ auto HandleParseNode(Context& context, Parse::RequireDeclId node_id) -> bool {
   }
 
   auto [constraint_type_id, identified_facet_type] = *validated;
-  if (identified_facet_type->required_interfaces().empty()) {
+  if (identified_facet_type->required_impls().empty()) {
     // A `require T impls type` adds no actual constraints, so nothing to do.
     // This is not an error though.
     DiscardGenericDecl(context);

+ 12 - 4
toolchain/check/impl.cpp

@@ -671,13 +671,19 @@ auto CheckRequireDeclsSatisfied(Context& context, SemIR::Impl& impl) -> void {
   const auto& interface = context.interfaces().Get(impl.interface.interface_id);
   auto require_ids =
       context.require_impls_blocks().Get(interface.require_impls_block_id);
+  if (require_ids.empty()) {
+    return;
+  }
+
+  // Make a facet value for the self type.
+  auto self_facet_value = GetConstantFacetValueForType(context, impl.self_id);
+
   for (auto require_id : require_ids) {
     const auto& require = context.require_impls().Get(require_id);
 
     auto require_specific =
-        GetRequireImplsSpecificFromEnclosingSpecificWithSelfType(
-            context, require, impl.interface.specific_id, impl.self_id,
-            impl.witness_id);
+        GetRequireImplsSpecificFromEnclosingSpecificWithSelfFacetValue(
+            context, require, impl.interface.specific_id, self_facet_value);
     auto self_const_id = GetConstantValueInRequireImplsSpecific(
         context, require_specific, require.self_id);
     auto facet_type_const_id = GetConstantValueInRequireImplsSpecific(
@@ -740,6 +746,7 @@ auto IsImplEffectivelyFinal(Context& context, const SemIR::Impl& impl) -> bool {
 }
 
 auto CheckConstraintIsInterface(Context& context, SemIR::InstId impl_decl_id,
+                                SemIR::InstId self_id,
                                 SemIR::TypeInstId constraint_id)
     -> SemIR::SpecificInterface {
   auto facet_type_id = context.types().GetTypeIdForTypeInstId(constraint_id);
@@ -755,7 +762,8 @@ auto CheckConstraintIsInterface(Context& context, SemIR::InstId impl_decl_id,
   }
 
   auto identified_id = RequireIdentifiedFacetType(
-      context, SemIR::LocId(constraint_id), *facet_type, [&] {
+      context, SemIR::LocId(constraint_id),
+      context.constant_values().Get(self_id), *facet_type, [&] {
         CARBON_DIAGNOSTIC(ImplOfUnidentifiedFacetType, Error,
                           "facet type {0} cannot be identified in `impl as`",
                           InstIdAsType);

+ 1 - 0
toolchain/check/impl.h

@@ -88,6 +88,7 @@ auto CheckAssociatedFunctionImplementation(
 // Returns the interface that the impl implements. On error, issues a diagnostic
 // and returns `None`.
 auto CheckConstraintIsInterface(Context& context, SemIR::InstId impl_decl_id,
+                                SemIR::InstId self_id,
                                 SemIR::TypeInstId constraint_id)
     -> SemIR::SpecificInterface;
 

+ 71 - 47
toolchain/check/impl_lookup.cpp

@@ -199,25 +199,26 @@ static auto FindAndDiagnoseImplLookupCycle(
   return false;
 }
 
-struct InterfacesFromConstantId {
-  llvm::ArrayRef<SemIR::SpecificInterface> interfaces;
+struct RequiredImplsFromConstraint {
+  llvm::ArrayRef<SemIR::IdentifiedFacetType::RequiredImpl> req_impls;
   bool other_requirements;
 };
 
 // Gets the set of `SpecificInterface`s that are required by a facet type
 // (as a constant value), and any special requirements.
-static auto GetInterfacesFromConstantId(
+static auto GetRequiredImplsFromConstraint(
     Context& context, SemIR::LocId loc_id,
+    SemIR::ConstantId query_self_const_id,
     SemIR::ConstantId query_facet_type_const_id)
-    -> std::optional<InterfacesFromConstantId> {
+    -> std::optional<RequiredImplsFromConstraint> {
   auto facet_type_inst_id =
       context.constant_values().GetInstId(query_facet_type_const_id);
   auto facet_type_inst =
       context.insts().GetAs<SemIR::FacetType>(facet_type_inst_id);
   const auto& facet_type_info =
       context.facet_types().Get(facet_type_inst.facet_type_id);
-  auto identified_id =
-      RequireIdentifiedFacetType(context, loc_id, facet_type_inst, [&] {
+  auto identified_id = RequireIdentifiedFacetType(
+      context, loc_id, query_self_const_id, facet_type_inst, [&] {
         CARBON_DIAGNOSTIC(ImplLookupInUnidentifiedFacetType, Error,
                           "facet type {0} can not be identified", InstIdAsType);
         return context.emitter().Build(
@@ -226,10 +227,10 @@ static auto GetInterfacesFromConstantId(
   if (!identified_id.has_value()) {
     return std::nullopt;
   }
-  return {{.interfaces = context.identified_facet_types()
-                             .Get(identified_id)
-                             .required_interfaces(),
-           .other_requirements = facet_type_info.other_requirements}};
+  return {
+      {.req_impls =
+           context.identified_facet_types().Get(identified_id).required_impls(),
+       .other_requirements = facet_type_info.other_requirements}};
 }
 
 static auto GetWitnessIdForImpl(Context& context, SemIR::LocId loc_id,
@@ -344,25 +345,32 @@ static auto LookupImplWitnessInSelfFacetValue(
     return EvalImplLookupResult::MakeNone();
   }
 
-  // The position of the interface in `required_interfaces()` is also the
+  auto self_facet_value_const_id =
+      context.constant_values().Get(self_facet_value_inst_id);
+
+  // The position of the interface in `required_impls()` is also the
   // position of the witness for that interface in `FacetValue`. The
   // `FacetValue` witnesses are the output of an impl lookup, which finds and
   // returns witnesses in the same order.
-  auto identified_id =
-      RequireIdentifiedFacetType(context, loc_id, *facet_type, nullptr);
+  auto identified_id = RequireIdentifiedFacetType(
+      context, loc_id, self_facet_value_const_id, *facet_type, nullptr);
   // This should not be possible as FacetValue is constructed by a conversion
   // to a facet type, which performs impl lookup for that facet type, and
   // lookup only succeeds for identified facet types.
   CARBON_CHECK(identified_id.has_value(),
                "FacetValue was constructed with an unidentified facet type");
-  auto facet_type_required_interfaces =
-      llvm::enumerate(context.identified_facet_types()
-                          .Get(identified_id)
-                          .required_interfaces());
-  auto it = llvm::find_if(facet_type_required_interfaces, [=](auto e) {
-    return e.value() == query_specific_interface;
+  auto facet_type_req_impls = llvm::enumerate(
+      context.identified_facet_types().Get(identified_id).required_impls());
+  auto it = llvm::find_if(facet_type_req_impls, [&](auto e) {
+    auto [req_self, req_specific_interface] = e.value();
+    // The `self_facet_value_inst_id` in eval is a canonicalized facet value, so
+    // we need to do the same to `req_self` that comes from the
+    // IdentifiedFacetType in order to compare them.
+    auto canonical_req_self = GetCanonicalFacetOrTypeValue(context, req_self);
+    return canonical_req_self == self_facet_value_const_id &&
+           req_specific_interface == query_specific_interface;
   });
-  if (it == facet_type_required_interfaces.end()) {
+  if (it == facet_type_req_impls.end()) {
     return EvalImplLookupResult::MakeNone();
   }
   auto index = (*it).index();
@@ -390,11 +398,13 @@ class SubstWitnessesCallbacks : public SubstInstCallbacks {
   // `context` must not be null.
   explicit SubstWitnessesCallbacks(
       Context* context, SemIR::LocId loc_id,
-      llvm::ArrayRef<SemIR::SpecificInterface> interfaces,
+      SemIR::ConstantId query_self_const_id,
+      llvm::ArrayRef<SemIR::IdentifiedFacetType::RequiredImpl> req_impls,
       llvm::ArrayRef<SemIR::InstId> witness_inst_ids)
       : SubstInstCallbacks(context),
         loc_id_(loc_id),
-        interfaces_(interfaces),
+        query_self_const_id_(query_self_const_id),
+        req_impls_(req_impls),
         witness_inst_ids_(witness_inst_ids) {}
 
   auto Subst(SemIR::InstId& inst_id) -> SubstResult override {
@@ -483,29 +493,41 @@ class SubstWitnessesCallbacks : public SubstInstCallbacks {
       -> SemIR::InstId {
     auto lookup_query_interface =
         context().specific_interfaces().Get(specific_interface_id);
-    for (auto [interface, witness_inst_id] :
-         llvm::zip_equal(interfaces_, witness_inst_ids_)) {
+    for (auto [req_impl, witness_inst_id] :
+         llvm::zip_equal(req_impls_, witness_inst_ids_)) {
+      auto [req_self, req_interface] = req_impl;
+      // The `LookupImplWitness` is for `.Self`, so if the witness is for some
+      // type other than the query self, we can't use it for `.Self`.
+      if (req_self != query_self_const_id_) {
+        continue;
+      }
       // If the `LookupImplWitness` for `.Self` is not looking for the same
       // interface as we have a witness for, this is not the right witness to
       // use to replace the lookup for `.Self`.
-      if (interface.interface_id == lookup_query_interface.interface_id) {
-        return witness_inst_id;
+      if (req_interface.interface_id != lookup_query_interface.interface_id) {
+        continue;
       }
+      return witness_inst_id;
     }
     return SemIR::InstId::None;
   }
 
   SemIR::LocId loc_id_;
-  llvm::ArrayRef<SemIR::SpecificInterface> interfaces_;
+  SemIR::ConstantId query_self_const_id_;
+  llvm::ArrayRef<SemIR::IdentifiedFacetType::RequiredImpl> req_impls_;
   llvm::ArrayRef<SemIR::InstId> witness_inst_ids_;
   int facet_type_depth_ = 0;
 };
 
 static auto VerifyQueryFacetTypeConstraints(
     Context& context, SemIR::LocId loc_id,
-    SemIR::InstId query_facet_type_inst_id,
-    llvm::ArrayRef<SemIR::SpecificInterface> interfaces,
+    SemIR::ConstantId query_self_const_id,
+    SemIR::ConstantId query_facet_type_const_id,
+    llvm::ArrayRef<SemIR::IdentifiedFacetType::RequiredImpl> req_impls,
     llvm::ArrayRef<SemIR::InstId> witness_inst_ids) -> bool {
+  SemIR::InstId query_facet_type_inst_id =
+      context.constant_values().GetInstId(query_facet_type_const_id);
+
   CARBON_CHECK(context.insts().Is<SemIR::FacetType>(query_facet_type_inst_id));
 
   const auto& facet_type_info = context.facet_types().Get(
@@ -514,8 +536,8 @@ static auto VerifyQueryFacetTypeConstraints(
           .facet_type_id);
 
   if (!facet_type_info.rewrite_constraints.empty()) {
-    auto callbacks =
-        SubstWitnessesCallbacks(&context, loc_id, interfaces, witness_inst_ids);
+    auto callbacks = SubstWitnessesCallbacks(
+        &context, loc_id, query_self_const_id, req_impls, witness_inst_ids);
 
     for (const auto& rewrite : facet_type_info.rewrite_constraints) {
       auto lhs_id = SubstInst(context, rewrite.lhs_id, callbacks);
@@ -585,17 +607,17 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
         context.constant_values().GetInstId(query_facet_type_const_id)));
   }
 
-  auto interfaces_from_constant_id =
-      GetInterfacesFromConstantId(context, loc_id, query_facet_type_const_id);
-  if (!interfaces_from_constant_id) {
+  auto req_impls_from_constraint = GetRequiredImplsFromConstraint(
+      context, loc_id, query_self_const_id, query_facet_type_const_id);
+  if (!req_impls_from_constraint) {
     return SemIR::InstBlockIdOrError::MakeError();
   }
-  auto [interfaces, other_requirements] = *interfaces_from_constant_id;
+  auto [req_impls, other_requirements] = *req_impls_from_constraint;
   if (other_requirements) {
     // TODO: Remove this when other requirements go away.
     return SemIR::InstBlockId::None;
   }
-  if (interfaces.empty()) {
+  if (req_impls.empty()) {
     return SemIR::InstBlockId::Empty;
   }
 
@@ -610,15 +632,18 @@ 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`. Every
-  // consumer of a facet type needs to agree on the order of interfaces used for
-  // its witnesses.
+  // We need to find a witness for each self+interface pair in `req_impls`.
+  //
+  // Every consumer of a facet type needs to agree on the order of interfaces
+  // used for its witnesses, which is done by following the order in the
+  // IdentifiedFacetType.
   llvm::SmallVector<SemIR::InstId> result_witness_ids;
-  for (const auto& interface : interfaces) {
+  for (const auto& req_impl : req_impls) {
     // 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.
-    auto result_witness_id = GetOrAddLookupImplWitness(
-        context, loc_id, query_self_const_id, interface);
+    auto result_witness_id =
+        GetOrAddLookupImplWitness(context, loc_id, req_impl.self_facet_value,
+                                  req_impl.specific_interface);
     if (result_witness_id.has_value()) {
       result_witness_ids.push_back(result_witness_id);
     } else {
@@ -632,16 +657,15 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
   // All interfaces in the query facet type must have been found to be available
   // through some impl, or directly on the value's facet type if
   // `query_self_const_id` is a facet value.
-  if (result_witness_ids.size() != interfaces.size()) {
+  if (result_witness_ids.size() != req_impls.size()) {
     return SemIR::InstBlockId::None;
   }
 
   // Verify rewrite constraints in the query constraint are satisfied after
   // applying the rewrites from the found witnesses.
-  if (!VerifyQueryFacetTypeConstraints(
-          context, loc_id,
-          context.constant_values().GetInstId(query_facet_type_const_id),
-          interfaces, result_witness_ids)) {
+  if (!VerifyQueryFacetTypeConstraints(context, loc_id, query_self_const_id,
+                                       query_facet_type_const_id, req_impls,
+                                       result_witness_ids)) {
     return SemIR::InstBlockId::None;
   }
 

+ 1 - 1
toolchain/check/import_ref.cpp

@@ -2625,7 +2625,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
   if (auto facet_type = resolver.local_insts().TryGetAs<SemIR::FacetType>(
           resolver.local_constant_values().GetInstId(constraint_const_id))) {
     RequireIdentifiedFacetType(resolver.local_context(), SemIR::LocId::None,
-                               *facet_type, nullptr);
+                               self_const_id, *facet_type, nullptr);
   }
   if (import_impl.is_complete()) {
     AddImplDefinition(resolver, import_impl, new_impl);

+ 13 - 1
toolchain/check/interface.cpp

@@ -81,7 +81,19 @@ static auto GetSelfBinding(Context& context,
   return self_binding_id;
 }
 
-auto GetSelfFacetValueForInterfaceMemberSpecific(
+// Construct a facet value that can be used for the `Self` binding of entities
+// inside an interface.
+//
+// The `interface_specific_id` is the specific of the interface around the
+// `Self`. The `generic_id` is for member of the interface that the `Self` value
+// will be for. The `self_witness_id` is an impl witness for the interface that
+// the `self_type_id` implements that interface. It should come from an impl
+// definition with the given self-type and the interface as its constraint.
+//
+// The returned facet value can be used as the `Self` value in a specific for
+// the generic member of the interface, and can appear in its specific. As such,
+// this is a building block of GetSelfSpecificForInterfaceMemberWithSelfType.
+static auto GetSelfFacetValueForInterfaceMemberSpecific(
     Context& context, SemIR::SpecificId interface_specific_id,
     SemIR::GenericId generic_id, SemIR::TypeId self_type_id,
     SemIR::InstId self_witness_id) -> SemIR::InstId {

+ 0 - 17
toolchain/check/interface.h

@@ -25,23 +25,6 @@ namespace Carbon::Check {
 auto BuildAssociatedEntity(Context& context, SemIR::InterfaceId interface_id,
                            SemIR::InstId decl_id) -> SemIR::InstId;
 
-// Construct a facet value that can be used for the `Self` binding of entities
-// inside an interface.
-//
-// The `interface_specific_id` is the specific of the interface around the
-// `Self`. The `generic_id` is for member of the interface that the `Self` value
-// will be for. The `self_witness_id` is an impl witness for the interface that
-// the `self_type_id` implements that interface. It should come from an impl
-// definition with the given self-type and the interface as its constraint.
-//
-// The returned facet value can be used as the `Self` value in a specific for
-// the generic member of the interface, and can appear in its specific. As such,
-// this is a building block of GetSelfSpecificForInterfaceMemberWithSelfType.
-auto GetSelfFacetValueForInterfaceMemberSpecific(
-    Context& context, SemIR::SpecificId interface_specific_id,
-    SemIR::GenericId generic_id, SemIR::TypeId self_type_id,
-    SemIR::InstId self_witness_id) -> SemIR::InstId;
-
 // Gets the self specific of a generic declaration that is an interface member,
 // given a specific for the interface plus a type to use as `Self`.
 auto GetSelfSpecificForInterfaceMemberWithSelfType(

+ 11 - 18
toolchain/check/require_impls.cpp

@@ -5,8 +5,10 @@
 #include "toolchain/check/require_impls.h"
 
 #include "toolchain/check/generic.h"
+#include "toolchain/check/inst.h"
 #include "toolchain/check/interface.h"
 #include "toolchain/sem_ir/ids.h"
+#include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Check {
 
@@ -68,28 +70,19 @@ auto GetRequireImplsSpecificFromEnclosingSpecific(
   return {.specific_id = specific_id};
 }
 
-auto GetRequireImplsSpecificFromEnclosingSpecificWithSelfType(
+auto GetRequireImplsSpecificFromEnclosingSpecificWithSelfFacetValue(
     Context& context, const SemIR::RequireImpls& require,
-    SemIR::SpecificId enclosing_specific_id, SemIR::TypeInstId self_id,
-    SemIR::InstId witness_id) -> RequireImplsSpecific {
-  if (enclosing_specific_id.has_value()) {
-    auto enclosing_generic_decl =
-        GetEnclosingDeclFromEnclosingSpecificId(context, enclosing_specific_id);
-    CARBON_CHECK(enclosing_generic_decl.Is<SemIR::InterfaceDecl>(),
-                 "Incorrect enclosing specific for RequireImpls with explicit "
-                 "self type. Expected an interface. Found {0}.",
-                 enclosing_generic_decl);
-  }
-
-  // Construct a facet value around the `self_id` type of the correct facet
-  // type for the `Self` in the require's self-specific.
-  auto self_facet_value = GetSelfFacetValueForInterfaceMemberSpecific(
-      context, enclosing_specific_id, require.generic_id,
-      context.types().GetTypeIdForTypeInstId(self_id), witness_id);
+    SemIR::SpecificId enclosing_specific_id,
+    SemIR::ConstantId self_facet_value_id) -> RequireImplsSpecific {
+  auto self_facet_value_inst_id =
+      context.constant_values().GetInstId(self_facet_value_id);
+  auto self_facet_value = context.insts().Get(self_facet_value_inst_id);
+  CARBON_CHECK(context.types().Is<SemIR::FacetType>(self_facet_value.type_id()),
+               "{0}", self_facet_value);
 
   auto arg_ids =
       GetSpecificArgsFromEnclosingSpecific(context, enclosing_specific_id);
-  arg_ids.push_back(self_facet_value);
+  arg_ids.push_back(self_facet_value_inst_id);
 
   auto specific_id = MakeSpecific(context, SemIR::LocId(require.decl_id),
                                   require.generic_id, arg_ids);

+ 4 - 12
toolchain/check/require_impls.h

@@ -26,19 +26,11 @@ auto GetRequireImplsSpecificFromEnclosingSpecific(
     SemIR::SpecificId enclosing_specific_id) -> RequireImplsSpecific;
 
 // Like GetRequireImplsSpecificFromEnclosingSpecific but the `Self` value in the
-// specific is replaced by a facet value.
-//
-// The self facet value must have a facet type to match the type of `Self` in
-// specific, which will be a facet type for the enclosing interface. The self
-// type comes from `self_id`, and the `witness_id` must be an impl witness for
-// the enclosing interface.
-//
-// This is only possible for an enclosing interface since we do not ever have an
-// impl witness for a named constraint.
-auto GetRequireImplsSpecificFromEnclosingSpecificWithSelfType(
+// specific is replaced by a given facet value.
+auto GetRequireImplsSpecificFromEnclosingSpecificWithSelfFacetValue(
     Context& context, const SemIR::RequireImpls& require,
-    SemIR::SpecificId enclosing_specific_id, SemIR::TypeInstId self_id,
-    SemIR::InstId witness_id) -> RequireImplsSpecific;
+    SemIR::SpecificId enclosing_specific_id,
+    SemIR::ConstantId self_facet_value_id) -> RequireImplsSpecific;
 
 // Returns the constant value of `inst_id` from inside a RequireImpls
 // declaration, mapped into `enclosing_specific_id`. If an error results, it

+ 2 - 10
toolchain/check/testdata/generic/identify_specific_facet_type.carbon

@@ -27,14 +27,10 @@ constraint J(N:! i32) {
   require impls K(array(i32, N));
 }
 
-// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+8]]:12: note: identifying facet type `J(-1)` here [IdentifyingFacetTypeHere]
+// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+4]]:12: note: identifying facet type `J(-1)` here [IdentifyingFacetTypeHere]
 // CHECK:STDERR: impl {} as J(-1) {}
 // CHECK:STDERR:            ^~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+4]]:1: error: impl as 0 interfaces, expected 1 [ImplOfNotOneInterface]
-// CHECK:STDERR: impl {} as J(-1) {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
 impl {} as J(-1) {}
 
 // --- fail_error_in_identifying_extend_require_facet_type.carbon
@@ -56,12 +52,8 @@ constraint J(N:! i32) {
   extend require impls K(array(i32, N));
 }
 
-// CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+8]]:12: note: identifying facet type `J(-1)` here [IdentifyingFacetTypeHere]
+// CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+4]]:12: note: identifying facet type `J(-1)` here [IdentifyingFacetTypeHere]
 // CHECK:STDERR: impl {} as J(-1) {}
 // CHECK:STDERR:            ^~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_error_in_identifying_extend_require_facet_type.carbon:[[@LINE+4]]:1: error: impl as 0 interfaces, expected 1 [ImplOfNotOneInterface]
-// CHECK:STDERR: impl {} as J(-1) {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
 impl {} as J(-1) {}

+ 37 - 15
toolchain/check/testdata/impl/import_generic.carbon

@@ -327,7 +327,8 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %J.impl_witness: <witness> = impl_witness @C.as.J.impl.%J.impl_witness_table [concrete]
 // CHECK:STDOUT:   %Self.9e4: %J.type.d6f = symbolic_binding Self, 1 [symbolic]
 // CHECK:STDOUT:   %complete_type.5b1: <witness> = complete_type_witness %I.type.ab5 [concrete]
-// CHECK:STDOUT:   %J.facet: %J.type.d6f = facet_value %C, (%J.impl_witness) [concrete]
+// CHECK:STDOUT:   %type: type = facet_type <type> [concrete]
+// CHECK:STDOUT:   %C.type.facet: %type = facet_value %C, () [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -475,10 +476,10 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %require_complete => constants.%complete_type.5b1
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @J.require50000000(constants.%empty_struct_type, constants.%J.facet) {
+// CHECK:STDOUT: specific @J.require50000000(constants.%empty_struct_type, constants.%C.type.facet) {
 // CHECK:STDOUT:   %T => constants.%empty_struct_type
 // CHECK:STDOUT:   %J.type => constants.%J.type.d6f
-// CHECK:STDOUT:   %Self => constants.%J.facet
+// CHECK:STDOUT:   %Self => constants.%C.type.facet
 // CHECK:STDOUT:   %Self.binding.as_type => constants.%C
 // CHECK:STDOUT:   %I.type => constants.%I.type.ab5
 // CHECK:STDOUT: }
@@ -616,6 +617,8 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %require_complete: <witness> = require_complete_type %I.type.1ab [symbolic]
 // CHECK:STDOUT:   %empty_struct.a40: %empty_struct_type = struct_value () [concrete]
 // CHECK:STDOUT:   %J.type.d682d6.2: type = facet_type <@J, @J(%empty_struct_type)> [concrete]
+// CHECK:STDOUT:   %type: type = facet_type <type> [concrete]
+// CHECK:STDOUT:   %C.type.facet: %type = facet_value %C, () [concrete]
 // CHECK:STDOUT:   %I.type.ab5: type = facet_type <@I, @I(%empty_struct_type)> [concrete]
 // CHECK:STDOUT:   %I.impl_witness: <witness> = impl_witness @C.as.I.impl.%I.impl_witness_table [concrete]
 // CHECK:STDOUT:   %Self.e1f642.2: %J.type.d682d6.2 = symbolic_binding Self, 1 [symbolic]
@@ -744,11 +747,11 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %require_complete => constants.%complete_type.5b1
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @J.require48000000(constants.%empty_struct_type, constants.%Self.e1f642.1) {
+// CHECK:STDOUT: specific @J.require48000000(constants.%empty_struct_type, constants.%C.type.facet) {
 // CHECK:STDOUT:   %T => constants.%empty_struct_type
 // CHECK:STDOUT:   %J.type => constants.%J.type.d682d6.2
-// CHECK:STDOUT:   %Self => constants.%Self.e1f642.1
-// CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
+// CHECK:STDOUT:   %Self => constants.%C.type.facet
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%C
 // CHECK:STDOUT:   %I.type => constants.%I.type.ab5
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -1008,6 +1011,7 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %N.type.d682d6.1: type = facet_type <@N, @N(%T)> [symbolic]
 // CHECK:STDOUT:   %Self.e1f: %N.type.d682d6.1 = symbolic_binding Self, 1 [symbolic]
 // CHECK:STDOUT:   %Self.binding.as_type: type = symbolic_binding_type Self, 1, %Self.e1f [symbolic]
+// CHECK:STDOUT:   %C.type.facet: %type = facet_value %C, () [concrete]
 // CHECK:STDOUT:   %N.type.d682d6.2: type = facet_type <@N, @N(%ptr)> [symbolic]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -1308,6 +1312,14 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %I.type => constants.%I.type.1ab
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: specific @N.require58000000(constants.%T, constants.%C.type.facet) {
+// CHECK:STDOUT:   %T => constants.%T
+// CHECK:STDOUT:   %N.type => constants.%N.type.d682d6.1
+// CHECK:STDOUT:   %Self => constants.%C.type.facet
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%C
+// CHECK:STDOUT:   %I.type => constants.%I.type.1ab
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: specific @C.as.I.impl.c9332a.1(constants.%T) {
 // CHECK:STDOUT:   %T.loc32_14.2 => constants.%T
 // CHECK:STDOUT:   %N.type.loc32_32.2 => constants.%N.type.d682d6.1
@@ -1317,11 +1329,11 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %T => constants.%ptr
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @N.require58000000(constants.%ptr, constants.%Self.e1f) {
+// CHECK:STDOUT: specific @N.require58000000(constants.%ptr, constants.%C.type.facet) {
 // CHECK:STDOUT:   %T => constants.%ptr
 // CHECK:STDOUT:   %N.type => constants.%N.type.d682d6.2
-// CHECK:STDOUT:   %Self => constants.%Self.e1f
-// CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
+// CHECK:STDOUT:   %Self => constants.%C.type.facet
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%C
 // CHECK:STDOUT:   %I.type => constants.%I.type.e19
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -1522,6 +1534,7 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %Self.binding.as_type: type = symbolic_binding_type Self, 1, %Self.e1f642.1 [symbolic]
 // CHECK:STDOUT:   %ptr.125: type = ptr_type %ptr.e8f [symbolic]
 // CHECK:STDOUT:   %N.type.d682d6.2: type = facet_type <@N, @N(%ptr.125)> [symbolic]
+// CHECK:STDOUT:   %C.type.facet: %type = facet_value %C, () [concrete]
 // CHECK:STDOUT:   %I.type.08a: type = facet_type <@I, @I(%ptr.125)> [symbolic]
 // CHECK:STDOUT:   %I.impl_witness.d57: <witness> = impl_witness @C.as.I.impl.c93.%I.impl_witness_table, @C.as.I.impl.c93(%T) [symbolic]
 // CHECK:STDOUT:   %Self.e1f642.2: %N.type.d682d6.2 = symbolic_binding Self, 1 [symbolic]
@@ -1736,11 +1749,11 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %require_complete => constants.%require_complete.ea6
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @N.require44000000(constants.%ptr.125, constants.%Self.e1f642.1) {
+// CHECK:STDOUT: specific @N.require44000000(constants.%ptr.125, constants.%C.type.facet) {
 // CHECK:STDOUT:   %T => constants.%ptr.125
 // CHECK:STDOUT:   %N.type => constants.%N.type.d682d6.2
-// CHECK:STDOUT:   %Self => constants.%Self.e1f642.1
-// CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
+// CHECK:STDOUT:   %Self => constants.%C.type.facet
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%C
 // CHECK:STDOUT:   %I.type => constants.%I.type.08a
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -1968,6 +1981,7 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %Self.e1f: %N.type.d682d6.1 = symbolic_binding Self, 1 [symbolic]
 // CHECK:STDOUT:   %Self.binding.as_type: type = symbolic_binding_type Self, 1, %Self.e1f [symbolic]
 // CHECK:STDOUT:   %require_complete: <witness> = require_complete_type %J.type.04e [symbolic]
+// CHECK:STDOUT:   %D.type.facet: %type = facet_value %D, () [concrete]
 // CHECK:STDOUT:   %N.type.d682d6.2: type = facet_type <@N, @N(%ptr)> [symbolic]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -2260,6 +2274,14 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %J.type => constants.%J.type.04e
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: specific @N.require54000000(constants.%T, constants.%D.type.facet) {
+// CHECK:STDOUT:   %T => constants.%T
+// CHECK:STDOUT:   %N.type => constants.%N.type.d682d6.1
+// CHECK:STDOUT:   %Self => constants.%D.type.facet
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%D
+// CHECK:STDOUT:   %J.type => constants.%J.type.04e
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: specific @D.as.J.impl.e28c26.1(constants.%T) {
 // CHECK:STDOUT:   %T.loc32_14.2 => constants.%T
 // CHECK:STDOUT:   %N.type.loc32_32.2 => constants.%N.type.d682d6.1
@@ -2269,11 +2291,11 @@ impl forall [T:! type] D as N(T*) {}
 // CHECK:STDOUT:   %T => constants.%ptr
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: specific @N.require54000000(constants.%ptr, constants.%Self.e1f) {
+// CHECK:STDOUT: specific @N.require54000000(constants.%ptr, constants.%D.type.facet) {
 // CHECK:STDOUT:   %T => constants.%ptr
 // CHECK:STDOUT:   %N.type => constants.%N.type.d682d6.2
-// CHECK:STDOUT:   %Self => constants.%Self.e1f
-// CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
+// CHECK:STDOUT:   %Self => constants.%D.type.facet
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%D
 // CHECK:STDOUT:   %J.type => constants.%J.type.80f
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 5 - 0
toolchain/check/testdata/impl/incomplete.carbon

@@ -866,6 +866,11 @@ interface B {
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @X1(constants.%Self.e52) {}
 // CHECK:STDOUT:
+// CHECK:STDOUT: specific @Y.require54000000(constants.%Self.e52) {
+// CHECK:STDOUT:   %Self => constants.%Self.e52
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type.806
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: specific @X.require54000001(constants.%Self.e52) {
 // CHECK:STDOUT:   %Self => constants.%Self.e52
 // CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type.806

+ 155 - 0
toolchain/check/testdata/named_constraint/require.carbon

@@ -285,6 +285,140 @@ constraint Z {
 }
 //@dump-sem-ir-end
 
+// --- require_self.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint N {
+  require impls Z(Self);
+}
+
+impl () as Z(()) {}
+
+fn F() {
+  () as N;
+}
+
+// --- fail_require_different_self.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint N {
+  require impls Z(Self);
+}
+
+impl () as Z({}) {}
+
+fn F() {
+  // N requires () impls Z(Self) which is Z(()) for this query, but it impls
+  // Z({}).
+  //
+  // CHECK:STDERR: fail_require_different_self.carbon:[[@LINE+4]]:3: error: cannot convert type `()` into type implementing `N` [ConversionFailureTypeToFacet]
+  // CHECK:STDERR:   () as N;
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  () as N;
+}
+
+// --- fail_require_different_parameter.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint N {
+  require impls Z({});
+}
+
+impl () as Z(()) {}
+
+fn F() {
+  // N requires () impls Z({}) but it impls Z(()).
+  //
+  // CHECK:STDERR: fail_require_different_parameter.carbon:[[@LINE+4]]:3: error: cannot convert type `()` into type implementing `N` [ConversionFailureTypeToFacet]
+  // CHECK:STDERR:   () as N;
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  () as N;
+}
+
+// --- require_different_type_impls.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint N {
+  require {} impls Z(Self);
+}
+
+impl {} as Z(()) {}
+
+fn F() {
+  () as N;
+}
+
+// --- fail_require_different_type_impls_different_parameter.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint N {
+  require {} impls Z(Self);
+}
+
+impl {} as Z({}) {}
+
+fn F() {
+  // N requires {} impls Z(Self) which is Z(()) for this query, but it impls
+  // Z({}).
+  //
+  // CHECK:STDERR: fail_require_different_type_impls_different_parameter.carbon:[[@LINE+4]]:3: error: cannot convert type `()` into type implementing `N` [ConversionFailureTypeToFacet]
+  // CHECK:STDERR:   () as N;
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  () as N;
+}
+
+// --- require_with_rewrite_constraint.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {
+  let X:! type;
+}
+
+constraint N {
+  require {} impls Z(Self) where .X = Self;
+}
+
+// Will satisfy `() as N`.
+impl {} as Z(()) where .X = () {}
+
+fn F() {
+  () as N;
+}
+
+// --- todo_fail_require_with_mismatching_rewrite_constraint.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {
+  let X:! type;
+}
+
+constraint N {
+  require {} impls Z(Self) where .X = Self;
+}
+
+// Will not satisfy `() as N` as the rewrite does not match.
+impl {} as Z(()) where .X = {} {}
+
+fn F() {
+  // TODO: Should fail, but we're currently checking the requirements of `N`
+  // rather than of the identified facet type of `N` (which would include the
+  // require decl's requirements).
+  () as N;
+}
+
 // CHECK:STDOUT: --- fail_todo_extend.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -292,6 +426,8 @@ constraint Z {
 // CHECK:STDOUT:   %Z.type: type = facet_type <@Z> [concrete]
 // CHECK:STDOUT:   %Self.550: %Z.type = symbolic_binding Self, 0 [symbolic]
 // CHECK:STDOUT:   %Self.binding.as_type: type = symbolic_binding_type Self, 0, %Self.550 [symbolic]
+// CHECK:STDOUT:   %T: %Z.type = symbolic_binding T, 0 [symbolic]
+// CHECK:STDOUT:   %T.binding.as_type: type = symbolic_binding_type T, 0, %T [symbolic]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -330,6 +466,11 @@ constraint Z {
 // CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: specific @Z.require60000000(constants.%T) {
+// CHECK:STDOUT:   %Self => constants.%T
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%T.binding.as_type
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- implicit_self_impls.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -337,6 +478,8 @@ constraint Z {
 // CHECK:STDOUT:   %Z.type: type = facet_type <@Z> [concrete]
 // CHECK:STDOUT:   %Self.550: %Z.type = symbolic_binding Self, 0 [symbolic]
 // CHECK:STDOUT:   %Self.binding.as_type: type = symbolic_binding_type Self, 0, %Self.550 [symbolic]
+// CHECK:STDOUT:   %T: %Z.type = symbolic_binding T, 0 [symbolic]
+// CHECK:STDOUT:   %T.binding.as_type: type = symbolic_binding_type T, 0, %T [symbolic]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -375,6 +518,11 @@ constraint Z {
 // CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: specific @Z.require50000000(constants.%T) {
+// CHECK:STDOUT:   %Self => constants.%T
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%T.binding.as_type
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- explicit_self_impls.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -382,6 +530,8 @@ constraint Z {
 // CHECK:STDOUT:   %Z.type: type = facet_type <@Z> [concrete]
 // CHECK:STDOUT:   %Self.550: %Z.type = symbolic_binding Self, 0 [symbolic]
 // CHECK:STDOUT:   %Self.binding.as_type: type = symbolic_binding_type Self, 0, %Self.550 [symbolic]
+// CHECK:STDOUT:   %T: %Z.type = symbolic_binding T, 0 [symbolic]
+// CHECK:STDOUT:   %T.binding.as_type: type = symbolic_binding_type T, 0, %T [symbolic]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -422,6 +572,11 @@ constraint Z {
 // CHECK:STDOUT:   %Self.binding.as_type => constants.%Self.binding.as_type
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: specific @Z.require70000000(constants.%T) {
+// CHECK:STDOUT:   %Self => constants.%T
+// CHECK:STDOUT:   %Self.binding.as_type => constants.%T.binding.as_type
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- explicit_self_specific_impls.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {

+ 110 - 44
toolchain/check/type_completion.cpp

@@ -8,6 +8,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/cpp/import.h"
+#include "toolchain/check/facet_type.h"
 #include "toolchain/check/generic.h"
 #include "toolchain/check/inst.h"
 #include "toolchain/check/literal.h"
@@ -15,6 +16,7 @@
 #include "toolchain/check/type.h"
 #include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/constant.h"
+#include "toolchain/sem_ir/facet_type_info.h"
 #include "toolchain/sem_ir/generic.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/specific_interface.h"
@@ -805,71 +807,98 @@ static auto RequireIdentifiedNamedConstraints(
   return true;
 }
 
-// Returns the `facet_type` mapped into `specific_id`. If an error results, it
-// returns None. In particular, this can surface as a monomorphization error
-// where the facet type was valid as a symbolic but becomes invalid with some
-// concrete specific.
-static auto TryGetRequireImplsFacetTypeInEnclosingSpecific(
-    Context& context, const SemIR::RequireImpls& require,
-    SemIR::SpecificId enclosing_specific_id) -> SemIR::FacetTypeId {
-  auto require_specific = GetRequireImplsSpecificFromEnclosingSpecific(
-      context, require, enclosing_specific_id);
-  auto const_id = GetConstantValueInRequireImplsSpecific(
-      context, require_specific, require.facet_type_inst_id);
-  if (auto facet_type_in_specific = context.insts().TryGetAs<SemIR::FacetType>(
-          context.constant_values().GetInstId(const_id))) {
-    return facet_type_in_specific->facet_type_id;
+static auto GetSelfFacetValue(Context& context, SemIR::ConstantId self_const_id)
+    -> SemIR::ConstantId {
+  if (self_const_id == SemIR::ErrorInst::ConstantId) {
+    return SemIR::ErrorInst::ConstantId;
   }
-  // An ErrorInst was encountered.
-  return SemIR::FacetTypeId::None;
+
+  // Avoid wrapping a FacetAccessType(FacetValue) in another layer of
+  // FacetValue. Just unwrap the FacetValue inside.
+  self_const_id = GetCanonicalFacetOrTypeValue(context, self_const_id);
+
+  auto self_inst_id = context.constant_values().GetInstId(self_const_id);
+  auto type_id = context.insts().Get(self_inst_id).type_id();
+  CARBON_CHECK(context.types().IsFacetType(type_id));
+
+  if (context.types().Is<SemIR::FacetType>(type_id)) {
+    return self_const_id;
+  }
+
+  return GetConstantFacetValueForType(
+      context, context.types().GetAsTypeInstId(self_inst_id));
 }
 
 auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
+                                SemIR::ConstantId self_const_id,
                                 const SemIR::FacetType& facet_type,
                                 MakeDiagnosticBuilderFn diagnoser)
     -> SemIR::IdentifiedFacetTypeId {
-  if (auto identified_id =
-          context.identified_facet_types().TryGetId(facet_type.facet_type_id);
+  auto key =
+      SemIR::IdentifiedFacetTypeKey{.facet_type_id = facet_type.facet_type_id,
+                                    .self_const_id = self_const_id};
+  if (auto identified_id = context.identified_facet_types().Lookup(key);
       identified_id.has_value()) {
     return identified_id;
   }
 
+  struct SelfImplsFacetType {
+    SemIR::ConstantId self;
+    SemIR::FacetTypeId facet_type;
+  };
+
   // Work queue.
-  llvm::SmallVector<SemIR::FacetTypeId> extend_facet_types = {
-      facet_type.facet_type_id};
-  llvm::SmallVector<SemIR::FacetTypeId> impls_facet_types;
+  llvm::SmallVector<SelfImplsFacetType> extend_facet_types = {
+      {self_const_id, facet_type.facet_type_id}};
+  llvm::SmallVector<SelfImplsFacetType> impls_facet_types;
 
   // Outputs for the IdentifiedFacetType.
-  llvm::SmallVector<SemIR::SpecificInterface> extends;
-  llvm::SmallVector<SemIR::SpecificInterface> self_impls;
+  llvm::SmallVector<SemIR::IdentifiedFacetType::RequiredImpl> extends;
+  llvm::SmallVector<SemIR::IdentifiedFacetType::RequiredImpl> impls;
 
   while (true) {
-    auto next_facet_type_id = SemIR::FacetTypeId::None;
+    SelfImplsFacetType next_impls = {SemIR::ConstantId::None,
+                                     SemIR::FacetTypeId::None};
     bool facet_type_extends = false;
     if (!extend_facet_types.empty()) {
-      next_facet_type_id = extend_facet_types.pop_back_val();
+      next_impls = extend_facet_types.pop_back_val();
       facet_type_extends = true;
     } else if (!impls_facet_types.empty()) {
-      next_facet_type_id = impls_facet_types.pop_back_val();
+      next_impls = impls_facet_types.pop_back_val();
       facet_type_extends = false;
     } else {
       break;
     }
 
-    const auto& facet_type_info = context.facet_types().Get(next_facet_type_id);
+    auto self_const_id = next_impls.self;
+    const auto& facet_type_info =
+        context.facet_types().Get(next_impls.facet_type);
 
     if (!RequireIdentifiedNamedConstraints(context, facet_type_info,
                                            diagnoser)) {
       return SemIR::IdentifiedFacetTypeId::None;
     }
 
+    auto self_and_interface = [&](SemIR::SpecificInterface interface)
+        -> SemIR::IdentifiedFacetType::RequiredImpl {
+      return {self_const_id, interface};
+    };
+
     if (facet_type_extends) {
-      llvm::append_range(extends, facet_type_info.extend_constraints);
+      llvm::append_range(extends,
+                         llvm::map_range(facet_type_info.extend_constraints,
+                                         self_and_interface));
     } else {
-      llvm::append_range(self_impls, facet_type_info.extend_constraints);
+      llvm::append_range(impls,
+                         llvm::map_range(facet_type_info.extend_constraints,
+                                         self_and_interface));
     }
-    llvm::append_range(self_impls, facet_type_info.self_impls_constraints);
+    llvm::append_range(impls,
+                       llvm::map_range(facet_type_info.self_impls_constraints,
+                                       self_and_interface));
 
+    // Constructing specifics for `require` decls can produce monomorphization
+    // errors, which we want to connect back to here.
     Diagnostics::AnnotationScope annotate_diagnostics(
         &context.emitter(), [&](auto& builder) {
           CARBON_DIAGNOSTIC(IdentifyingFacetTypeHere, Note,
@@ -879,20 +908,44 @@ auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
                        facet_type.facet_type_id);
         });
 
+    if (facet_type_info.extend_named_constraints.empty() &&
+        facet_type_info.self_impls_named_constraints.empty()) {
+      continue;
+    }
+
+    // The self may have type TypeType. But the `Self` in a generic require decl
+    // has type FacetType, so we need something similar to replace it in the
+    // specific.
+    auto self_facet_value = GetSelfFacetValue(context, self_const_id);
+
     for (auto extends : facet_type_info.extend_named_constraints) {
       const auto& constraint =
           context.named_constraints().Get(extends.named_constraint_id);
       for (auto require_impls_id : context.require_impls_blocks().Get(
                constraint.require_impls_block_id)) {
         const auto& require = context.require_impls().Get(require_impls_id);
-        auto facet_type_id = TryGetRequireImplsFacetTypeInEnclosingSpecific(
-            context, require, extends.specific_id);
-        if (facet_type_id.has_value()) {
-          if (facet_type_extends && require.extend_self) {
-            extend_facet_types.push_back(facet_type_id);
-          } else {
-            impls_facet_types.push_back(facet_type_id);
-          }
+        auto require_specific =
+            GetRequireImplsSpecificFromEnclosingSpecificWithSelfFacetValue(
+                context, require, extends.specific_id, self_facet_value);
+        auto require_self = GetConstantValueInRequireImplsSpecific(
+            context, require_specific, require.self_id);
+        auto require_facet_type = GetConstantValueInRequireImplsSpecific(
+            context, require_specific, require.facet_type_inst_id);
+        if (require_self == SemIR::ErrorInst::ConstantId ||
+            require_facet_type == SemIR::ErrorInst::ConstantId) {
+          return SemIR::IdentifiedFacetTypeId::None;
+        }
+
+        // TODO: Add and use constant_values().GetAs<SemIR::FacetType>().
+        auto facet_type_inst_id =
+            context.constant_values().GetInstId(require_facet_type);
+        auto facet_type_id = context.insts()
+                                 .GetAs<SemIR::FacetType>(facet_type_inst_id)
+                                 .facet_type_id;
+        if (facet_type_extends && require.extend_self) {
+          extend_facet_types.push_back({require_self, facet_type_id});
+        } else {
+          impls_facet_types.push_back({require_self, facet_type_id});
         }
       }
     }
@@ -903,18 +956,31 @@ auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
       for (auto require_impls_id : context.require_impls_blocks().Get(
                constraint.require_impls_block_id)) {
         const auto& require = context.require_impls().Get(require_impls_id);
-        auto facet_type_id = TryGetRequireImplsFacetTypeInEnclosingSpecific(
-            context, require, impls.specific_id);
-        if (facet_type_id.has_value()) {
-          impls_facet_types.push_back(facet_type_id);
+        auto require_specific =
+            GetRequireImplsSpecificFromEnclosingSpecificWithSelfFacetValue(
+                context, require, impls.specific_id, self_facet_value);
+        auto require_self = GetConstantValueInRequireImplsSpecific(
+            context, require_specific, require.self_id);
+        auto require_facet_type = GetConstantValueInRequireImplsSpecific(
+            context, require_specific, require.facet_type_inst_id);
+        if (require_self == SemIR::ErrorInst::ConstantId ||
+            require_facet_type == SemIR::ErrorInst::ConstantId) {
+          return SemIR::IdentifiedFacetTypeId::None;
         }
+
+        // TODO: Add and use constant_values().GetAs<SemIR::FacetType>().
+        auto facet_type_inst_id =
+            context.constant_values().GetInstId(require_facet_type);
+        auto facet_type_id = context.insts()
+                                 .GetAs<SemIR::FacetType>(facet_type_inst_id)
+                                 .facet_type_id;
+        impls_facet_types.push_back({require_self, facet_type_id});
       }
     }
   }
 
   // TODO: Process other kinds of requirements.
-  return context.identified_facet_types().Add(facet_type.facet_type_id,
-                                              {extends, self_impls});
+  return context.identified_facet_types().Add({key, extends, impls});
 }
 
 auto AsCompleteType(Context& context, SemIR::TypeId type_id,

+ 6 - 2
toolchain/check/type_completion.h

@@ -70,9 +70,13 @@ auto AsConcreteType(Context& context, SemIR::TypeId type_id,
     -> SemIR::TypeId;
 
 // Requires the named constraints in the facet type to be complete, so that the
-// set of interfaces the facet type requires is known. Diagnoses an error and
-// returns None if any named constraint is not complete.
+// set of interfaces the facet type requires is known. The `self_const_id` is
+// a type or facet type expression that is the self that the FacetType is
+// constraining. Produces a set of interfaces that must be implemented for a set
+// of types, most of them for the `self_const_id`. Diagnoses an error and
+// returns None if any error is found.
 auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
+                                SemIR::ConstantId self_const_id,
                                 const SemIR::FacetType& facet_type,
                                 MakeDiagnosticBuilderFn diagnoser)
     -> SemIR::IdentifiedFacetTypeId;

+ 4 - 7
toolchain/sem_ir/dump.cpp

@@ -137,11 +137,6 @@ LLVM_DUMP_METHOD auto Dump(const File& file, FacetTypeId facet_type_id)
         << "  - " << DumpInstSummary(file, rewrite.lhs_id) << "\n"
         << "  - " << DumpInstSummary(file, rewrite.rhs_id);
   }
-  if (auto identified_id =
-          file.identified_facet_types().TryGetId(facet_type_id);
-      identified_id.has_value()) {
-    out << "\nidentified: " << Dump(file, identified_id);
-  }
   return out.TakeStr();
 }
 
@@ -186,8 +181,10 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
 
   const auto& identified_facet_type =
       file.identified_facet_types().Get(identified_facet_type_id);
-  for (auto [i, req_interface] :
-       llvm::enumerate(identified_facet_type.required_interfaces())) {
+  for (auto [i, req_impl] :
+       llvm::enumerate(identified_facet_type.required_impls())) {
+    auto [self, req_interface] = req_impl;
+    // TODO: Dump the self too.
     out << "\n  - " << Dump(file, req_interface.interface_id);
     if (req_interface.specific_id.has_value()) {
       out << "; " << DumpSpecificSummary(file, req_interface.specific_id);

+ 17 - 13
toolchain/sem_ir/facet_type_info.cpp

@@ -46,11 +46,14 @@ static auto NamedConstraintsLess(const SpecificNamedConstraint& lhs,
 }
 
 // Canonically ordered by the numerical ids.
-static auto RequiredLess(const IdentifiedFacetType::RequiredInterface& lhs,
-                         const IdentifiedFacetType::RequiredInterface& rhs)
-    -> bool {
-  return std::tie(lhs.interface_id.index, lhs.specific_id.index) <
-         std::tie(rhs.interface_id.index, rhs.specific_id.index);
+static auto RequiredLess(const IdentifiedFacetType::RequiredImpl& lhs,
+                         const IdentifiedFacetType::RequiredImpl& rhs) -> bool {
+  return std::tie(lhs.self_facet_value.index,
+                  lhs.specific_interface.interface_id.index,
+                  lhs.specific_interface.specific_id.index) <
+         std::tie(rhs.self_facet_value.index,
+                  rhs.specific_interface.interface_id.index,
+                  rhs.specific_interface.specific_id.index);
 }
 
 // Assuming both `a` and `b` are sorted and deduplicated, replaces `a` with `a -
@@ -209,20 +212,21 @@ auto FacetTypeInfo::Print(llvm::raw_ostream& out) const -> void {
 }
 
 IdentifiedFacetType::IdentifiedFacetType(
-    llvm::ArrayRef<RequiredInterface> extends,
-    llvm::ArrayRef<RequiredInterface> self_impls) {
+    IdentifiedFacetTypeKey key, llvm::ArrayRef<RequiredImpl> extends,
+    llvm::ArrayRef<RequiredImpl> self_impls)
+    : key_(key) {
   if (extends.size() == 1) {
-    interface_id_ = extends.front().interface_id;
-    specific_id_ = extends.front().specific_id;
+    interface_id_ = extends.front().specific_interface.interface_id;
+    specific_id_ = extends.front().specific_interface.specific_id;
   } else {
     interface_id_ = InterfaceId::None;
     num_interface_to_impl_ = extends.size();
   }
 
-  required_interfaces_.reserve(extends.size() + self_impls.size());
-  llvm::append_range(required_interfaces_, extends);
-  llvm::append_range(required_interfaces_, self_impls);
-  SortAndDeduplicate(required_interfaces_, RequiredLess);
+  required_impls_.reserve(extends.size() + self_impls.size());
+  llvm::append_range(required_impls_, extends);
+  llvm::append_range(required_impls_, self_impls);
+  SortAndDeduplicate(required_impls_, RequiredLess);
 }
 
 auto AddCanonicalWitnessesBlock(File& sem_ir,

+ 43 - 9
toolchain/sem_ir/facet_type_info.h

@@ -97,6 +97,15 @@ struct FacetTypeInfo : Printable<FacetTypeInfo> {
     return std::nullopt;
   }
 
+  // Returns whether the facet type has no constraints, making it the facet type
+  // version of `TypeType`.
+  auto HasNoConstraints() const -> bool {
+    return extend_constraints.empty() && extend_named_constraints.empty() &&
+           self_impls_constraints.empty() &&
+           self_impls_named_constraints.empty() &&
+           rewrite_constraints.empty() && !other_requirements;
+  }
+
   friend auto operator==(const FacetTypeInfo& lhs, const FacetTypeInfo& rhs)
       -> bool {
     return lhs.extend_constraints == rhs.extend_constraints &&
@@ -116,15 +125,31 @@ constexpr FacetTypeInfo::RewriteConstraint
 using FacetTypeInfoStore =
     CanonicalValueStore<FacetTypeId, FacetTypeInfo, Tag<CheckIRId>>;
 
+struct IdentifiedFacetTypeKey {
+  FacetTypeId facet_type_id;
+  ConstantId self_const_id;
+
+  friend auto operator==(const IdentifiedFacetTypeKey& lhs,
+                         const IdentifiedFacetTypeKey& rhs) -> bool = default;
+};
+
 struct IdentifiedFacetType {
-  using RequiredInterface = SpecificInterface;
+  // A requirement that `self_facet_value` implements the `specific_interface`.
+  struct RequiredImpl {
+    ConstantId self_facet_value;
+    SpecificInterface specific_interface;
 
-  IdentifiedFacetType(llvm::ArrayRef<RequiredInterface> extends,
-                      llvm::ArrayRef<RequiredInterface> self_impls);
+    friend auto operator==(const RequiredImpl& lhs, const RequiredImpl& rhs)
+        -> bool = default;
+  };
+
+  IdentifiedFacetType(IdentifiedFacetTypeKey key,
+                      llvm::ArrayRef<RequiredImpl> extends,
+                      llvm::ArrayRef<RequiredImpl> self_impls);
 
   // The order here defines the order of impl witnesses for this facet type.
-  auto required_interfaces() const -> llvm::ArrayRef<RequiredInterface> {
-    return required_interfaces_;
+  auto required_impls() const -> llvm::ArrayRef<RequiredImpl> {
+    return required_impls_;
   }
 
   // Can this be used to the right of an `as` in an `impl` declaration?
@@ -150,12 +175,17 @@ struct IdentifiedFacetType {
     }
   }
 
+  auto GetAsKey() const -> IdentifiedFacetTypeKey { return key_; }
+
  private:
-  // Interfaces mentioned explicitly in the facet type expression, or
-  // transitively through a named constraint. Sorted and deduplicated.
-  llvm::SmallVector<RequiredInterface> required_interfaces_;
+  IdentifiedFacetTypeKey key_;
 
-  // The single interface from `required_interfaces` to implement if this is
+  // Requirements that a facet value implements an interface, mentioned
+  // explicitly in the facet type expression or transitively through a named
+  // constraint. Sorted and deduplicated.
+  llvm::SmallVector<RequiredImpl> required_impls_;
+
+  // The single interface from `required_impls` to implement if this is
   // the facet type to the right of an `impl`...`as`, or `None` if no such
   // single interface.
   InterfaceId interface_id_ = InterfaceId::None;
@@ -168,6 +198,10 @@ struct IdentifiedFacetType {
   };
 };
 
+using IdentifiedFacetTypeStore =
+    CanonicalValueStore<IdentifiedFacetTypeId, IdentifiedFacetTypeKey,
+                        Tag<CheckIRId>, IdentifiedFacetType>;
+
 // See common/hashing.h.
 inline auto CarbonHashValue(const FacetTypeInfo& value, uint64_t seed)
     -> HashCode {

+ 1 - 1
toolchain/sem_ir/file.cpp

@@ -46,7 +46,7 @@ File::File(const Parse::Tree* parse_tree, CheckIRId check_ir_id,
       require_impls_blocks_(allocator_, check_ir_id, 1),
       associated_constants_(check_ir_id),
       facet_types_(check_ir_id),
-      identified_facet_types_(&facet_types_),
+      identified_facet_types_(check_ir_id),
       impls_(*this),
       specific_interfaces_(check_ir_id),
       generics_(check_ir_id),

+ 0 - 4
toolchain/sem_ir/file.h

@@ -71,10 +71,6 @@ using CustomLayoutStore =
 // The semantic IR for a single file.
 class File : public Printable<File> {
  public:
-  using IdentifiedFacetTypeStore =
-      RelationalValueStore<FacetTypeInfoStore, IdentifiedFacetTypeId,
-                           IdentifiedFacetType>;
-
   // Starts a new file for Check::CheckParseTree.
   explicit File(const Parse::Tree* parse_tree, CheckIRId check_ir_id,
                 const std::optional<Parse::Tree::PackagingDecl>& packaging_decl,

+ 8 - 8
toolchain/sem_ir/impl.cpp

@@ -16,14 +16,14 @@ ImplStore::ImplStore(File& sem_ir)
     : sem_ir_(sem_ir), values_(sem_ir.check_ir_id()) {}
 
 auto ImplStore::GetOrAddLookupBucket(const Impl& impl) -> LookupBucketRef {
-  auto self_id = sem_ir_.constant_values().GetConstantInstId(impl.self_id);
+  auto self_const_id = sem_ir_.constant_values().Get(impl.self_id);
   auto impl_as_interface = SpecificInterface::None;
   auto facet_type_type_id = TypeId::ForTypeConstant(
       sem_ir_.constant_values().Get(impl.constraint_id));
   if (auto facet_type =
           sem_ir_.types().TryGetAs<FacetType>(facet_type_type_id)) {
-    auto identified_id =
-        sem_ir_.identified_facet_types().TryGetId(facet_type->facet_type_id);
+    auto identified_id = sem_ir_.identified_facet_types().Lookup(
+        {facet_type->facet_type_id, self_const_id});
     if (identified_id.has_value()) {
       const auto& identified =
           sem_ir_.identified_facet_types().Get(identified_id);
@@ -32,11 +32,11 @@ auto ImplStore::GetOrAddLookupBucket(const Impl& impl) -> LookupBucketRef {
       }
     }
   }
-  return LookupBucketRef(*this,
-                         lookup_
-                             .Insert(std::pair{self_id, impl_as_interface},
-                                     [] { return ImplOrLookupBucketId::None; })
-                             .value());
+  return LookupBucketRef(
+      *this, lookup_
+                 .Insert(std::pair{self_const_id, impl_as_interface},
+                         [] { return ImplOrLookupBucketId::None; })
+                 .value());
 }
 
 }  // namespace Carbon::SemIR

+ 1 - 1
toolchain/sem_ir/impl.h

@@ -213,7 +213,7 @@ class ImplStore {
  private:
   File& sem_ir_;
   ValueStore<ImplId, Impl, Tag<CheckIRId>> values_;
-  Map<std::pair<InstId, SpecificInterface>, ImplOrLookupBucketId> lookup_;
+  Map<std::pair<ConstantId, SpecificInterface>, ImplOrLookupBucketId> lookup_;
   // Buckets with at least 2 entries, which will be rare; see LookupBucketRef.
   llvm::SmallVector<llvm::SmallVector<ImplId, 2>> lookup_buckets_;
 };

+ 47 - 11
toolchain/sem_ir/inst_namer.cpp

@@ -48,6 +48,11 @@ class InstNamer::NamingContext {
   // Adds the instruction's name.
   auto AddInstName(std::string name) -> void;
 
+  // Adds the instruction's name for a StructType, which contains the names of
+  // its fields.
+  auto AddStructTypeInstName(StructType struct_ty, llvm::StringRef type_suffix,
+                             llvm::StringRef name_suffix) -> void;
+
   // Adds the instruction's name by `NameId`.
   auto AddInstNameId(NameId name_id, llvm::StringRef suffix = "") -> void {
     AddInstName((sem_ir().names().GetIRBaseName(name_id) + suffix).str());
@@ -750,6 +755,26 @@ auto InstNamer::NamingContext::AddInstName(std::string name) -> void {
   }
 }
 
+auto InstNamer::NamingContext::AddStructTypeInstName(
+    StructType struct_ty, llvm::StringRef type_suffix,
+    llvm::StringRef name_suffix) -> void {
+  RawStringOstream out;
+  const auto& fields = sem_ir().struct_type_fields().Get(struct_ty.fields_id);
+  if (fields.empty()) {
+    out << "empty_struct" << type_suffix;
+  } else {
+    RawStringOstream name;
+    name << "struct" << type_suffix;
+    for (auto field : fields) {
+      name << ".";
+      name << sem_ir().names().GetIRBaseName(field.name_id);
+    }
+    out << name.TakeStr();
+  }
+  out << name_suffix;
+  AddInstName(out.TakeStr());
+}
+
 auto InstNamer::NamingContext::AddIntOrFloatTypeName(char type_literal_prefix,
                                                      InstId bit_width_id,
                                                      llvm::StringRef suffix)
@@ -988,6 +1013,27 @@ auto InstNamer::NamingContext::NameInst() -> void {
           }
           return;
         }
+        if (facet_type_info.HasNoConstraints()) {
+          if (auto class_ty =
+                  sem_ir().insts().TryGetAs<ClassType>(inst.type_inst_id)) {
+            AddEntityNameAndMaybePush(class_ty->class_id, ".type.facet");
+            return;
+          }
+          if (auto tuple_ty = sem_ir().insts().TryGetAs<SemIR::TupleType>(
+                  inst.type_inst_id)) {
+            if (tuple_ty->type_elements_id == InstBlockId::Empty) {
+              AddInstName("empty_tuple.type.facet");
+            } else {
+              AddInstName("tuple.type.facet");
+            }
+            return;
+          }
+          if (auto struct_ty = sem_ir().insts().TryGetAs<SemIR::StructType>(
+                  inst.type_inst_id)) {
+            AddStructTypeInstName(*struct_ty, "", ".type.facet");
+            return;
+          }
+        }
       }
       AddInstName("facet_value");
       return;
@@ -1237,17 +1283,7 @@ auto InstNamer::NamingContext::NameInst() -> void {
       return;
     }
     case CARBON_KIND(StructType inst): {
-      const auto& fields = sem_ir().struct_type_fields().Get(inst.fields_id);
-      if (fields.empty()) {
-        AddInstName("empty_struct_type");
-        return;
-      }
-      std::string name = "struct_type";
-      for (auto field : fields) {
-        name += ".";
-        name += sem_ir().names().GetIRBaseName(field.name_id).str();
-      }
-      AddInstName(std::move(name));
+      AddStructTypeInstName(inst, "_type", "");
       return;
     }
     case CARBON_KIND(StructValue inst): {