Просмотр исходного кода

Start adding tracking of the complete list of IRs under check. (#3915)

As we talk about how imports should work, we want to start having
indirectly imported IRs in `import_irs`, where it's currently only
directly imported IRs. The tracking of `CheckIRId` is set up to be able
to determine whether an IR being indirectly imported is actually already
tracked (and may be a direct import). Changes here to how ApiForImpl is
handled start using the logic.

Note that indirect imports may be added while processing even the first
import from the current library. As a consequence, it's necessary to
prepare this map before starting imports, in particular, setting
ApiForImpl before any indirect imports may encounter the same import.
Also, prior validation that `num_irs` matches the final size are no
longer relevant.

check_ir_id is tracked on SemIR because I want to be able to figure it
out given an ImportIR, but the mapping is ephemeral after checking and
so I store that in Context.

I'm putting relevant logic into import_ref.cpp because the primary
alternative is import.cpp, and as more logic is added, ImportRefResolver
will be adding import IRs.
Jon Ross-Perkins 2 лет назад
Родитель
Сommit
c82ce8faae

+ 44 - 37
toolchain/check/check.cpp

@@ -165,13 +165,15 @@ struct UnitInfo {
     llvm::SmallVector<Import> imports;
   };
 
-  explicit UnitInfo(Unit& unit)
-      : unit(&unit),
+  explicit UnitInfo(SemIR::CheckIRId check_ir_id, Unit& unit)
+      : check_ir_id(check_ir_id),
+        unit(&unit),
         converter(unit.tokens, unit.tokens->source().filename(),
                   unit.parse_tree),
         err_tracker(*unit.consumer),
         emitter(converter, err_tracker) {}
 
+  SemIR::CheckIRId check_ir_id;
   Unit* unit;
 
   // Emitter information.
@@ -193,26 +195,29 @@ struct UnitInfo {
   // imports only touch `api` files.
   llvm::SmallVector<UnitInfo*> incoming_imports;
 
-  // True if this is an `impl` file and an `api` implicit import has
-  // successfully been added. Used for determining the number of import IRs.
-  bool has_api_for_impl = false;
+  // The corresponding `api` unit if this is an `impl` file. The entry should
+  // also be in the corresponding `PackageImports`.
+  UnitInfo* api_for_impl = nullptr;
 };
 
 // Add imports to the root block.
-static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
-    -> void {
+static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info,
+                                       int total_ir_count) -> void {
   // First create the constant values map for all imported IRs. We'll populate
   // these with mappings for namespaces as we go.
-  size_t num_irs = context.import_irs().size();
+  size_t num_irs = 0;
   for (auto& [_, package_imports] : unit_info.package_imports_map) {
     num_irs += package_imports.imports.size();
   }
-  if (unit_info.has_api_for_impl) {
-    // One of the IRs replaces ImportIRId::ApiForImpl.
-    --num_irs;
+  if (!unit_info.api_for_impl) {
+    // Leave an empty slot for ImportIRId::ApiForImpl.
+    ++num_irs;
   }
-  context.import_ir_constant_values().resize(
-      num_irs, SemIR::ConstantValueStore(SemIR::ConstantId::Invalid));
+
+  context.import_irs().Reserve(num_irs);
+  context.import_ir_constant_values().reserve(num_irs);
+
+  context.check_ir_map().resize(total_ir_count, SemIR::ImportIRId::Invalid);
 
   // Importing makes many namespaces, so only canonicalize the type once.
   auto namespace_type_id =
@@ -231,19 +236,26 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
                         SemIR::InstId::Invalid}});
   CARBON_CHECK(package_inst_id == SemIR::InstId::PackageNamespace);
 
+  // If there is an implicit `api` import, set it first so that it uses the
+  // ImportIRId::ApiForImpl when processed for imports.
+  if (unit_info.api_for_impl) {
+    SetApiImportIR(
+        context,
+        {.node_id = context.parse_tree().packaging_directive()->names.node_id,
+         .sem_ir = &**unit_info.api_for_impl->unit->sem_ir});
+  } else {
+    SetApiImportIR(context,
+                   {.node_id = Parse::InvalidNodeId(), .sem_ir = nullptr});
+  }
+
   // Add imports from the current package.
   auto self_import = unit_info.package_imports_map.find(IdentifierId::Invalid);
   if (self_import != unit_info.package_imports_map.end()) {
     bool error_in_import = self_import->second.has_load_error;
-    const auto& packaging = context.parse_tree().packaging_directive();
-    auto current_library_id =
-        packaging ? packaging->names.library_id : StringLiteralValueId::Invalid;
     for (const auto& import : self_import->second.imports) {
-      bool is_api_for_impl = current_library_id == import.names.library_id;
       const auto& import_sem_ir = **import.unit_info->unit->sem_ir;
       ImportLibraryFromCurrentPackage(context, namespace_type_id,
-                                      import.names.node_id, is_api_for_impl,
-                                      import_sem_ir);
+                                      import.names.node_id, import_sem_ir);
       error_in_import |= import_sem_ir.name_scopes()
                              .Get(SemIR::NameScopeId::Package)
                              .has_error;
@@ -277,10 +289,6 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
                                     package_imports.node_id, package_id,
                                     import_irs, package_imports.has_load_error);
   }
-
-  CARBON_CHECK(context.import_irs().size() == num_irs)
-      << "Created an unexpected number of IRs: expected " << num_irs
-      << ", have " << context.import_irs().size();
 }
 
 namespace {
@@ -728,9 +736,10 @@ static auto ProcessNodeIds(Context& context, llvm::raw_ostream* vlog_stream,
 static auto CheckParseTree(
     llvm::DenseMap<const SemIR::File*, Parse::NodeLocConverter*>*
         node_converters,
-    UnitInfo& unit_info, llvm::raw_ostream* vlog_stream) -> void {
+    UnitInfo& unit_info, int total_ir_count, llvm::raw_ostream* vlog_stream)
+    -> void {
   unit_info.unit->sem_ir->emplace(
-      *unit_info.unit->value_stores,
+      unit_info.check_ir_id, *unit_info.unit->value_stores,
       unit_info.unit->tokens->source().filename().str());
 
   // For ease-of-access.
@@ -747,7 +756,7 @@ static auto CheckParseTree(
   // Add a block for the file.
   context.inst_block_stack().Push();
 
-  InitPackageScopeAndImports(context, unit_info);
+  InitPackageScopeAndImports(context, unit_info, total_ir_count);
 
   // Import all impls declared in imports.
   // TODO: Do this selectively when we see an impl query.
@@ -924,10 +933,10 @@ static auto TrackImport(
     ++unit_info.imports_remaining;
     api->second->incoming_imports.push_back(&unit_info);
 
-    // If this is the implicit import, note it was successfully imported.
+    // If this is the implicit import, note we have it.
     if (!explicit_import_map) {
-      CARBON_CHECK(!import.package_id.is_valid());
-      unit_info.has_api_for_impl = true;
+      CARBON_CHECK(!unit_info.api_for_impl);
+      unit_info.api_for_impl = api->second;
     }
   } else {
     // The imported api is missing.
@@ -1040,8 +1049,8 @@ auto CheckParseTrees(llvm::MutableArrayRef<Unit> units, bool prelude_import,
   // UnitInfo is big due to its SmallVectors, so we default to 0 on the stack.
   llvm::SmallVector<UnitInfo, 0> unit_infos;
   unit_infos.reserve(units.size());
-  for (auto& unit : units) {
-    unit_infos.emplace_back(unit);
+  for (auto [i, unit] : llvm::enumerate(units)) {
+    unit_infos.emplace_back(SemIR::CheckIRId(i), unit);
   }
 
   llvm::DenseMap<ImportKey, UnitInfo*> api_map =
@@ -1093,7 +1102,7 @@ auto CheckParseTrees(llvm::MutableArrayRef<Unit> units, bool prelude_import,
   for (int check_index = 0;
        check_index < static_cast<int>(ready_to_check.size()); ++check_index) {
     auto* unit_info = ready_to_check[check_index];
-    CheckParseTree(&node_converters, *unit_info, vlog_stream);
+    CheckParseTree(&node_converters, *unit_info, units.size(), vlog_stream);
     for (auto* incoming_import : unit_info->incoming_imports) {
       --incoming_import->imports_remaining;
       if (incoming_import->imports_remaining == 0) {
@@ -1110,7 +1119,6 @@ auto CheckParseTrees(llvm::MutableArrayRef<Unit> units, bool prelude_import,
     // on a part of it.
     // TODO: Better identify cycles, maybe try to untangle them.
     for (auto& unit_info : unit_infos) {
-      const auto& packaging = unit_info.unit->parse_tree->packaging_directive();
       if (unit_info.imports_remaining > 0) {
         for (auto& [package_id, package_imports] :
              unit_info.package_imports_map) {
@@ -1128,9 +1136,8 @@ auto CheckParseTrees(llvm::MutableArrayRef<Unit> units, bool prelude_import,
                                      ImportCycleDetected);
               // Make this look the same as an import which wasn't found.
               package_imports.has_load_error = true;
-              if (packaging && !package_id.is_valid() &&
-                  packaging->names.library_id == import_it->names.library_id) {
-                unit_info.has_api_for_impl = false;
+              if (unit_info.api_for_impl == import_it->unit_info) {
+                unit_info.api_for_impl = nullptr;
               }
               import_it = package_imports.imports.erase(import_it);
             }
@@ -1143,7 +1150,7 @@ auto CheckParseTrees(llvm::MutableArrayRef<Unit> units, bool prelude_import,
     // incomplete imports.
     for (auto& unit_info : unit_infos) {
       if (unit_info.imports_remaining > 0) {
-        CheckParseTree(&node_converters, unit_info, vlog_stream);
+        CheckParseTree(&node_converters, unit_info, units.size(), vlog_stream);
       }
     }
   }

+ 1 - 19
toolchain/check/context.cpp

@@ -153,24 +153,6 @@ auto Context::ReplaceInstBeforeConstantUse(SemIR::InstId inst_id,
   constant_values().Set(inst_id, const_id);
 }
 
-auto Context::AddImportRef(SemIR::ImportIRInst import_ir_inst)
-    -> SemIR::InstId {
-  auto import_ir_inst_id = import_ir_insts().Add(import_ir_inst);
-  auto import_ref_id = AddPlaceholderInstInNoBlock(
-      {import_ir_inst_id, SemIR::ImportRefUnloaded{import_ir_inst_id}});
-
-  // We can't insert this instruction into whatever block we happen to be in,
-  // because this function is typically called by name lookup in the middle of
-  // an otherwise unknown checking step. But we need to add the instruction
-  // somewhere, because it's referenced by other instructions and needs to be
-  // visible in textual IR. Adding it to the file block is arbitrary but is the
-  // best place we have right now.
-  //
-  // TODO: Consider adding a dedicated block for import_refs.
-  inst_block_stack().AddInstIdToFileBlock(import_ref_id);
-  return import_ref_id;
-}
-
 auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def)
     -> void {
   CARBON_DIAGNOSTIC(NameDeclDuplicate, Error,
@@ -342,7 +324,7 @@ static auto LookupInImportIRScopes(Context& context, SemIRLoc loc,
       continue;
     }
     auto import_inst_id =
-        context.AddImportRef({.ir_id = import_ir_id, .inst_id = it->second});
+        AddImportRef(context, {.ir_id = import_ir_id, .inst_id = it->second});
     if (result_id.is_valid()) {
       MergeImportRef(context, import_inst_id, result_id);
     } else {

+ 7 - 4
toolchain/check/context.h

@@ -82,10 +82,6 @@ class Context {
   auto ReplaceInstBeforeConstantUse(SemIR::InstId inst_id, SemIR::Inst inst)
       -> void;
 
-  // Adds an import_ref instruction for the specified instruction in the
-  // specified IR. The import_ref is initially marked as unused.
-  auto AddImportRef(SemIR::ImportIRInst import_ir_inst) -> SemIR::InstId;
-
   // Sets only the parse node of an instruction. This is only used when setting
   // the parse node of an imported namespace. Versus
   // ReplaceInstBeforeConstantUse, it is safe to use after the namespace is used
@@ -315,6 +311,10 @@ class Context {
     return scope_stack().break_continue_stack();
   }
 
+  auto check_ir_map() -> llvm::SmallVector<SemIR::ImportIRId>& {
+    return check_ir_map_;
+  }
+
   auto import_ir_constant_values()
       -> llvm::SmallVector<SemIR::ConstantValueStore, 0>& {
     return import_ir_constant_values_;
@@ -434,6 +434,9 @@ class Context {
   // The list which will form NodeBlockId::Exports.
   llvm::SmallVector<SemIR::InstId> exports_;
 
+  // Maps CheckIRId to ImportIRId.
+  llvm::SmallVector<SemIR::ImportIRId> check_ir_map_;
+
   // Per-import constant values. These refer to the main IR and mainly serve as
   // a lookup table for quick access.
   //

+ 5 - 13
toolchain/check/import.cpp

@@ -7,6 +7,7 @@
 #include "common/check.h"
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/context.h"
+#include "toolchain/check/import_ref.h"
 #include "toolchain/check/merge.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/sem_ir/file.h"
@@ -212,18 +213,9 @@ static auto CopyEnclosingNameScopesFromImportIR(
 auto ImportLibraryFromCurrentPackage(Context& context,
                                      SemIR::TypeId namespace_type_id,
                                      Parse::ImportDirectiveId node_id,
-                                     bool is_api_for_impl,
                                      const SemIR::File& import_sem_ir) -> void {
-  auto ir_id = SemIR::ImportIRId::Invalid;
-  if (is_api_for_impl) {
-    ir_id = SemIR::ImportIRId::ApiForImpl;
-    auto& import_ir = context.import_irs().Get(ir_id);
-    CARBON_CHECK(import_ir.sem_ir == nullptr) << "ApiForImpl is only set once";
-    import_ir = {.node_id = node_id, .sem_ir = &import_sem_ir};
-  } else {
-    ir_id = context.import_irs().Add(
-        {.node_id = node_id, .sem_ir = &import_sem_ir});
-  }
+  auto ir_id =
+      AddImportIR(context, {.node_id = node_id, .sem_ir = &import_sem_ir});
 
   context.import_ir_constant_values()[ir_id.index].Set(
       SemIR::InstId::PackageNamespace,
@@ -256,7 +248,7 @@ auto ImportLibraryFromCurrentPackage(Context& context,
     } else {
       // Leave a placeholder that the inst comes from the other IR.
       auto target_id =
-          context.AddImportRef({.ir_id = ir_id, .inst_id = import_inst_id});
+          AddImportRef(context, {.ir_id = ir_id, .inst_id = import_inst_id});
       auto [it, success] = context.name_scopes()
                                .Get(enclosing_scope_id)
                                .names.insert({name_id, target_id});
@@ -285,7 +277,7 @@ auto ImportLibrariesFromOtherPackage(Context& context,
   auto& scope = context.name_scopes().Get(namespace_scope_id);
   scope.is_closed_import = !is_duplicate;
   for (auto import_ir : import_irs) {
-    auto ir_id = context.import_irs().Add(import_ir);
+    auto ir_id = AddImportIR(context, import_ir);
     scope.import_ir_scopes.push_back({ir_id, SemIR::NameScopeId::Package});
     context.import_ir_constant_values()[ir_id.index].Set(
         SemIR::InstId::PackageNamespace, namespace_const_id);

+ 0 - 1
toolchain/check/import.h

@@ -17,7 +17,6 @@ namespace Carbon::Check {
 auto ImportLibraryFromCurrentPackage(Context& context,
                                      SemIR::TypeId namespace_type_id,
                                      Parse::ImportDirectiveId node_id,
-                                     bool is_api_for_impl,
                                      const SemIR::File& import_sem_ir) -> void;
 
 // Adds another package's imports to name lookup, with all libraries together.

+ 56 - 7
toolchain/check/import_ref.cpp

@@ -17,6 +17,55 @@
 
 namespace Carbon::Check {
 
+// Adds the ImportIR, excluding the update to the check_ir_map.
+static auto InternalAddImportIR(Context& context, SemIR::ImportIR import_ir)
+    -> SemIR::ImportIRId {
+  context.import_ir_constant_values().push_back(
+      SemIR::ConstantValueStore(SemIR::ConstantId::Invalid));
+  return context.import_irs().Add(import_ir);
+}
+
+auto SetApiImportIR(Context& context, SemIR::ImportIR import_ir) -> void {
+  auto ir_id = SemIR::ImportIRId::Invalid;
+  if (import_ir.sem_ir != nullptr) {
+    ir_id = AddImportIR(context, import_ir);
+  } else {
+    // We don't have a check_ir_id, so add without touching check_ir_map.
+    ir_id = InternalAddImportIR(context, import_ir);
+  }
+  CARBON_CHECK(ir_id == SemIR::ImportIRId::ApiForImpl)
+      << "ApiForImpl must be the first IR";
+}
+
+auto AddImportIR(Context& context, SemIR::ImportIR import_ir)
+    -> SemIR::ImportIRId {
+  auto& ir_id = context.check_ir_map()[import_ir.sem_ir->check_ir_id().index];
+  if (ir_id.is_valid()) {
+    return ir_id;
+  }
+  // Note this updates check_ir_map.
+  ir_id = InternalAddImportIR(context, import_ir);
+  return ir_id;
+}
+
+auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst)
+    -> SemIR::InstId {
+  auto import_ir_inst_id = context.import_ir_insts().Add(import_ir_inst);
+  auto import_ref_id = context.AddPlaceholderInstInNoBlock(
+      {import_ir_inst_id, SemIR::ImportRefUnloaded{import_ir_inst_id}});
+
+  // We can't insert this instruction into whatever block we happen to be in,
+  // because this function is typically called by name lookup in the middle of
+  // an otherwise unknown checking step. But we need to add the instruction
+  // somewhere, because it's referenced by other instructions and needs to be
+  // visible in textual IR. Adding it to the file block is arbitrary but is the
+  // best place we have right now.
+  //
+  // TODO: Consider adding a dedicated block for import_refs.
+  context.inst_block_stack().AddInstIdToFileBlock(import_ref_id);
+  return import_ref_id;
+}
+
 // Resolves an instruction from an imported IR into a constant referring to the
 // current IR.
 //
@@ -338,8 +387,8 @@ class ImportRefResolver {
   auto AddNameScopeImportRefs(const SemIR::NameScope& import_scope,
                               SemIR::NameScope& new_scope) -> void {
     for (auto [entry_name_id, entry_inst_id] : import_scope.names) {
-      auto ref_id = context_.AddImportRef(
-          {.ir_id = import_ir_id_, .inst_id = entry_inst_id});
+      auto ref_id = AddImportRef(
+          context_, {.ir_id = import_ir_id_, .inst_id = entry_inst_id});
       CARBON_CHECK(
           new_scope.names.insert({GetLocalNameId(entry_name_id), ref_id})
               .second);
@@ -359,7 +408,7 @@ class ImportRefResolver {
     new_associated_entities.reserve(associated_entities.size());
     for (auto inst_id : associated_entities) {
       new_associated_entities.push_back(
-          context_.AddImportRef({.ir_id = import_ir_id_, .inst_id = inst_id}));
+          AddImportRef(context_, {.ir_id = import_ir_id_, .inst_id = inst_id}));
     }
     return context_.inst_blocks().Add(new_associated_entities);
   }
@@ -456,8 +505,8 @@ class ImportRefResolver {
     }
 
     // Add a lazy reference to the target declaration.
-    auto decl_id = context_.AddImportRef(
-        {.ir_id = import_ir_id_, .inst_id = inst.decl_id});
+    auto decl_id = AddImportRef(
+        context_, {.ir_id = import_ir_id_, .inst_id = inst.decl_id});
 
     auto inst_id = context_.AddInstInNoBlock(
         {AddImportIRInst(inst.decl_id),
@@ -1062,8 +1111,8 @@ static auto ImportImpl(Context& context, SemIR::ImportIRId import_ir_id,
     // TODO: Consider importing the definition_id.
 
     auto& impl = context.impls().Get(impl_id);
-    impl.witness_id = context.AddImportRef(
-        {.ir_id = import_ir_id, .inst_id = import_impl.witness_id});
+    impl.witness_id = AddImportRef(
+        context, {.ir_id = import_ir_id, .inst_id = import_impl.witness_id});
   }
 }
 

+ 14 - 0
toolchain/check/import_ref.h

@@ -7,9 +7,23 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/sem_ir/file.h"
+#include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::Check {
 
+// Sets the IR for ImportIRId::ApiForImpl. Should be called before AddImportIR
+// in order to ensure the correct ID is assigned.
+auto SetApiImportIR(Context& context, SemIR::ImportIR import_ir) -> void;
+
+// Adds an ImportIR, returning the ID. May use an existing ID if already added.
+auto AddImportIR(Context& context, SemIR::ImportIR import_ir)
+    -> SemIR::ImportIRId;
+
+// Adds an import_ref instruction for the specified instruction in the
+// specified IR. The import_ref is initially marked as unused.
+auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst)
+    -> SemIR::InstId;
+
 // If the passed in instruction ID is an ImportRefUnloaded, loads it for use.
 // The result will be an ImportRefUsed.
 auto LoadImportRef(Context& context, SemIR::InstId inst_id, SemIRLoc loc)

+ 4 - 7
toolchain/sem_ir/file.cpp

@@ -62,18 +62,15 @@ auto TypeInfo::Print(llvm::raw_ostream& out) const -> void {
   out << "{constant: " << constant_id << ", value_rep: " << value_repr << "}";
 }
 
-File::File(SharedValueStores& value_stores, std::string filename)
-    : value_stores_(&value_stores),
+File::File(CheckIRId check_ir_id, SharedValueStores& value_stores,
+           std::string filename)
+    : check_ir_id_(check_ir_id),
+      value_stores_(&value_stores),
       filename_(std::move(filename)),
       type_blocks_(allocator_),
       constant_values_(ConstantId::NotConstant),
       inst_blocks_(allocator_),
       constants_(*this, allocator_) {
-  auto api_placeholder_id =
-      import_irs_.Add({.node_id = Parse::NodeId::Invalid, .sem_ir = nullptr});
-  CARBON_CHECK(api_placeholder_id == ImportIRId::ApiForImpl)
-      << "ApiForImpl must be the first IR";
-
   insts_.Reserve(BuiltinKind::ValidCount);
 // Error uses a self-referential type so that it's not accidentally treated as
 // a normal type. Every other builtin is a type, including the

+ 6 - 1
toolchain/sem_ir/file.h

@@ -47,7 +47,8 @@ class File;
 class File : public Printable<File> {
  public:
   // Starts a new file for Check::CheckParseTree.
-  explicit File(SharedValueStores& value_stores, std::string filename);
+  explicit File(CheckIRId check_ir_id, SharedValueStores& value_stores,
+                std::string filename);
 
   File(const File&) = delete;
   auto operator=(const File&) -> File& = delete;
@@ -98,6 +99,8 @@ class File : public Printable<File> {
   // type expression rather than a canonical type.
   auto StringifyTypeExpr(InstId outer_inst_id) const -> std::string;
 
+  auto check_ir_id() const -> CheckIRId { return check_ir_id_; }
+
   // Directly expose SharedValueStores members.
   auto identifiers() -> StringStoreWrapper<IdentifierId>& {
     return value_stores_->identifiers();
@@ -192,6 +195,8 @@ class File : public Printable<File> {
  private:
   bool has_errors_ = false;
 
+  CheckIRId check_ir_id_;
+
   // Shared, compile-scoped values.
   SharedValueStores* value_stores_;
 

+ 11 - 1
toolchain/sem_ir/ids.h

@@ -220,6 +220,16 @@ struct FunctionId : public IdBase, public Printable<FunctionId> {
 
 constexpr FunctionId FunctionId::Invalid = FunctionId(InvalidIndex);
 
+// The ID of an IR within the set of all IRs being evaluated in the current
+// check execution.
+struct CheckIRId : public IdBase, public Printable<CheckIRId> {
+  using IdBase::IdBase;
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "check_ir";
+    IdBase::Print(out);
+  }
+};
+
 // The ID of a class.
 struct ClassId : public IdBase, public Printable<ClassId> {
   using ValueType = Class;
@@ -268,7 +278,7 @@ struct ImplId : public IdBase, public Printable<ImplId> {
 
 constexpr ImplId ImplId::Invalid = ImplId(InvalidIndex);
 
-// The ID of an imported IR.
+// The ID of an IR within the set of imported IRs, both direct and indirect.
 struct ImportIRId : public IdBase, public Printable<ImportIRId> {
   using ValueType = ImportIR;