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

Make empty ids for all block types (#4502)

Adding empty IDs to all block types. This is primarily for
TypeBlockId::Empty, which allows me to disambiguate empty tuple types,
which I think might yield a SemIR readability improvement (PR
forthcoming). I'm splitting PRs so that the limited test impact here is
clear.
Jon Ross-Perkins 1 год назад
Родитель
Сommit
cab7818df8

+ 1 - 1
toolchain/check/pattern_match.cpp

@@ -126,7 +126,7 @@ auto MatchContext::DoWork(Context& context) -> SemIR::InstBlockId {
   while (!stack_.empty()) {
     EmitPatternMatch(context, stack_.pop_back_val());
   }
-  auto block_id = context.inst_blocks().AddOrEmpty(results_);
+  auto block_id = context.inst_blocks().Add(results_);
   results_.clear();
   return block_id;
 }

+ 4 - 2
toolchain/check/testdata/basics/builtin_insts.carbon

@@ -23,12 +23,14 @@
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   specifics:       {}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}
 // CHECK:STDOUT:     'type(instNamespaceType)': {kind: copy, type: type(instNamespaceType)}
-// CHECK:STDOUT:   type_blocks:     {}
+// CHECK:STDOUT:   type_blocks:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   insts:
 // CHECK:STDOUT:     instTypeType:    {kind: BuiltinInst, arg0: TypeType, type: typeTypeType}
 // CHECK:STDOUT:     instError:       {kind: BuiltinInst, arg0: Error, type: typeError}

+ 4 - 2
toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon

@@ -40,7 +40,8 @@ fn B() {
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   specifics:       {}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}
@@ -116,7 +117,8 @@ fn B() {
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   specifics:       {}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}

+ 4 - 2
toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon

@@ -40,7 +40,8 @@ fn B() {
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   specifics:       {}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}
@@ -95,7 +96,8 @@ fn B() {
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   specifics:       {}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}

+ 2 - 1
toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon

@@ -31,7 +31,8 @@ fn Foo(n: ()) -> ((), ()) {
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   specifics:       {}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}

+ 2 - 1
toolchain/check/testdata/basics/no_prelude/raw_ir.carbon

@@ -34,7 +34,8 @@ fn Foo[T:! type](n: T) -> (T, ()) {
 // CHECK:STDOUT:     generic0:        {decl: inst+24, bindings: block11}
 // CHECK:STDOUT:   specifics:
 // CHECK:STDOUT:     specific0:       {generic: generic0, args: block13}
-// CHECK:STDOUT:   struct_type_fields: {}
+// CHECK:STDOUT:   struct_type_fields:
+// CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     typeTypeType:    {kind: copy, type: typeTypeType}
 // CHECK:STDOUT:     typeError:       {kind: copy, type: typeError}

+ 12 - 1
toolchain/sem_ir/block_value_store.h

@@ -30,10 +30,18 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
   using ElementType = IdT::ElementType;
 
   explicit BlockValueStore(llvm::BumpPtrAllocator& allocator)
-      : allocator_(&allocator) {}
+      : allocator_(&allocator) {
+    auto empty = llvm::MutableArrayRef<ElementType>();
+    auto empty_val = canonical_blocks_.Insert(
+        empty, [&] { return values_.Add(empty); }, KeyContext(this));
+    CARBON_CHECK(empty_val.key() == IdT::Empty);
+  }
 
   // Adds a block with the given content, returning an ID to reference it.
   auto Add(llvm::ArrayRef<ElementType> content) -> IdT {
+    if (content.empty()) {
+      return IdT::Empty;
+    }
     return values_.Add(AllocateCopy(content));
   }
 
@@ -50,6 +58,9 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
   // Adds a block or finds an existing canonical block with the given content,
   // and returns an ID to reference it.
   auto AddCanonical(llvm::ArrayRef<ElementType> content) -> IdT {
+    if (content.empty()) {
+      return IdT::Empty;
+    }
     auto result = canonical_blocks_.Insert(
         content, [&] { return Add(content); }, KeyContext(this));
     return result.key();

+ 10 - 0
toolchain/sem_ir/ids.h

@@ -732,6 +732,10 @@ struct StructTypeFieldsId : public IdBase,
   // An explicitly invalid ID.
   static const StructTypeFieldsId Invalid;
 
+  // The canonical empty block, reused to avoid allocating empty vectors. Always
+  // the 0-index block.
+  static const StructTypeFieldsId Empty;
+
   using IdBase::IdBase;
   auto Print(llvm::raw_ostream& out) const -> void {
     out << "type_block";
@@ -741,6 +745,7 @@ struct StructTypeFieldsId : public IdBase,
 
 constexpr StructTypeFieldsId StructTypeFieldsId::Invalid =
     StructTypeFieldsId(InvalidIndex);
+constexpr StructTypeFieldsId StructTypeFieldsId::Empty = StructTypeFieldsId(0);
 
 // The ID of a type.
 struct TypeId : public IdBase, public Printable<TypeId> {
@@ -804,6 +809,10 @@ struct TypeBlockId : public IdBase, public Printable<TypeBlockId> {
   // An explicitly invalid ID.
   static const TypeBlockId Invalid;
 
+  // The canonical empty block, reused to avoid allocating empty vectors. Always
+  // the 0-index block.
+  static const TypeBlockId Empty;
+
   using IdBase::IdBase;
   auto Print(llvm::raw_ostream& out) const -> void {
     out << "type_block";
@@ -812,6 +821,7 @@ struct TypeBlockId : public IdBase, public Printable<TypeBlockId> {
 };
 
 constexpr TypeBlockId TypeBlockId::Invalid = TypeBlockId(InvalidIndex);
+constexpr TypeBlockId TypeBlockId::Empty = TypeBlockId(0);
 
 // An index for element access, for structs, tuples, and classes.
 struct ElementIndex : public IndexBase, public Printable<ElementIndex> {

+ 0 - 8
toolchain/sem_ir/inst.h

@@ -445,8 +445,6 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {
 
   explicit InstBlockStore(llvm::BumpPtrAllocator& allocator)
       : BaseType(allocator) {
-    auto empty_id = AddCanonical({});
-    CARBON_CHECK(empty_id == InstBlockId::Empty);
     auto exports_id = AddDefaultValue();
     CARBON_CHECK(exports_id == InstBlockId::Exports);
     auto import_refs_id = AddDefaultValue();
@@ -455,12 +453,6 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {
     CARBON_CHECK(global_init_id == InstBlockId::GlobalInit);
   }
 
-  // Adds a block with the given content, returning an ID to reference it.
-  // Returns Empty rather than creating a unique ID if the block is empty.
-  auto AddOrEmpty(llvm::ArrayRef<ElementType> content) -> InstBlockId {
-    return content.empty() ? InstBlockId::Empty : Add(content);
-  }
-
   auto Set(InstBlockId block_id, llvm::ArrayRef<InstId> content) -> void {
     CARBON_CHECK(block_id != InstBlockId::Unreachable);
     BlockValueStore<InstBlockId>::SetContent(block_id, content);

+ 1 - 1
toolchain/sem_ir/yaml_test.cpp

@@ -63,7 +63,7 @@ TEST(SemIRTest, YAML) {
       Pair("classes", Yaml::Mapping(SizeIs(0))),
       Pair("generics", Yaml::Mapping(SizeIs(0))),
       Pair("specifics", Yaml::Mapping(SizeIs(0))),
-      Pair("struct_type_fields", Yaml::Mapping(SizeIs(0))),
+      Pair("struct_type_fields", Yaml::Mapping(SizeIs(1))),
       Pair("types", Yaml::Mapping(Each(type_builtin))),
       Pair("type_blocks", Yaml::Mapping(SizeIs(Ge(1)))),
       Pair("insts",