specific-named-constraints

Author: danakj

toolchain/check/handle_require.cpp

@@ -183,8 +183,8 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
     return std::nullopt;
   }
 
-  auto identified_facet_type_id =
-      RequireIdentifiedFacetType(context, *constraint_facet_type, [&] {
+  auto identified_facet_type_id = RequireIdentifiedFacetType(
+      context, SemIR::LocId(constraint_inst_id), *constraint_facet_type, [&] {
         CARBON_DIAGNOSTIC(
             RequireImplsUnidentifiedFacetType, Error,
             "facet type {0} cannot be identified in `require` declaration",

toolchain/check/impl.cpp

@@ -297,13 +297,14 @@ auto CheckConstraintIsInterface(Context& context, SemIR::InstId impl_decl_id,
     return SemIR::SpecificInterface::None;
   }
 
-  auto identified_id = RequireIdentifiedFacetType(context, *facet_type, [&] {
-    CARBON_DIAGNOSTIC(ImplOfUnidentifiedFacetType, Error,
-                      "facet type {0} cannot be identified in `impl as`",
-                      InstIdAsType);
-    return context.emitter().Build(impl_decl_id, ImplOfUnidentifiedFacetType,
-                                   constraint_id);
-  });
+  auto identified_id = RequireIdentifiedFacetType(
+      context, SemIR::LocId(constraint_id), *facet_type, [&] {
+        CARBON_DIAGNOSTIC(ImplOfUnidentifiedFacetType, Error,
+                          "facet type {0} cannot be identified in `impl as`",
+                          InstIdAsType);
+        return context.emitter().Build(
+            impl_decl_id, ImplOfUnidentifiedFacetType, constraint_id);
+      });
   if (!identified_id.has_value()) {
     return SemIR::SpecificInterface::None;
   }

toolchain/check/impl_lookup.cpp

@@ -202,7 +202,7 @@ static auto GetInterfacesFromConstantId(
   const auto& facet_type_info =
       context.facet_types().Get(facet_type_inst.facet_type_id);
   auto identified_id =
-      RequireIdentifiedFacetType(context, facet_type_inst, [&] {
+      RequireIdentifiedFacetType(context, loc_id, facet_type_inst, [&] {
         CARBON_DIAGNOSTIC(ImplLookupInUnidentifiedFacetType, Error,
                           "facet type {0} can not be identified", InstIdAsType);
         return context.emitter().Build(
@@ -323,7 +323,8 @@ static auto GetWitnessIdForImpl(Context& context, SemIR::LocId loc_id,
 // is allowed to be a non-canonical facet value in order to find a concrete
 // witness, so it's not referenced as a constant value.
 static auto LookupImplWitnessInSelfFacetValue(
-    Context& context, SemIR::InstId self_facet_value_inst_id,
+    Context& context, SemIR::LocId loc_id,
+    SemIR::InstId self_facet_value_inst_id,
     SemIR::SpecificInterface query_specific_interface) -> EvalImplLookupResult {
   auto facet_type = context.types().TryGetAs<SemIR::FacetType>(
       context.insts().Get(self_facet_value_inst_id).type_id());
@@ -336,7 +337,7 @@ static auto LookupImplWitnessInSelfFacetValue(
   // `FacetValue` witnesses are the output of an impl lookup, which finds and
   // returns witnesses in the same order.
   auto identified_id =
-      RequireIdentifiedFacetType(context, *facet_type, nullptr);
+      RequireIdentifiedFacetType(context, loc_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 complete facet types.
@@ -879,7 +880,7 @@ auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id,
       context.specific_interfaces().Get(eval_query.query_specific_interface_id);
 
   auto facet_lookup_result = LookupImplWitnessInSelfFacetValue(
-      context, self_facet_value_inst_id, query_specific_interface);
+      context, loc_id, self_facet_value_inst_id, query_specific_interface);
   if (facet_lookup_result.has_final_value()) {
     return facet_lookup_result;
   }

toolchain/check/import_ref.cpp

@@ -15,6 +15,7 @@
 #include "toolchain/base/shared_value_stores.h"
 #include "toolchain/check/context.h"
 #include "toolchain/check/eval.h"
+#include "toolchain/check/facet_type.h"
 #include "toolchain/check/generic.h"
 #include "toolchain/check/import.h"
 #include "toolchain/check/inst.h"
@@ -494,7 +495,7 @@ static auto SetIndirectConstantValues(
 // Adds an import_ref instruction for an instruction that we have already loaded
 // from an imported IR, with a known constant value. This is useful when the
 // instruction has a symbolic constant value, in order to produce an instruction
-// that hold that symbolic constant.
+// that holds that symbolic constant.
 static auto AddLoadedImportRef(ImportContext& context,
                                SemIR::TypeId local_type_id,
                                SemIR::InstId import_inst_id,
@@ -869,8 +870,14 @@ static auto GetLocalConstantId(ImportRefResolver& resolver,
     // declaration.
     return GetLocalConstantId(resolver, import_decl_inst_id);
   }
+  if (import_decl_inst.Is<SemIR::RequireImplsDecl>()) {
+    // For an impl declaration, the imported entity can be found via the
+    // declaration.
+    return GetLocalConstantId(resolver, import_decl_inst_id);
+  }
   // For all other kinds of declaration, the imported entity can be found via
   // the type of the declaration.
+  CARBON_CHECK(import_decl_inst.type_id().has_value());
   return GetLocalConstantId(resolver, import_decl_inst.type_id());
 }
 
@@ -904,6 +911,11 @@ static auto GetLocalGenericId(ImportContext& context,
     case CARBON_KIND(SemIR::ImplDecl impl_decl): {
       return context.local_impls().Get(impl_decl.impl_id).generic_id;
     }
+    case CARBON_KIND(SemIR::RequireImplsDecl require_decl): {
+      return context.local_require_impls()
+          .Get(require_decl.require_impls_id)
+          .generic_id;
+    }
     default: {
       CARBON_FATAL("Unexpected inst for generic declaration: {0}", inst);
     }
@@ -2562,6 +2574,13 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
       resolver, import_impl.constraint_id, constraint_const_id);
   new_impl.interface = GetLocalSpecificInterface(
       resolver, import_impl.interface, specific_interface_data);
+  // Create a local IdentifiedFacetType for the imported facet type, since impl
+  // declarations always identify the facet type.
+  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);
+  }
   if (import_impl.is_complete()) {
     AddImplDefinition(resolver, import_impl, new_impl);
   }
@@ -2579,10 +2598,45 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
-                                SemIR::RequireImplsDecl inst) -> ResolveResult {
+                                SemIR::RequireImplsDecl inst,
+                                SemIR::ConstantId require_decl_const_id)
+    -> ResolveResult {
   const auto& import_require =
       resolver.import_require_impls().Get(inst.require_impls_id);
 
+  auto require_decl_id = SemIR::InstId::None;
+  auto require_impls_id = SemIR::RequireImplsId::None;
+  if (!require_decl_const_id.has_value()) {
+    // Phase one: Make the decl and structure with placeholder values to be
+    // filled in. Begin the generic so instructions can be attached to it.
+    SemIR::RequireImplsDecl require_decl = {
+        .require_impls_id = SemIR::RequireImplsId::None,
+        .decl_block_id = SemIR::InstBlockId::Empty};
+    auto require_decl_id = AddPlaceholderImportedInst(
+        resolver, import_require.decl_id, require_decl);
+    require_impls_id = resolver.local_require_impls().Add(
+        {.self_id = SemIR::TypeInstId::None,
+         .facet_type_inst_id = SemIR::TypeInstId::None,
+         .facet_type_id = SemIR::FacetTypeId::None,
+         .decl_id = require_decl_id,
+         .parent_scope_id = SemIR::NameScopeId::None,
+         .generic_id = MakeIncompleteGeneric(resolver, require_decl_id,
+                                             import_require.generic_id)});
+
+    // Write the RequireImplsId into the RequireImplsDecl.
+    require_decl.require_impls_id = require_impls_id;
+    require_decl_const_id =
+        ReplacePlaceholderImportedInst(resolver, require_decl_id, require_decl);
+  } else {
+    // Phase two: Get the `require_decl_id` and `require_impls_id` from the
+    // RequireImplsDecl constructed in phase one.
+    require_decl_id =
+        resolver.local_constant_values().GetInstId(require_decl_const_id);
+    require_impls_id = resolver.local_insts()
+                           .GetAs<SemIR::RequireImplsDecl>(require_decl_id)
+                           .require_impls_id;
+  }
+
   // Load dependent constants.
   auto parent_scope_id =
       GetLocalNameScopeId(resolver, import_require.parent_scope_id);
@@ -2592,29 +2646,9 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
       GetLocalConstantId(resolver, import_require.facet_type_inst_id);
 
   if (resolver.HasNewWork()) {
-    return ResolveResult::Retry();
+    return ResolveResult::Retry(require_decl_const_id, require_decl_id);
   }
 
-  // Make the decl and structure with placeholder values to be filled in.
-  SemIR::RequireImplsDecl require_decl = {
-      .require_impls_id = SemIR::RequireImplsId::None,
-      .decl_block_id = SemIR::InstBlockId::Empty};
-  auto require_decl_id = AddPlaceholderImportedInst(
-      resolver, import_require.decl_id, require_decl);
-  auto require_impls_id = resolver.local_require_impls().Add(
-      {.self_id = SemIR::TypeInstId::None,
-       .facet_type_inst_id = SemIR::TypeInstId::None,
-       .facet_type_id = SemIR::FacetTypeId::None,
-       .decl_id = require_decl_id,
-       .parent_scope_id = SemIR::NameScopeId::None,
-       .generic_id = MakeIncompleteGeneric(resolver, require_decl_id,
-                                           import_require.generic_id)});
-
-  // Write the RequireImplsId into the RequireImplsDecl.
-  require_decl.require_impls_id = require_impls_id;
-  auto require_decl_const_id =
-      ReplacePlaceholderImportedInst(resolver, require_decl_id, require_decl);
-
   // Fill in the RequireImpls structure.
   auto& new_require = resolver.local_require_impls().Get(require_impls_id);
   new_require.self_id = AddLoadedImportRefForType(
@@ -3604,6 +3638,9 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
     case CARBON_KIND(SemIR::NamedConstraintDecl inst): {
       return TryResolveTypedInst(resolver, inst, const_id);
     }
+    case CARBON_KIND(SemIR::RequireImplsDecl inst): {
+      return TryResolveTypedInst(resolver, inst, const_id);
+    }
     default:
       break;
   }
@@ -3774,9 +3811,6 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
     case CARBON_KIND(SemIR::RequireCompleteType inst): {
       return TryResolveTypedInst(resolver, inst);
     }
-    case CARBON_KIND(SemIR::RequireImplsDecl inst): {
-      return TryResolveTypedInst(resolver, inst);
-    }
     case CARBON_KIND(SemIR::RequireSpecificDefinition inst): {
       return TryResolveTypedInst(resolver, inst);
     }

toolchain/check/testdata/facet/require_import.carbon

@@ -53,10 +53,10 @@ fn F(A:! X, B:! Y) {}
 // CHECK:STDOUT:   %Main.Y: type = import_ref Main//a, Y, loaded [concrete = constants.%Y.type]
 // CHECK:STDOUT:   %Main.X: type = import_ref Main//a, X, loaded [concrete = constants.%X.type]
 // CHECK:STDOUT:   %Main.import_ref.462 = import_ref Main//a, loc3_13, unloaded
-// CHECK:STDOUT:   %Main.import_ref.b7c: type = import_ref Main//a, loc10_18, loaded [symbolic = constants.%Self.binding.as_type.1a1]
+// CHECK:STDOUT:   %Main.import_ref.b7c: type = import_ref Main//a, loc10_18, loaded [symbolic = @X.require0.%Self.binding.as_type (constants.%Self.binding.as_type.1a1)]
 // CHECK:STDOUT:   %Main.import_ref.dec: %X.type = import_ref Main//a, loc9_14, loaded [symbolic = constants.%Self.861]
 // CHECK:STDOUT:   %Main.import_ref.cae = import_ref Main//a, loc9_14, unloaded
-// CHECK:STDOUT:   %Main.import_ref.3e6: type = import_ref Main//a, loc6_11, loaded [symbolic = constants.%Self.binding.as_type.384]
+// CHECK:STDOUT:   %Main.import_ref.3e6: type = import_ref Main//a, loc6_11, loaded [symbolic = @Y.require1.%Self.binding.as_type (constants.%Self.binding.as_type.384)]
 // CHECK:STDOUT:   %Main.import_ref.35b: %Y.type = import_ref Main//a, loc5_13, loaded [symbolic = constants.%Self.29b]
 // CHECK:STDOUT:   %Main.import_ref.581 = import_ref Main//a, loc5_13, unloaded
 // CHECK:STDOUT: }

toolchain/check/testdata/impl/impl_as_named_constraint.carbon

@@ -128,7 +128,7 @@ constraint C {
 // CHECK:STDERR:
 impl () as C {}
 
-// --- todo_fail_duplicate_through_generic_constraint.carbon
+// --- fail_duplicate_through_generic_constraint.carbon
 library "[[@TEST_NAME]]";
 
 interface A(T:! type) {}
@@ -140,5 +140,11 @@ constraint B(X:! type, Y:! type) {
 // This should impl () as A({}).
 impl () as B((), {}) {}
 
-// This should be considered a duplicate.
+// CHECK:STDERR: fail_duplicate_through_generic_constraint.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
+// CHECK:STDERR: impl () as A({}) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_duplicate_through_generic_constraint.carbon:[[@LINE-5]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
+// CHECK:STDERR: impl () as B((), {}) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
 impl () as A({}) {}