Track the instruction used to name the type and constraint in an impl. (

carbon-language#4368) This is necessary in order to have access to the specific versions of their constant values in a generic impl. Stub out impl deduction.
josh11b · Oct 4, 2024 · 568ad19 · 568ad19
1 parent eab5dd6
commit 568ad19
Show file tree

Hide file tree

Showing 79 changed files with 590 additions and 341 deletions.
diff --git a/toolchain/check/deduce.cpp b/toolchain/check/deduce.cpp
@@ -10,6 +10,7 @@
 #include "toolchain/check/generic.h"
 #include "toolchain/check/subst.h"
 #include "toolchain/sem_ir/ids.h"
+#include "toolchain/sem_ir/impl.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Check {
@@ -238,4 +239,18 @@ auto DeduceGenericCallArguments(
                       context.inst_blocks().AddCanonical(result_arg_ids));
 }
 
+// Deduces the impl arguments to use in a use of a parameterized impl. Returns
+// `Invalid` if deduction fails.
+auto DeduceImplArguments(Context& context, const SemIR::Impl& impl,
+                         SemIR::ConstantId self_id,
+                         SemIR::ConstantId constraint_id) -> SemIR::SpecificId {
+  CARBON_CHECK(impl.generic_id.is_valid(),
+               "Performing deduction for non-generic impl");
+  // TODO: This is a placeholder. Implement deduction.
+  static_cast<void>(context);
+  static_cast<void>(self_id);
+  static_cast<void>(constraint_id);
+  return SemIR::SpecificId::Invalid;
+}
+
 }  // namespace Carbon::Check
diff --git a/toolchain/check/deduce.h b/toolchain/check/deduce.h
@@ -20,6 +20,12 @@ auto DeduceGenericCallArguments(Context& context, SemIR::LocId loc_id,
                                 llvm::ArrayRef<SemIR::InstId> arg_ids)
     -> SemIR::SpecificId;
 
+// Deduces the impl arguments to use in a use of a parameterized impl. Returns
+// `Invalid` if deduction fails.
+auto DeduceImplArguments(Context& context, const SemIR::Impl& impl,
+                         SemIR::ConstantId self_id,
+                         SemIR::ConstantId constraint_id) -> SemIR::SpecificId;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_DEDUCE_H_
diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp
@@ -55,14 +55,14 @@ auto HandleParseNode(Context& context, Parse::ImplForallId node_id) -> bool {
 
 auto HandleParseNode(Context& context, Parse::TypeImplAsId node_id) -> bool {
   auto [self_node, self_id] = context.node_stack().PopExprWithNodeId();
-  auto [self_inst_id, self_type_id] = ExprAsType(context, self_node, self_id);
-  context.node_stack().Push(node_id, self_type_id);
+  self_id = ExprAsType(context, self_node, self_id).inst_id;
+  context.node_stack().Push(node_id, self_id);
 
   // Introduce `Self`. Note that we add this name lexically rather than adding
   // to the `NameScopeId` of the `impl`, because this happens before we enter
   // the `impl` scope or even identify which `impl` we're declaring.
   // TODO: Revisit this once #3714 is resolved.
-  context.AddNameToLookup(SemIR::NameId::SelfType, self_inst_id);
+  context.AddNameToLookup(SemIR::NameId::SelfType, self_id);
   return true;
 }
 
@@ -103,9 +103,21 @@ auto HandleParseNode(Context& context, Parse::DefaultSelfImplAsId node_id)
     self_type_id = SemIR::TypeId::Error;
   }
 
+  // Build the implicit access to the enclosing `Self`.
+  // TODO: Consider calling `HandleNameAsExpr` to build this implicit `Self`
+  // expression. We've already done the work to check that the enclosing context
+  // is a class and found its `Self`, so additionally performing an unqualified
+  // name lookup would be redundant work, but would avoid duplicating the
+  // handling of the `Self` expression.
+  auto self_inst_id = context.AddInst(
+      node_id,
+      SemIR::NameRef{.type_id = SemIR::TypeId::TypeType,
+                     .name_id = SemIR::NameId::SelfType,
+                     .value_id = context.types().GetInstId(self_type_id)});
+
   // There's no need to push `Self` into scope here, because we can find it in
   // the parent class scope.
-  context.node_stack().Push(node_id, self_type_id);
+  context.node_stack().Push(node_id, self_inst_id);
   return true;
 }
 
@@ -241,16 +253,17 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
     -> std::pair<SemIR::ImplId, SemIR::InstId> {
   auto [constraint_node, constraint_id] =
       context.node_stack().PopExprWithNodeId();
-  auto [self_type_node, self_type_id] =
+  auto [self_type_node, self_inst_id] =
       context.node_stack().PopWithNodeId<Parse::NodeCategory::ImplAs>();
+  auto self_type_id = context.GetTypeIdForTypeInst(self_inst_id);
   // Pop the `impl` introducer and any `forall` parameters as a "name".
   auto name = PopImplIntroducerAndParamsAsNameComponent(context, node_id);
   auto decl_block_id = context.inst_block_stack().Pop();
 
   // Convert the constraint expression to a type.
   // TODO: Check that its constant value is a constraint.
-  auto constraint_type_id =
-      ExprAsType(context, constraint_node, constraint_id).type_id;
+  auto [constraint_inst_id, constraint_type_id] =
+      ExprAsType(context, constraint_node, constraint_id);
 
   // Process modifiers.
   // TODO: Should we somehow permit access specifiers on `impl`s?
@@ -276,11 +289,10 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
       name_context.MakeEntityWithParamsBase(name, impl_decl_id,
                                             /*is_extern=*/false,
                                             SemIR::LibraryNameId::Invalid),
-      {.self_id = self_type_id, .constraint_id = constraint_type_id}};
+      {.self_id = self_inst_id, .constraint_id = constraint_inst_id}};
 
   // Add the impl declaration.
-  auto lookup_bucket_ref = context.impls().GetOrAddLookupBucket(
-      impl_info.self_id, impl_info.constraint_id);
+  auto lookup_bucket_ref = context.impls().GetOrAddLookupBucket(impl_info);
   for (auto prev_impl_id : lookup_bucket_ref) {
     if (MergeImplRedecl(context, impl_info, prev_impl_id)) {
       impl_decl.impl_id = prev_impl_id;
@@ -329,13 +341,14 @@ auto HandleParseNode(Context& context, Parse::ImplDefinitionStartId node_id)
 
   if (impl_info.is_defined()) {
     CARBON_DIAGNOSTIC(ImplRedefinition, Error,
-                      "redefinition of `impl {0} as {1}`", SemIR::TypeId,
-                      SemIR::TypeId);
+                      "redefinition of `impl {0} as {1}`", std::string,
+                      std::string);
     CARBON_DIAGNOSTIC(ImplPreviousDefinition, Note,
                       "previous definition was here");
     context.emitter()
-        .Build(node_id, ImplRedefinition, impl_info.self_id,
-               impl_info.constraint_id)
+        .Build(node_id, ImplRedefinition,
+               context.sem_ir().StringifyTypeExpr(impl_info.self_id),
+               context.sem_ir().StringifyTypeExpr(impl_info.constraint_id))
         .Note(impl_info.definition_id, ImplPreviousDefinition)
         .Emit();
   } else {

diff --git a/toolchain/check/impl.cpp b/toolchain/check/impl.cpp
@@ -139,6 +139,7 @@ static auto BuildInterfaceWitness(
   }
 
   auto& impl_scope = context.name_scopes().Get(impl.scope_id);
+  auto self_type_id = context.GetTypeIdForTypeInst(impl.self_id);
 
   llvm::SmallVector<SemIR::InstId> table;
   auto assoc_entities =
@@ -168,7 +169,7 @@ static auto BuildInterfaceWitness(
         if (impl_decl_id.is_valid()) {
           used_decl_ids.push_back(impl_decl_id);
           table.push_back(CheckAssociatedFunctionImplementation(
-              context, *fn_type, impl_decl_id, impl.self_id));
+              context, *fn_type, impl_decl_id, self_type_id));
         } else {
           CARBON_DIAGNOSTIC(
               ImplMissingFunction, Error,
@@ -210,16 +211,17 @@ auto BuildImplWitness(Context& context, SemIR::ImplId impl_id)
   CARBON_CHECK(impl.is_being_defined());
 
   // TODO: Handle non-interface constraints.
+  auto interface_type_id = context.GetTypeIdForTypeInst(impl.constraint_id);
   auto interface_type =
-      context.types().TryGetAs<SemIR::InterfaceType>(impl.constraint_id);
+      context.types().TryGetAs<SemIR::InterfaceType>(interface_type_id);
   if (!interface_type) {
     context.TODO(impl.definition_id, "impl as non-interface");
     return SemIR::InstId::BuiltinError;
   }
 
   llvm::SmallVector<SemIR::InstId> used_decl_ids;
 
-  auto witness_id = BuildInterfaceWitness(context, impl, impl.constraint_id,
+  auto witness_id = BuildInterfaceWitness(context, impl, interface_type_id,
                                           *interface_type, used_decl_ids);
 
   // TODO: Diagnose if any declarations in the impl are not in used_decl_ids.

diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp
@@ -56,7 +56,8 @@ auto AddImportIR(Context& context, SemIR::ImportIR import_ir)
 }
 
 auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst,
-                  SemIR::EntityNameId entity_name_id) -> SemIR::InstId {
+                  SemIR::EntityNameId entity_name_id =
+                      SemIR::EntityNameId::Invalid) -> SemIR::InstId {
   auto import_ir_inst_id = context.import_ir_insts().Add(import_ir_inst);
   SemIR::ImportRefUnloaded inst = {.import_ir_inst_id = import_ir_inst_id,
                                    .entity_name_id = entity_name_id};
@@ -70,6 +71,29 @@ auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst,
   return import_ref_id;
 }
 
+// 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.
+static auto AddLoadedImportRef(Context& context,
+                               SemIR::ImportIRInst import_ir_inst,
+                               SemIR::TypeId type_id,
+                               SemIR::ConstantId const_id) -> SemIR::InstId {
+  auto import_ir_inst_id = context.import_ir_insts().Add(import_ir_inst);
+  SemIR::ImportRefLoaded inst = {
+      .type_id = type_id,
+      .import_ir_inst_id = import_ir_inst_id,
+      .entity_name_id = SemIR::EntityNameId::Invalid};
+  auto inst_id = context.AddPlaceholderInstInNoBlock(
+      context.MakeImportedLocAndInst(import_ir_inst_id, inst));
+  context.import_ref_ids().push_back(inst_id);
+
+  context.constant_values().Set(inst_id, const_id);
+  context.import_ir_constant_values()[import_ir_inst.ir_id.index].Set(
+      import_ir_inst.inst_id, const_id);
+  return inst_id;
+}
+
 auto GetCanonicalImportIRInst(Context& context, const SemIR::File* cursor_ir,
                               SemIR::InstId cursor_inst_id)
     -> SemIR::ImportIRInst {
@@ -115,8 +139,7 @@ auto VerifySameCanonicalImportIRInst(Context& context, SemIR::InstId prev_id,
     return;
   }
   auto conflict_id =
-      AddImportRef(context, {.ir_id = new_ir_id, .inst_id = new_inst_id},
-                   SemIR::EntityNameId::Invalid);
+      AddImportRef(context, {.ir_id = new_ir_id, .inst_id = new_inst_id});
   context.DiagnoseDuplicateName(conflict_id, prev_id);
 }
 
@@ -902,8 +925,7 @@ class ImportRefResolver {
                               SemIR::NameScope& new_scope) -> void {
     for (auto entry : import_scope.names) {
       auto ref_id = AddImportRef(
-          context_, {.ir_id = import_ir_id_, .inst_id = entry.inst_id},
-          SemIR::EntityNameId::Invalid);
+          context_, {.ir_id = import_ir_id_, .inst_id = entry.inst_id});
       new_scope.AddRequired({.name_id = GetLocalNameId(entry.name_id),
                              .inst_id = ref_id,
                              .access_kind = entry.access_kind});
@@ -923,8 +945,7 @@ class ImportRefResolver {
     new_associated_entities.reserve(associated_entities.size());
     for (auto inst_id : associated_entities) {
       new_associated_entities.push_back(
-          AddImportRef(context_, {.ir_id = import_ir_id_, .inst_id = inst_id},
-                       SemIR::EntityNameId::Invalid));
+          AddImportRef(context_, {.ir_id = import_ir_id_, .inst_id = inst_id}));
     }
     return context_.inst_blocks().Add(new_associated_entities);
   }
@@ -1143,8 +1164,7 @@ class ImportRefResolver {
 
     // Add a lazy reference to the target declaration.
     auto decl_id = AddImportRef(
-        context_, {.ir_id = import_ir_id_, .inst_id = inst.decl_id},
-        SemIR::EntityNameId::Invalid);
+        context_, {.ir_id = import_ir_id_, .inst_id = inst.decl_id});
 
     return ResolveAs<SemIR::AssociatedEntity>(
         {.type_id = context_.GetTypeIdForTypeConstant(type_const_id),
@@ -1589,8 +1609,8 @@ class ImportRefResolver {
             AddImportIRInst(import_impl.latest_decl_id()), impl_decl));
     impl_decl.impl_id = context_.impls().Add(
         {GetIncompleteLocalEntityBase(impl_decl_id, import_impl),
-         {.self_id = SemIR::TypeId::Invalid,
-          .constraint_id = SemIR::TypeId::Invalid,
+         {.self_id = SemIR::InstId::Invalid,
+          .constraint_id = SemIR::InstId::Invalid,
           .witness_id = SemIR::InstId::Invalid}});
 
     // Write the impl ID into the ImplDecl.
@@ -1654,8 +1674,10 @@ class ImportRefResolver {
     auto parent_scope_id = GetLocalNameScopeId(import_impl.parent_scope_id);
     LoadLocalParamConstantIds(import_impl.implicit_param_refs_id);
     auto generic_data = GetLocalGenericData(import_impl.generic_id);
-    auto self_const_id = GetLocalConstantId(import_impl.self_id);
-    auto constraint_const_id = GetLocalConstantId(import_impl.constraint_id);
+    auto self_const_id = GetLocalConstantId(
+        import_ir_.constant_values().Get(import_impl.self_id));
+    auto constraint_const_id = GetLocalConstantId(
+        import_ir_.constant_values().Get(import_impl.constraint_id));
 
     if (HasNewWork()) {
       return Retry(impl_const_id);
@@ -1668,24 +1690,29 @@ class ImportRefResolver {
     CARBON_CHECK(!import_impl.param_refs_id.is_valid() &&
                  !new_impl.param_refs_id.is_valid());
     SetGenericData(import_impl.generic_id, new_impl.generic_id, generic_data);
-    new_impl.self_id = context_.GetTypeIdForTypeConstant(self_const_id);
-    new_impl.constraint_id =
-        context_.GetTypeIdForTypeConstant(constraint_const_id);
+
+    // Create instructions for self and constraint to hold the symbolic constant
+    // value for a generic impl.
+    new_impl.self_id = AddLoadedImportRef(
+        context_, {.ir_id = import_ir_id_, .inst_id = import_impl.self_id},
+        SemIR::TypeId::TypeType, self_const_id);
+    new_impl.constraint_id = AddLoadedImportRef(
+        context_,
+        {.ir_id = import_ir_id_, .inst_id = import_impl.constraint_id},
+        SemIR::TypeId::TypeType, constraint_const_id);
 
     if (import_impl.is_defined()) {
       auto witness_id = AddImportRef(
-          context_, {.ir_id = import_ir_id_, .inst_id = import_impl.witness_id},
-          SemIR::EntityNameId::Invalid);
+          context_,
+          {.ir_id = import_ir_id_, .inst_id = import_impl.witness_id});
       AddImplDefinition(import_impl, new_impl, witness_id);
     }
 
     // If the `impl` is declared in the API file corresponding to the current
     // file, add this to impl lookup so that it can be found by redeclarations
     // in the current file.
     if (import_ir_id_ == SemIR::ImportIRId::ApiForImpl) {
-      context_.impls()
-          .GetOrAddLookupBucket(new_impl.self_id, new_impl.constraint_id)
-          .push_back(impl_id);
+      context_.impls().GetOrAddLookupBucket(new_impl).push_back(impl_id);
     }
 
     return ResolveAsConstant(impl_const_id);

diff --git a/toolchain/check/member_access.cpp b/toolchain/check/member_access.cpp
@@ -10,6 +10,7 @@
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/deduce.h"
 #include "toolchain/check/import_ref.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/sem_ir/generic.h"
@@ -177,18 +178,30 @@ static auto ScopeNeedsImplLookup(Context& context, LookupScope scope) -> bool {
 // Returns an invalid InstId if no matching impl is found.
 static auto LookupInterfaceWitness(Context& context,
                                    SemIR::ConstantId type_const_id,
-                                   SemIR::TypeId interface_type_id)
+                                   SemIR::ConstantId interface_const_id)
     -> SemIR::InstId {
   // TODO: Add a better impl lookup system. At the very least, we should only be
   // considering impls that are for the same interface we're querying. We can
   // also skip impls that mention any types that aren't part of our impl query.
   for (const auto& impl : context.impls().array_ref()) {
+    auto specific_id = SemIR::SpecificId::Invalid;
+    if (impl.generic_id.is_valid()) {
+      specific_id =
+          DeduceImplArguments(context, impl, type_const_id, interface_const_id);
+      if (!specific_id.is_valid()) {
+        continue;
+      }
+    }
     if (!context.constant_values().AreEqualAcrossDeclarations(
-            context.types().GetConstantId(impl.self_id), type_const_id)) {
+            SemIR::GetConstantValueInSpecific(context.sem_ir(), specific_id,
+                                              impl.self_id),
+            type_const_id)) {
       continue;
     }
-    if (!context.types().AreEqualAcrossDeclarations(impl.constraint_id,
-                                                    interface_type_id)) {
+    if (!context.constant_values().AreEqualAcrossDeclarations(
+            SemIR::GetConstantValueInSpecific(context.sem_ir(), specific_id,
+                                              impl.constraint_id),
+            interface_const_id)) {
       // TODO: An impl of a constraint type should be treated as implementing
       // the constraint's interfaces.
       continue;
@@ -198,7 +211,9 @@ static auto LookupInterfaceWitness(Context& context,
       return SemIR::InstId::Invalid;
     }
     LoadImportRef(context, impl.witness_id);
-    return impl.witness_id;
+    return context.constant_values().GetInstId(
+        SemIR::GetConstantValueInSpecific(context.sem_ir(), specific_id,
+                                          impl.witness_id));
   }
   return SemIR::InstId::Invalid;
 }
@@ -213,8 +228,8 @@ static auto PerformImplLookup(
   auto interface_type =
       context.types().GetAs<SemIR::InterfaceType>(assoc_type.interface_type_id);
   auto& interface = context.interfaces().Get(interface_type.interface_id);
-  auto witness_id = LookupInterfaceWitness(context, type_const_id,
-                                           assoc_type.interface_type_id);
+  auto witness_id = LookupInterfaceWitness(
+      context, type_const_id, assoc_type.interface_type_id.AsConstantId());
   if (!witness_id.is_valid()) {
     if (missing_impl_diagnoser) {
       CARBON_DIAGNOSTIC(MissingImplInMemberAccessNote, Note,

diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h
@@ -391,7 +391,7 @@ class NodeStack {
     set_id_if_category_is(Parse::NodeCategory::MemberName,
                           Id::KindFor<SemIR::NameId>());
     set_id_if_category_is(Parse::NodeCategory::ImplAs,
-                          Id::KindFor<SemIR::TypeId>());
+                          Id::KindFor<SemIR::InstId>());
     set_id_if_category_is(Parse::NodeCategory::Decl |
                               Parse::NodeCategory::Statement |
                               Parse::NodeCategory::Modifier,