Change BlockValueStore to take ElementT as a parameter (#5758)

Also modify CopyOnWriteBlock to just pull block type information from
the return type of the function it receives, rather than taking some as
a parameter.

I chose the `RefType`/`ConstRefType` names based on other similar
`ValueStore` uses which I think are equivalent.

toolchain/sem_ir/block_value_store.h

@@ -22,26 +22,24 @@ namespace Carbon::SemIR {
 //
 // BlockValueStore is used as-is, but there are also children that expose the
 // protected members for type-specific functionality.
-//
-// On IdT, this requires:
-//   - IdT::ElementType to represent the underlying type in the block.
-//   - IdT::ValueType to be llvm::MutableArrayRef<IdT::ElementType> for
-//     compatibility with ValueStore.
-template <typename IdT>
-class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
+template <typename IdT, typename ElementT>
+class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT, ElementT>> {
  public:
-  using ElementType = IdT::ElementType;
+  using IdType = IdT;
+  using ElementType = ElementT;
+  using RefType = llvm::MutableArrayRef<ElementT>;
+  using ConstRefType = llvm::ArrayRef<ElementT>;
 
   explicit BlockValueStore(llvm::BumpPtrAllocator& allocator)
       : allocator_(&allocator) {
-    auto empty = llvm::MutableArrayRef<ElementType>();
+    auto empty = RefType();
     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 {
+  auto Add(ConstRefType content) -> IdT {
     if (content.empty()) {
       return IdT::Empty;
     }
@@ -49,16 +47,12 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
   }
 
   // Returns the requested block.
-  auto Get(IdT id) const -> llvm::ArrayRef<ElementType> {
-    return values_.Get(id);
-  }
+  auto Get(IdT id) const -> ConstRefType { return values_.Get(id); }
 
   // Returns a mutable view of the requested block. This operation should be
   // avoided where possible; we generally want blocks to be immutable once
   // created.
-  auto GetMutable(IdT id) -> llvm::MutableArrayRef<ElementType> {
-    return values_.Get(id);
-  }
+  auto GetMutable(IdT id) -> RefType { return values_.Get(id); }
 
   // Returns a new block formed by applying `transform(elem_id)` to each element
   // in the specified block.
@@ -70,7 +64,7 @@ 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 {
+  auto AddCanonical(ConstRefType content) -> IdT {
     if (content.empty()) {
       return IdT::Empty;
     }
@@ -115,44 +109,40 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
 
  protected:
   // Allocates a copy of the given data using our slab allocator.
-  auto AllocateCopy(llvm::ArrayRef<ElementType> data)
-      -> llvm::MutableArrayRef<ElementType> {
+  auto AllocateCopy(ConstRefType data) -> RefType {
     auto result = AllocateUninitialized(data.size());
     std::uninitialized_copy(data.begin(), data.end(), result.begin());
     return result;
   }
 
   // Allocates an uninitialized array using our slab allocator.
-  auto AllocateUninitialized(size_t size)
-      -> llvm::MutableArrayRef<ElementType> {
+  auto AllocateUninitialized(size_t size) -> RefType {
     // We're not going to run a destructor, so ensure that's OK.
     static_assert(std::is_trivially_destructible_v<ElementType>);
 
     auto storage = static_cast<ElementType*>(
         allocator_->Allocate(size * sizeof(ElementType), alignof(ElementType)));
-    return llvm::MutableArrayRef<ElementType>(storage, size);
+    return RefType(storage, size);
   }
 
   // Allow children to have more complex value handling.
-  auto values() -> ValueStore<IdT, typename IdT::ValueType>& { return values_; }
+  auto values() -> ValueStore<IdT, RefType>& { return values_; }
 
  private:
   class KeyContext;
 
   llvm::BumpPtrAllocator* allocator_;
-  ValueStore<IdT, typename IdT::ValueType> values_;
+  ValueStore<IdT, RefType> values_;
   Set<IdT, /*SmallSize=*/0, KeyContext> canonical_blocks_;
 };
 
-template <typename IdT>
-class BlockValueStore<IdT>::KeyContext
+template <typename IdT, typename ElementT>
+class BlockValueStore<IdT, ElementT>::KeyContext
     : public TranslatingKeyContext<KeyContext> {
  public:
   explicit KeyContext(const BlockValueStore* store) : store_(store) {}
 
-  auto TranslateKey(IdT id) const -> llvm::ArrayRef<ElementType> {
-    return store_->Get(id);
-  }
+  auto TranslateKey(IdT id) const -> ConstRefType { return store_->Get(id); }
 
  private:
   const BlockValueStore* store_;

toolchain/sem_ir/copy_on_write_block.h

@@ -5,6 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_COPY_ON_WRITE_BLOCK_H_
 #define CARBON_TOOLCHAIN_SEM_IR_COPY_ON_WRITE_BLOCK_H_
 
+#include "llvm/ADT/STLExtras.h"
 #include "toolchain/sem_ir/file.h"
 #include "toolchain/sem_ir/ids.h"
 
@@ -19,39 +20,42 @@ namespace Carbon::SemIR {
 //
 // This is intended to avoid an unnecessary block allocation in the case where
 // the new block ends up being exactly the same as the original block.
-template <typename BlockIdType, auto (File::*ValueStore)()>
+template <auto (File::*ValueStore)()>
 class CopyOnWriteBlock {
  public:
+  using BlockType = std::remove_cvref_t<
+      typename llvm::function_traits<decltype(ValueStore)>::result_t>;
+
   struct UninitializedBlock {
     size_t size;
   };
 
   // Constructs the block. `source_id` is used as the initial value of the
   // block. `file` must not be null.
-  explicit CopyOnWriteBlock(File* file, BlockIdType source_id)
+  explicit CopyOnWriteBlock(File* file, BlockType::IdType source_id)
       : file_(file), source_id_(source_id) {}
 
   // Constructs the block, treating the original block as an uninitialized block
   // with `size` elements. `file` must not be null.
   explicit CopyOnWriteBlock(File* file, UninitializedBlock uninit)
       : file_(file),
-        source_id_(BlockIdType::None),
+        source_id_(BlockType::IdType::None),
         id_((file_->*ValueStore)().AddUninitialized(uninit.size)) {}
 
   // Gets a block ID containing the resulting elements. Note that further
   // modifications may or may not allocate a new ID, so this should only be
   // called once all modifications have been performed.
-  auto id() const -> BlockIdType { return id_; }
+  auto id() const -> BlockType::IdType { return id_; }
 
   // Gets a canonical block ID containing the resulting elements. This assumes
   // the original block ID, if specified, was also canonical.
-  auto GetCanonical() const -> BlockIdType {
+  auto GetCanonical() const -> BlockType::IdType {
     return id_ == source_id_ ? id_ : (file_->*ValueStore)().MakeCanonical(id_);
   }
 
   // Sets the element at index `i` within the block. Lazily allocates a new
   // block when the value changes for the first time.
-  auto Set(int i, typename BlockIdType::ElementType value) -> void {
+  auto Set(int i, BlockType::ElementType value) -> void {
     if (source_id_.has_value() && (file_->*ValueStore)().Get(id_)[i] == value) {
       return;
     }
@@ -63,13 +67,13 @@ class CopyOnWriteBlock {
 
  private:
   File* file_;
-  BlockIdType source_id_;
-  BlockIdType id_ = source_id_;
+  BlockType::IdType source_id_;
+  BlockType::IdType id_ = source_id_;
 };
 
-using CopyOnWriteInstBlock = CopyOnWriteBlock<InstBlockId, &File::inst_blocks>;
+using CopyOnWriteInstBlock = CopyOnWriteBlock<&File::inst_blocks>;
 using CopyOnWriteStructTypeFieldsBlock =
-    CopyOnWriteBlock<StructTypeFieldsId, &File::struct_type_fields>;
+    CopyOnWriteBlock<&File::struct_type_fields>;
 
 }  // namespace Carbon::SemIR
 

toolchain/sem_ir/ids.h

@@ -20,7 +20,6 @@ namespace Carbon::SemIR {
 class File;
 struct FacetTypeInfo;
 struct SpecificInterface;
-struct StructTypeField;
 
 // The ID of an `Inst`.
 struct InstId : public IdBase<InstId> {
@@ -579,12 +578,9 @@ struct NameScopeId : public IdBase<NameScopeId> {
 
 constexpr NameScopeId NameScopeId::Package = NameScopeId(0);
 
-// The ID of an instruction block.
+// The ID of an `InstId` block.
 struct InstBlockId : public IdBase<InstBlockId> {
   static constexpr llvm::StringLiteral Label = "inst_block";
-  // Types for BlockValueStore<InstBlockId>.
-  using ElementType = InstId;
-  using ValueType = llvm::MutableArrayRef<ElementType>;
 
   // The canonical empty block, reused to avoid allocating empty vectors. Always
   // the 0-index block.
@@ -714,12 +710,9 @@ struct ExprRegionId : public IdBase<ExprRegionId> {
   using IdBase::IdBase;
 };
 
-// The ID of a struct type field block.
+// The ID of a `StructTypeField` block.
 struct StructTypeFieldsId : public IdBase<StructTypeFieldsId> {
   static constexpr llvm::StringLiteral Label = "struct_type_fields";
-  // Types for BlockValueStore<StructTypeFieldsId>.
-  using ElementType = StructTypeField;
-  using ValueType = llvm::MutableArrayRef<StructTypeField>;
 
   // The canonical empty block, reused to avoid allocating empty vectors. Always
   // the 0-index block.

toolchain/sem_ir/inst.h

@@ -619,9 +619,9 @@ class InstStore {
 };
 
 // Adapts BlockValueStore for instruction blocks.
-class InstBlockStore : public BlockValueStore<InstBlockId> {
+class InstBlockStore : public BlockValueStore<InstBlockId, InstId> {
  public:
-  using BaseType = BlockValueStore<InstBlockId>;
+  using BaseType = BlockValueStore<InstBlockId, InstId>;
 
   explicit InstBlockStore(llvm::BumpPtrAllocator& allocator)
       : BaseType(allocator) {
@@ -642,7 +642,7 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {
   // Reserves and returns a block ID. The contents of the block should be
   // specified by calling ReplacePlaceholder.
   auto AddPlaceholder() -> InstBlockId {
-    return values().Add(llvm::MutableArrayRef<ElementType>());
+    return values().Add(llvm::MutableArrayRef<InstId>());
   }
 
   // Sets the contents of a placeholder block to the given content.

toolchain/sem_ir/struct_type_field.h

@@ -24,7 +24,8 @@ struct StructTypeField : Printable<StructTypeField> {
   TypeInstId type_inst_id;
 };
 
-using StructTypeFieldsStore = BlockValueStore<StructTypeFieldsId>;
+using StructTypeFieldsStore =
+    BlockValueStore<StructTypeFieldsId, StructTypeField>;
 
 // See common/hashing.h. Supports canonicalization of fields.
 inline auto CarbonHashValue(const StructTypeField& value, uint64_t seed)