Преглед изворни кода

Rename the ImportRefs block to Imports (#5618)

I've been mulling the name of this, changing it and updating comments to
try and better reflect the current semantic. "Imports" reflects how
we're currently printing this in SemIR.
Jon Ross-Perkins пре 11 месеци
родитељ
комит
1e9e148c3b

+ 2 - 2
toolchain/check/check_unit.cpp

@@ -590,8 +590,8 @@ auto CheckUnit::FinishRun() -> void {
   context_.inst_blocks().ReplacePlaceholder(SemIR::InstBlockId::Exports,
                                             context_.exports());
   // Finalizes the ImportRef inst block.
-  context_.inst_blocks().ReplacePlaceholder(SemIR::InstBlockId::ImportRefs,
-                                            context_.import_ref_ids());
+  context_.inst_blocks().ReplacePlaceholder(SemIR::InstBlockId::Imports,
+                                            context_.imports());
   // Finalizes __global_init.
   context_.global_init().Finalize();
 

+ 10 - 11
toolchain/check/context.h

@@ -161,9 +161,7 @@ class Context {
 
   auto global_init() -> GlobalInit& { return global_init_; }
 
-  auto import_ref_ids() -> llvm::SmallVector<SemIR::InstId>& {
-    return import_ref_ids_;
-  }
+  auto imports() -> llvm::SmallVector<SemIR::InstId>& { return imports_; }
 
   // Pre-computed parts of a binding pattern.
   // TODO: Consider putting this behind a narrower API to guard against emitting
@@ -352,7 +350,8 @@ class Context {
   // defined, regardless of whether the class can have virtual functions.
   InstBlockStack vtable_stack_;
 
-  // The list which will form NodeBlockId::Exports.
+  // Instructions which are operands to an `export` directive. This becomes
+  // `InstBlockId::Exports`.
   llvm::SmallVector<SemIR::InstId> exports_;
 
   // Maps CheckIRId to ImportIRId.
@@ -377,14 +376,14 @@ class Context {
   // State for global initialization.
   GlobalInit global_init_;
 
-  // A list of import refs which can't be inserted into their current context.
-  // They're typically added during name lookup or import ref resolution, where
-  // the current block on inst_block_stack_ is unrelated.
+  // Instructions which are generated as a result of imports; both `ImportRef`s
+  // and instructions they generate. For example, when a name reference resolves
+  // an imported function, the `ImportRefLoaded` results in a `FunctionDecl`,
+  // and both end up here. The `FunctionDecl` shouldn't use the current block on
+  // inst_block_stack_ because it's not tied to the referencing scope.
   //
-  // These are instead added here because they're referenced by other
-  // instructions and needs to be visible in textual IR.
-  // FinalizeImportRefBlock() will produce an inst block for them.
-  llvm::SmallVector<SemIR::InstId> import_ref_ids_;
+  // This becomes `InstBlockId::Imports`.
+  llvm::SmallVector<SemIR::InstId> imports_;
 
   // Map from an AnyBindingPattern inst to precomputed parts of the
   // pattern-match SemIR for it.

+ 2 - 2
toolchain/check/import.cpp

@@ -139,7 +139,7 @@ auto AddImportNamespace(Context& context, SemIR::TypeId namespace_type_id,
   AddImportNamespaceResult result = {
       .name_scope_id = SemIR::NameScopeId::None,
       .inst_id = AddPlaceholderInstInNoBlock(context, namespace_inst_and_loc)};
-  context.import_ref_ids().push_back(result.inst_id);
+  context.imports().push_back(result.inst_id);
   namespace_inst.name_scope_id =
       context.name_scopes().Add(result.inst_id, name_id, parent_scope_id);
   result.name_scope_id = namespace_inst.name_scope_id;
@@ -236,7 +236,7 @@ static auto CopySingleNameScopeFromImportIR(
                      {.type_id = namespace_type_id,
                       .import_ir_inst_id = import_ir_inst_id,
                       .entity_name_id = entity_name_id}));
-    context.import_ref_ids().push_back(inst_id);
+    context.imports().push_back(inst_id);
     return inst_id;
   };
   AddImportNamespaceToScopeResult result = AddImportNamespaceToScope(

+ 2 - 2
toolchain/check/import_cpp.cpp

@@ -460,7 +460,7 @@ static auto ImportFunctionDecl(Context& context, SemIR::LocId loc_id,
       SemIR::TypeId::None, SemIR::FunctionId::None, SemIR::InstBlockId::Empty};
   auto decl_id =
       AddPlaceholderInstInNoBlock(context, Parse::NodeId::None, function_decl);
-  context.import_ref_ids().push_back(decl_id);
+  context.imports().push_back(decl_id);
 
   auto function_info = SemIR::Function{
       {.name_id = name_id,
@@ -520,7 +520,7 @@ static auto BuildClassDecl(Context& context, SemIR::NameScopeId parent_scope_id,
   // TODO: Consider setting a proper location.
   auto class_decl_id = AddPlaceholderInstInNoBlock(
       context, SemIR::LocIdAndInst::NoLoc(class_decl));
-  context.import_ref_ids().push_back(class_decl_id);
+  context.imports().push_back(class_decl_id);
 
   SemIR::Class class_info = {
       {.name_id = name_id,

+ 3 - 3
toolchain/check/import_ref.cpp

@@ -87,7 +87,7 @@ auto AddImportRef(Context& context, SemIR::ImportIRInst import_ir_inst,
   // ImportRefs have a dedicated block because this may be called during
   // processing where the instruction shouldn't be inserted in the current inst
   // block.
-  context.import_ref_ids().push_back(import_ref_id);
+  context.imports().push_back(import_ref_id);
   return import_ref_id;
 }
 
@@ -104,7 +104,7 @@ static auto AddLoadedImportRef(Context& context, SemIR::TypeId type_id,
                                  .entity_name_id = SemIR::EntityNameId::None};
   auto inst_id = AddPlaceholderInstInNoBlock(
       context, MakeImportedLocIdAndInst(context, import_ir_inst_id, inst));
-  context.import_ref_ids().push_back(inst_id);
+  context.imports().push_back(inst_id);
 
   context.constant_values().Set(inst_id, const_id);
   context.import_ir_constant_values()[import_ir_inst.ir_id().index].Set(
@@ -674,7 +674,7 @@ static auto AddPlaceholderImportedInst(ImportContext& context,
                  "AddPlaceholderImportedInst: {0}\n", inst);
   // Track the instruction in the imports block so that it's included in
   // formatted SemIR if it's referenced.
-  context.local_context().import_ref_ids().push_back(inst_id);
+  context.local_context().imports().push_back(inst_id);
   return inst_id;
 }
 

+ 1 - 1
toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon

@@ -69,7 +69,7 @@
 // CHECK:STDOUT:   inst_blocks:
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:         {}
-// CHECK:STDOUT:     import_refs:     {}
+// CHECK:STDOUT:     imports:         {}
 // CHECK:STDOUT:     global_init:     {}
 // CHECK:STDOUT:     inst_block4:
 // CHECK:STDOUT:       0:               inst14

+ 2 - 2
toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon

@@ -72,7 +72,7 @@ fn B() {
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:
 // CHECK:STDOUT:       0:               inst15
-// CHECK:STDOUT:     import_refs:     {}
+// CHECK:STDOUT:     imports:         {}
 // CHECK:STDOUT:     global_init:     {}
 // CHECK:STDOUT:     inst_block4:     {}
 // CHECK:STDOUT:     inst_block5:
@@ -151,7 +151,7 @@ fn B() {
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:
 // CHECK:STDOUT:       0:               inst17
-// CHECK:STDOUT:     import_refs:
+// CHECK:STDOUT:     imports:
 // CHECK:STDOUT:       0:               inst16
 // CHECK:STDOUT:       1:               inst22
 // CHECK:STDOUT:       2:               inst23

+ 2 - 2
toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon

@@ -72,7 +72,7 @@ fn B() {
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:
 // CHECK:STDOUT:       0:               inst15
-// CHECK:STDOUT:     import_refs:     {}
+// CHECK:STDOUT:     imports:         {}
 // CHECK:STDOUT:     global_init:     {}
 // CHECK:STDOUT:     inst_block4:     {}
 // CHECK:STDOUT:     inst_block5:
@@ -171,7 +171,7 @@ fn B() {
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:
 // CHECK:STDOUT:       0:               inst17
-// CHECK:STDOUT:     import_refs:
+// CHECK:STDOUT:     imports:
 // CHECK:STDOUT:       0:               inst16
 // CHECK:STDOUT:       1:               inst22
 // CHECK:STDOUT:       2:               inst23

+ 1 - 1
toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon

@@ -169,7 +169,7 @@ fn Foo[T:! type](n: T) -> (T, ()) {
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:
 // CHECK:STDOUT:       0:               inst42
-// CHECK:STDOUT:     import_refs:     {}
+// CHECK:STDOUT:     imports:         {}
 // CHECK:STDOUT:     global_init:     {}
 // CHECK:STDOUT:     inst_block4:     {}
 // CHECK:STDOUT:     inst_block5:

+ 1 - 1
toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon

@@ -122,7 +122,7 @@ fn Foo(n: ()) -> ((), ()) {
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:
 // CHECK:STDOUT:       0:               inst36
-// CHECK:STDOUT:     import_refs:     {}
+// CHECK:STDOUT:     imports:         {}
 // CHECK:STDOUT:     global_init:     {}
 // CHECK:STDOUT:     inst_block4:
 // CHECK:STDOUT:       0:               inst16

+ 1 - 1
toolchain/driver/testdata/stdin.carbon

@@ -54,7 +54,7 @@
 // CHECK:STDOUT:   inst_blocks:
 // CHECK:STDOUT:     inst_block_empty: {}
 // CHECK:STDOUT:     exports:         {}
-// CHECK:STDOUT:     import_refs:     {}
+// CHECK:STDOUT:     imports:         {}
 // CHECK:STDOUT:     global_init:     {}
 // CHECK:STDOUT:     inst_block4:
 // CHECK:STDOUT:       0:               inst14

+ 3 - 3
toolchain/sem_ir/formatter.cpp

@@ -53,7 +53,7 @@ Formatter::Formatter(const File* sem_ir,
   // Create empty placeholder chunks for instructions that we output lazily.
   for (auto inst_id : llvm::concat<const InstId>(
            sem_ir_->constants().array_ref(),
-           sem_ir_->inst_blocks().Get(InstBlockId::ImportRefs))) {
+           sem_ir_->inst_blocks().Get(InstBlockId::Imports))) {
     tentative_inst_chunks_[inst_id.index] = AddChunkNoFlush(false);
   }
 
@@ -67,8 +67,8 @@ auto Formatter::Format() -> void {
   FormatTopLevelScopeIfUsed(InstNamer::ScopeId::Constants,
                             sem_ir_->constants().array_ref(),
                             /*use_tentative_output_scopes=*/true);
-  FormatTopLevelScopeIfUsed(InstNamer::ScopeId::ImportRefs,
-                            sem_ir_->inst_blocks().Get(InstBlockId::ImportRefs),
+  FormatTopLevelScopeIfUsed(InstNamer::ScopeId::Imports,
+                            sem_ir_->inst_blocks().Get(InstBlockId::Imports),
                             /*use_tentative_output_scopes=*/true);
   FormatTopLevelScopeIfUsed(
       InstNamer::ScopeId::File,

+ 2 - 2
toolchain/sem_ir/ids.cpp

@@ -121,8 +121,8 @@ auto InstBlockId::Print(llvm::raw_ostream& out) const -> void {
     out << Label << "_empty";
   } else if (*this == Exports) {
     out << "exports";
-  } else if (*this == ImportRefs) {
-    out << "import_refs";
+  } else if (*this == Imports) {
+    out << "imports";
   } else if (*this == GlobalInit) {
     out << "global_init";
   } else {

+ 4 - 4
toolchain/sem_ir/ids.h

@@ -622,9 +622,9 @@ struct InstBlockId : public IdBase<InstBlockId> {
   // state is in the Check::Context.
   static const InstBlockId Exports;
 
-  // ImportRef instructions. Empty until the File is fully checked; intermediate
-  // state is in the Check::Context.
-  static const InstBlockId ImportRefs;
+  // Instructions produced through import logic. Empty until the File is fully
+  // checked; intermediate state is in the Check::Context.
+  static const InstBlockId Imports;
 
   // Global declaration initialization instructions. Empty if none are present.
   // Otherwise, __global_init function will be generated and this block will
@@ -640,7 +640,7 @@ struct InstBlockId : public IdBase<InstBlockId> {
 
 constexpr InstBlockId InstBlockId::Empty = InstBlockId(0);
 constexpr InstBlockId InstBlockId::Exports = InstBlockId(1);
-constexpr InstBlockId InstBlockId::ImportRefs = InstBlockId(2);
+constexpr InstBlockId InstBlockId::Imports = InstBlockId(2);
 constexpr InstBlockId InstBlockId::GlobalInit = InstBlockId(3);
 constexpr InstBlockId InstBlockId::Unreachable = InstBlockId(NoneIndex - 1);
 

+ 2 - 2
toolchain/sem_ir/inst.h

@@ -573,8 +573,8 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {
       : BaseType(allocator) {
     auto exports_id = AddPlaceholder();
     CARBON_CHECK(exports_id == InstBlockId::Exports);
-    auto import_refs_id = AddPlaceholder();
-    CARBON_CHECK(import_refs_id == InstBlockId::ImportRefs);
+    auto imports_id = AddPlaceholder();
+    CARBON_CHECK(imports_id == InstBlockId::Imports);
     auto global_init_id = AddPlaceholder();
     CARBON_CHECK(global_init_id == InstBlockId::GlobalInit);
   }

+ 6 - 6
toolchain/sem_ir/inst_namer.cpp

@@ -86,9 +86,9 @@ InstNamer::InstNamer(const File* sem_ir) : sem_ir_(sem_ir) {
   // Build the constants scope.
   CollectNamesInBlock(ScopeId::Constants, sem_ir->constants().array_ref());
 
-  // Build the ImportRef scope.
-  CollectNamesInBlock(ScopeId::ImportRefs,
-                      sem_ir->inst_blocks().Get(InstBlockId::ImportRefs));
+  // Build the imports scope.
+  CollectNamesInBlock(ScopeId::Imports,
+                      sem_ir->inst_blocks().Get(InstBlockId::Imports));
 
   // Build the file scope.
   CollectNamesInBlock(ScopeId::File, sem_ir->top_inst_block_id());
@@ -213,7 +213,7 @@ auto InstNamer::GetScopeName(ScopeId scope) const -> std::string {
     // These are treated as SemIR keywords.
     case ScopeId::File:
       return "file";
-    case ScopeId::ImportRefs:
+    case ScopeId::Imports:
       return "imports";
     case ScopeId::Constants:
       return "constants";
@@ -533,7 +533,7 @@ auto InstNamer::NamingContext::AddInstName(std::string name) -> void {
   ScopeId old_scope_id = inst_namer_->insts_[inst_id_.index].first;
   if (old_scope_id == ScopeId::None) {
     std::variant<LocId, uint64_t> loc_id_or_fingerprint = LocId::None;
-    if (scope_id_ == ScopeId::Constants || scope_id_ == ScopeId::ImportRefs) {
+    if (scope_id_ == ScopeId::Constants || scope_id_ == ScopeId::Imports) {
       loc_id_or_fingerprint =
           inst_namer_->fingerprinter_.GetOrCompute(&sem_ir(), inst_id_);
     } else {
@@ -855,7 +855,7 @@ auto InstNamer::NamingContext::NameInst() -> void {
       if (const_id.has_value() && const_id.is_concrete()) {
         auto const_inst_id = sem_ir().constant_values().GetInstId(const_id);
         if (!inst_namer_->insts_[const_inst_id.index].second) {
-          inst_namer_->QueueBlockInsts(*queue_, ScopeId::ImportRefs,
+          inst_namer_->QueueBlockInsts(*queue_, ScopeId::Imports,
                                        llvm::ArrayRef(const_inst_id));
         }
       }

+ 1 - 1
toolchain/sem_ir/inst_namer.h

@@ -23,7 +23,7 @@ class InstNamer {
     None = -1,
     // The three top-level scopes.
     File = 0,
-    ImportRefs = 1,
+    Imports = 1,
     Constants = 2,
     // The first entity scope; see entities in `ScopeIdTypeEnum`.
     FirstEntityScope = 3,

+ 1 - 1
toolchain/sem_ir/yaml_test.cpp

@@ -91,7 +91,7 @@ TEST(SemIRTest, Yaml) {
            Yaml::Mapping(ElementsAre(
                Pair("inst_block_empty", Yaml::Mapping(IsEmpty())),
                Pair("exports", Yaml::Mapping(Each(Pair(_, inst_id)))),
-               Pair("import_refs", Yaml::Mapping(IsEmpty())),
+               Pair("imports", Yaml::Mapping(IsEmpty())),
                Pair("global_init", Yaml::Mapping(IsEmpty())),
                Pair("inst_block4", Yaml::Mapping(Each(Pair(_, inst_id)))),
                Pair("inst_block5", Yaml::Mapping(Each(Pair(_, inst_id)))),