Преглед на файлове

Make ValueStore require a ValueT parameter (#5757)

This is reducing ValueStore inference of types from `using`, and removes
`using ValueType = ...` from affected id types.

I'm adding a number of `using FooStore = ValueStore<FooId, Foo>` because
I think it's a little repetitive otherwise; often 4 cases where I'm
doing this: getter, const getter, member, and getter on `Context`. Note
we also have a number of `-> decltype(auto)` that were added I think
mainly to avoid repeating the type, but I'm not sure whether there'll be
agreement on replacing those and so am not changing them here.

I'm placing these aliases with the value type in general, because I
think it's probably easier to view that way. An alternative would be to
put all the types on `File`, but:

- That would be inconsistent with things like `InstStore`, which are
very `ValueStore`-adjacent and put with their value type.
- `File` would have a _lot_ of using's, and the accessors are already
noisy -- I think it would just make the file harder to skim.

Note this is the heart of what I'd brought up [on
Discord](https://discord.com/channels/655572317891461132/655578254970716160/1388199282250613019).
This PR still leaves CanonicalValueStore and BlockValueStore as things
to also add parameters to, but I thought it best to try breaking the set
of changes apart by type. Both of those rely on ValueStore, so
ValueStore needs to change first.
Jon Ross-Perkins преди 10 месеца
родител
ревизия
a65f4b89e2

+ 7 - 7
toolchain/base/canonical_value_store.h

@@ -48,7 +48,8 @@ class CanonicalValueStore {
     return values_.OutputYaml();
   }
 
-  auto values() const [[clang::lifetimebound]] -> ValueStore<IdT>::Range {
+  auto values() const [[clang::lifetimebound]]
+  -> ValueStore<IdT, ValueType>::Range {
     return values_.values();
   }
   auto size() const -> size_t { return values_.size(); }
@@ -64,7 +65,7 @@ class CanonicalValueStore {
  private:
   class KeyContext;
 
-  ValueStore<IdT> values_;
+  ValueStore<IdT, ValueType> values_;
   Set<IdT, /*SmallSize=*/0, KeyContext> set_;
 };
 
@@ -72,16 +73,15 @@ template <typename IdT>
 class CanonicalValueStore<IdT>::KeyContext
     : public TranslatingKeyContext<KeyContext> {
  public:
-  explicit KeyContext(const ValueStore<IdT>* values) : values_(values) {}
+  explicit KeyContext(const ValueStore<IdT, ValueType>* values)
+      : values_(values) {}
 
   // Note that it is safe to return a `const` reference here as the underlying
   // object's lifetime is provided by the `ValueStore`.
-  auto TranslateKey(IdT id) const -> ValueStore<IdT>::ConstRefType {
-    return values_->Get(id);
-  }
+  auto TranslateKey(IdT id) const -> ConstRefType { return values_->Get(id); }
 
  private:
-  const ValueStore<IdT>* values_;
+  const ValueStore<IdT, ValueType>* values_;
 };
 
 template <typename IdT>

+ 1 - 1
toolchain/base/shared_value_stores.h

@@ -20,7 +20,7 @@ class SharedValueStores : public Yaml::Printable<SharedValueStores> {
  public:
   // Provide types that can be used by APIs to forward access to these stores.
   using IntStore = IntStore;
-  using RealStore = ValueStore<RealId>;
+  using RealStore = ValueStore<RealId, Real>;
   using FloatStore = CanonicalValueStore<FloatId>;
   using IdentifierStore = CanonicalValueStore<IdentifierId>;
   using StringLiteralStore = CanonicalValueStore<StringLiteralValueId>;

+ 0 - 1
toolchain/base/value_ids.h

@@ -54,7 +54,6 @@ constexpr FloatId FloatId::None(FloatId::NoneIndex);
 // Corresponds to a Real value.
 struct RealId : public IdBase<RealId> {
   static constexpr llvm::StringLiteral Label = "real";
-  using ValueType = Real;
   static const RealId None;
   using IdBase::IdBase;
 };

+ 15 - 19
toolchain/base/value_store.h

@@ -33,20 +33,16 @@ class ValueStoreNotPrintable {};
 
 // A simple wrapper for accumulating values, providing IDs to later retrieve the
 // value. This does not do deduplication.
-//
-// IdT::ValueType must represent the type being indexed.
-template <typename IdT>
-  requires(requires { typename IdT::ValueType; })
+template <typename IdT, typename ValueT>
 class ValueStore
-    : public std::conditional<
-          std::is_base_of_v<Printable<typename IdT::ValueType>,
-                            typename IdT::ValueType>,
-          Yaml::Printable<ValueStore<IdT>>, Internal::ValueStoreNotPrintable> {
+    : public std::conditional<std::is_base_of_v<Printable<ValueT>, ValueT>,
+                              Yaml::Printable<ValueStore<IdT, ValueT>>,
+                              Internal::ValueStoreNotPrintable> {
  public:
   using IdType = IdT;
-  using ValueType = ValueStoreTypes<IdT>::ValueType;
-  using RefType = ValueStoreTypes<IdT>::RefType;
-  using ConstRefType = ValueStoreTypes<IdT>::ConstRefType;
+  using ValueType = ValueStoreTypes<IdT, ValueT>::ValueType;
+  using RefType = ValueStoreTypes<IdT, ValueT>::RefType;
+  using ConstRefType = ValueStoreTypes<IdT, ValueT>::ConstRefType;
 
   // A range over references to the values in a ValueStore, returned from
   // `ValueStore::values()`. Hides the complex type name of the iterator
@@ -68,7 +64,7 @@ class ValueStore
       // can use llvm::seq to walk all indices in the store.
       return llvm::map_range(
           llvm::seq(store.size_),
-          [&](int32_t i) -> ConstRefType { return store.Get(IdT(i)); });
+          [&](int32_t i) -> ConstRefType { return store.Get(IdType(i)); });
     }
 
     using FlattenedRangeType =
@@ -79,13 +75,13 @@ class ValueStore
   ValueStore() = default;
 
   // Stores the value and returns an ID to reference it.
-  auto Add(ValueType value) -> IdT {
+  auto Add(ValueType value) -> IdType {
     // This routine is especially hot and the check here relatively expensive
     // for the value provided, so only do this in non-optimized builds to make
     // tracking down issues easier.
     CARBON_DCHECK(size_ < std::numeric_limits<int32_t>::max(), "Id overflow");
 
-    IdT id(size_);
+    IdType id(size_);
     auto [chunk_index, pos] = IdToChunkIndices(id);
     ++size_;
 
@@ -101,7 +97,7 @@ class ValueStore
   }
 
   // Returns a mutable value for an ID.
-  auto Get(IdT id) -> RefType {
+  auto Get(IdType id) -> RefType {
     CARBON_DCHECK(id.index >= 0, "{0}", id);
     CARBON_DCHECK(id.index < size_, "{0}", id);
     auto [chunk_index, pos] = IdToChunkIndices(id);
@@ -109,7 +105,7 @@ class ValueStore
   }
 
   // Returns the value for an ID.
-  auto Get(IdT id) const -> ConstRefType {
+  auto Get(IdType id) const -> ConstRefType {
     CARBON_DCHECK(id.index >= 0, "{0}", id);
     CARBON_DCHECK(id.index < size_, "{0}", id);
     auto [chunk_index, pos] = IdToChunkIndices(id);
@@ -163,8 +159,8 @@ class ValueStore
     // For `it->val`, writing `const std::pair` is required; otherwise
     // `mapped_iterator` incorrectly infers the pointer type for `PointerProxy`.
     // NOLINTNEXTLINE(readability-const-return-type)
-    auto index_to_id = [&](int32_t i) -> const std::pair<IdT, ConstRefType> {
-      return std::pair<IdT, ConstRefType>(IdT(i), Get(IdT(i)));
+    auto index_to_id = [&](int32_t i) -> const std::pair<IdType, ConstRefType> {
+      return std::pair<IdType, ConstRefType>(IdType(i), Get(IdType(i)));
     };
     // Because indices into `ValueStore` are all sequential values from 0, we
     // can use llvm::seq to walk all indices in the store.
@@ -277,7 +273,7 @@ class ValueStore
 
   // Converts an id into an index into the set of chunks, and an offset into
   // that specific chunk. Looks for index overflow in non-optimized builds.
-  static auto IdToChunkIndices(IdT id) -> std::pair<int32_t, int32_t> {
+  static auto IdToChunkIndices(IdType id) -> std::pair<int32_t, int32_t> {
     constexpr auto LowBits = Chunk::IndexBits();
 
     // Verify there are no unused bits when indexing up to the `Capacity`. This

+ 1 - 1
toolchain/base/value_store_test.cpp

@@ -25,7 +25,7 @@ TEST(ValueStore, Real) {
              .exponent = llvm::APInt(64, 22),
              .is_decimal = false};
 
-  ValueStore<RealId> reals;
+  ValueStore<RealId, Real> reals;
   RealId id1 = reals.Add(real1);
   RealId id2 = reals.Add(real2);
 

+ 1 - 0
toolchain/check/BUILD

@@ -271,6 +271,7 @@ cc_library(
         "//common:vlog",
         "//toolchain/base:canonical_value_store",
         "//toolchain/base:index_base",
+        "//toolchain/base:shared_value_stores",
         "//toolchain/base:value_ids",
         "//toolchain/base:value_store",
         "//toolchain/sem_ir:file",

+ 7 - 13
toolchain/check/context.h

@@ -239,15 +239,11 @@ class Context {
   auto entity_names() -> SemIR::EntityNameStore& {
     return sem_ir().entity_names();
   }
-  auto functions() -> ValueStore<SemIR::FunctionId>& {
-    return sem_ir().functions();
-  }
-  auto classes() -> ValueStore<SemIR::ClassId>& { return sem_ir().classes(); }
-  auto vtables() -> ValueStore<SemIR::VtableId>& { return sem_ir().vtables(); }
-  auto interfaces() -> ValueStore<SemIR::InterfaceId>& {
-    return sem_ir().interfaces();
-  }
-  auto associated_constants() -> ValueStore<SemIR::AssociatedConstantId>& {
+  auto functions() -> SemIR::FunctionStore& { return sem_ir().functions(); }
+  auto classes() -> SemIR::ClassStore& { return sem_ir().classes(); }
+  auto vtables() -> SemIR::VtableStore& { return sem_ir().vtables(); }
+  auto interfaces() -> SemIR::InterfaceStore& { return sem_ir().interfaces(); }
+  auto associated_constants() -> SemIR::AssociatedConstantStore& {
     return sem_ir().associated_constants();
   }
   auto facet_types() -> CanonicalValueStore<SemIR::FacetTypeId>& {
@@ -263,10 +259,8 @@ class Context {
   }
   auto generics() -> SemIR::GenericStore& { return sem_ir().generics(); }
   auto specifics() -> SemIR::SpecificStore& { return sem_ir().specifics(); }
-  auto import_irs() -> ValueStore<SemIR::ImportIRId>& {
-    return sem_ir().import_irs();
-  }
-  auto import_ir_insts() -> ValueStore<SemIR::ImportIRInstId>& {
+  auto import_irs() -> SemIR::ImportIRStore& { return sem_ir().import_irs(); }
+  auto import_ir_insts() -> SemIR::ImportIRInstStore& {
     return sem_ir().import_ir_insts();
   }
   auto ast_context() -> clang::ASTContext& {

+ 3 - 5
toolchain/check/eval.cpp

@@ -168,13 +168,11 @@ class EvalContext {
   auto entity_names() -> SemIR::EntityNameStore& {
     return sem_ir().entity_names();
   }
-  auto functions() -> const ValueStore<SemIR::FunctionId>& {
+  auto functions() -> const SemIR::FunctionStore& {
     return sem_ir().functions();
   }
-  auto classes() -> const ValueStore<SemIR::ClassId>& {
-    return sem_ir().classes();
-  }
-  auto interfaces() -> const ValueStore<SemIR::InterfaceId>& {
+  auto classes() -> const SemIR::ClassStore& { return sem_ir().classes(); }
+  auto interfaces() -> const SemIR::InterfaceStore& {
     return sem_ir().interfaces();
   }
   auto specific_interfaces()

+ 2 - 1
toolchain/check/lexical_lookup.h

@@ -7,6 +7,7 @@
 #define CARBON_TOOLCHAIN_CHECK_LEXICAL_LOOKUP_H_
 
 #include "toolchain/base/canonical_value_store.h"
+#include "toolchain/base/shared_value_stores.h"
 #include "toolchain/base/value_ids.h"
 #include "toolchain/check/scope_index.h"
 #include "toolchain/sem_ir/ids.h"
@@ -39,7 +40,7 @@ class LexicalLookup {
     SemIR::InstId inst_id;
   };
 
-  explicit LexicalLookup(const CanonicalValueStore<IdentifierId>& identifiers)
+  explicit LexicalLookup(const SharedValueStores::IdentifierStore& identifiers)
       : lookup_(identifiers.size() + SemIR::NameId::NonIndexValueCount) {}
 
   // Returns the lexical lookup results for a name.

+ 1 - 1
toolchain/docs/adding_features.md

@@ -274,7 +274,7 @@ If the resulting SemIR needs a new instruction:
 
         // 0-2 id fields, with types from sem_ir/ids.h or
         // sem_ir/builtin_kind.h. For example, fields would look like:
-        StringId name_id;
+        NameId name_id;
         InstId value_id;
     };
     ```

+ 9 - 12
toolchain/docs/idioms.md

@@ -149,21 +149,18 @@ The indices typically use `IdBase`.
     constant reference).
 -   Other vector-like functionality, including `size` or `Reserve`
 
-ValueStores should be named after the type they contain. The index type used on
-the value store should have a `using ValueType...` which indicates the stored
-type. When taking a return of one of these functions, it's common to use `auto`
-and rely on the name of the storage type to imply the returned type.
+Each `ValueStore` instance should be named after its value type. When taking a
+return of one of these functions, it's common to use `auto` and rely on the name
+of the storage type to imply the returned type.
 
 Some name mirroring examples are:
 
--   `ints` is a `ValueStore<IntId>`, which has an index type of `IntId` and a
-    value type of `llvm::APInt`.
-
--   `functions` is a `ValueStore<SemIR::FunctionId>`, which has an index type of
-    `SemIR::FunctionId` and a value type of `SemIR::` `Function`.
-
--   `strings` is a `ValueStore<StringId>`, which has an index type of
-    `StringId`, but for copy-related reasons, uses `llvm::StringRef` for values.
+-   `ints` is a `ValueStore<IntId, llvm::APInt>`; values are integers.
+-   `functions` is a `ValueStore<SemIR::FunctionId, SemIR::Function>`; values
+    are functions.
+-   `string_literals` is a `ValueStore<StringLiteralValueId, llvm::StringRef>`;
+    values are string literals.
+    -   A reference is used in order to avoid string copies.
 
 There are also a number of wrappers around `ValueStore` that provide some
 additional functionality and which are named with the `Store` suffix, such as

+ 1 - 1
toolchain/lex/lex.cpp

@@ -1552,7 +1552,7 @@ class Lexer::ErrorRecoveryBuffer {
 
   // Merge the recovery tokens into the token list of the tokenized buffer.
   auto Apply() -> void {
-    ValueStore<TokenIndex> old_tokens =
+    ValueStore<TokenIndex, TokenInfo> old_tokens =
         std::exchange(buffer_->token_infos_, {});
     int new_size = old_tokens.size() + new_tokens_.size();
     buffer_->token_infos_.Reserve(new_size);

+ 0 - 2
toolchain/lex/token_index.h

@@ -26,8 +26,6 @@ class TokenInfo;
 //
 // All other APIs to query a `TokenIndex` are on the `TokenizedBuffer`.
 struct TokenIndex : public IndexBase<TokenIndex> {
-  using ValueType = TokenInfo;
-
   // The number of bits which must be allotted for `TokenIndex`.
   static constexpr int Bits = 23;
   // The maximum number of tokens that can be stored, including the FileStart

+ 5 - 9
toolchain/lex/tokenized_buffer.h

@@ -39,7 +39,7 @@ struct LineInfo {
   int32_t indent;
 };
 
-// A lightweight handle to a lexed line in a `TokenizedBuffer`.
+// A lightweight handle to a lexed `LineInfo` in a `TokenizedBuffer`.
 //
 // `LineIndex` objects are designed to be passed by value, not reference or
 // pointer. They are also designed to be small and efficient to store in data
@@ -51,8 +51,6 @@ struct LineInfo {
 //
 // All other APIs to query a `LineIndex` are on the `TokenizedBuffer`.
 struct LineIndex : public IndexBase<LineIndex> {
-  using ValueType = LineInfo;
-
   static constexpr llvm::StringLiteral Label = "line";
   static const LineIndex None;
   using IndexBase::IndexBase;
@@ -74,10 +72,8 @@ struct CommentData {
   int32_t length;
 };
 
-// Indices for comments within the buffer.
+// Indices for `CommentData` within the buffer.
 struct CommentIndex : public IndexBase<CommentIndex> {
-  using ValueType = CommentData;
-
   static constexpr llvm::StringLiteral Label = "comment";
   static const CommentIndex None;
   using IndexBase::IndexBase;
@@ -327,12 +323,12 @@ class TokenizedBuffer : public Printable<TokenizedBuffer> {
   SharedValueStores* value_stores_;
   SourceBuffer* source_;
 
-  ValueStore<TokenIndex> token_infos_;
+  ValueStore<TokenIndex, TokenInfo> token_infos_;
 
-  ValueStore<LineIndex> line_infos_;
+  ValueStore<LineIndex, LineInfo> line_infos_;
 
   // Comments in the file.
-  ValueStore<CommentIndex> comments_;
+  ValueStore<CommentIndex, CommentData> comments_;
 
   // A range of tokens marked by `//@dump-semir-[begin|end]`.
   //

+ 3 - 5
toolchain/parse/tree.h

@@ -23,11 +23,9 @@ namespace Carbon::Parse {
 
 struct DeferredDefinition;
 
-// The index of a deferred function definition within the parse tree's deferred
-// definition store.
+// The index of a `DeferredDefinition` within the parse tree.
 struct DeferredDefinitionIndex : public IndexBase<DeferredDefinitionIndex> {
   static constexpr llvm::StringLiteral Label = "deferred_def";
-  using ValueType = DeferredDefinition;
 
   using IndexBase::IndexBase;
 };
@@ -167,7 +165,7 @@ class Tree : public Printable<Tree> {
   }
   auto imports() const -> llvm::ArrayRef<PackagingNames> { return imports_; }
   auto deferred_definitions() const
-      -> const ValueStore<DeferredDefinitionIndex>& {
+      -> const ValueStore<DeferredDefinitionIndex, DeferredDefinition>& {
     return deferred_definitions_;
   }
 
@@ -263,7 +261,7 @@ class Tree : public Printable<Tree> {
 
   std::optional<PackagingDecl> packaging_decl_;
   llvm::SmallVector<PackagingNames> imports_;
-  ValueStore<DeferredDefinitionIndex> deferred_definitions_;
+  ValueStore<DeferredDefinitionIndex, DeferredDefinition> deferred_definitions_;
 };
 
 // A random-access iterator to the depth-first postorder sequence of parse nodes

+ 4 - 0
toolchain/sem_ir/associated_constant.h

@@ -6,6 +6,7 @@
 #define CARBON_TOOLCHAIN_SEM_IR_ASSOCIATED_CONSTANT_H_
 
 #include "common/ostream.h"
+#include "toolchain/base/value_store.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::SemIR {
@@ -48,6 +49,9 @@ struct AssociatedConstant : public Printable<AssociatedConstant> {
   InstId default_value_id = InstId::None;
 };
 
+using AssociatedConstantStore =
+    ValueStore<AssociatedConstantId, AssociatedConstant>;
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_ASSOCIATED_CONSTANT_H_

+ 2 - 2
toolchain/sem_ir/block_value_store.h

@@ -134,13 +134,13 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
   }
 
   // Allow children to have more complex value handling.
-  auto values() -> ValueStore<IdT>& { return values_; }
+  auto values() -> ValueStore<IdT, typename IdT::ValueType>& { return values_; }
 
  private:
   class KeyContext;
 
   llvm::BumpPtrAllocator* allocator_;
-  ValueStore<IdT> values_;
+  ValueStore<IdT, typename IdT::ValueType> values_;
   Set<IdT, /*SmallSize=*/0, KeyContext> canonical_blocks_;
 };
 

+ 3 - 0
toolchain/sem_ir/class.h

@@ -5,6 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_CLASS_H_
 #define CARBON_TOOLCHAIN_SEM_IR_CLASS_H_
 
+#include "toolchain/base/value_store.h"
 #include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
 
@@ -115,6 +116,8 @@ struct Class : public EntityWithParamsBase,
   auto GetObjectRepr(const File& file, SpecificId specific_id) const -> TypeId;
 };
 
+using ClassStore = ValueStore<ClassId, Class>;
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_CLASS_H_

+ 1 - 1
toolchain/sem_ir/entity_name.h

@@ -56,7 +56,7 @@ struct EntityName : public Printable<EntityName> {
 
 // Value store for EntityName. In addition to the regular ValueStore
 // functionality, this can provide optional canonical IDs for EntityNames.
-struct EntityNameStore : public ValueStore<EntityNameId> {
+struct EntityNameStore : public ValueStore<EntityNameId, EntityName> {
  public:
   // Adds an entity name for a symbolic binding.
   auto AddSymbolicBindingName(NameId name_id, NameScopeId parent_scope_id,

+ 1 - 0
toolchain/sem_ir/facet_type_info.h

@@ -7,6 +7,7 @@
 
 #include "common/hashing.h"
 #include "llvm/ADT/StringExtras.h"
+#include "toolchain/base/canonical_value_store.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::SemIR {

+ 34 - 40
toolchain/sem_ir/file.h

@@ -58,6 +58,8 @@ struct ExprRegion {
   InstId result_id;
 };
 
+using ExprRegionStore = ValueStore<ExprRegionId, ExprRegion>;
+
 // Provides semantic analysis on a Parse::Tree.
 class File : public Printable<File> {
  public:
@@ -147,18 +149,16 @@ class File : public Printable<File> {
 
   auto entity_names() -> EntityNameStore& { return entity_names_; }
   auto entity_names() const -> const EntityNameStore& { return entity_names_; }
-  auto functions() -> ValueStore<FunctionId>& { return functions_; }
-  auto functions() const -> const ValueStore<FunctionId>& { return functions_; }
-  auto classes() -> ValueStore<ClassId>& { return classes_; }
-  auto classes() const -> const ValueStore<ClassId>& { return classes_; }
-  auto interfaces() -> ValueStore<InterfaceId>& { return interfaces_; }
-  auto interfaces() const -> const ValueStore<InterfaceId>& {
-    return interfaces_;
-  }
-  auto associated_constants() -> ValueStore<AssociatedConstantId>& {
+  auto functions() -> FunctionStore& { return functions_; }
+  auto functions() const -> const FunctionStore& { return functions_; }
+  auto classes() -> ClassStore& { return classes_; }
+  auto classes() const -> const ClassStore& { return classes_; }
+  auto interfaces() -> InterfaceStore& { return interfaces_; }
+  auto interfaces() const -> const InterfaceStore& { return interfaces_; }
+  auto associated_constants() -> AssociatedConstantStore& {
     return associated_constants_;
   }
-  auto associated_constants() const -> const ValueStore<AssociatedConstantId>& {
+  auto associated_constants() const -> const AssociatedConstantStore& {
     return associated_constants_;
   }
   // TODO: Rename these to `facet_type_infos`.
@@ -187,20 +187,14 @@ class File : public Printable<File> {
   auto generics() const -> const GenericStore& { return generics_; }
   auto specifics() -> SpecificStore& { return specifics_; }
   auto specifics() const -> const SpecificStore& { return specifics_; }
-  auto import_irs() -> ValueStore<ImportIRId>& { return import_irs_; }
-  auto import_irs() const -> const ValueStore<ImportIRId>& {
-    return import_irs_;
-  }
-  auto import_ir_insts() -> ValueStore<ImportIRInstId>& {
-    return import_ir_insts_;
-  }
-  auto import_ir_insts() const -> const ValueStore<ImportIRInstId>& {
+  auto import_irs() -> ImportIRStore& { return import_irs_; }
+  auto import_irs() const -> const ImportIRStore& { return import_irs_; }
+  auto import_ir_insts() -> ImportIRInstStore& { return import_ir_insts_; }
+  auto import_ir_insts() const -> const ImportIRInstStore& {
     return import_ir_insts_;
   }
-  auto import_cpps() -> ValueStore<ImportCppId>& { return import_cpps_; }
-  auto import_cpps() const -> const ValueStore<ImportCppId>& {
-    return import_cpps_;
-  }
+  auto import_cpps() -> ImportCppStore& { return import_cpps_; }
+  auto import_cpps() const -> const ImportCppStore& { return import_cpps_; }
   auto cpp_ast() -> clang::ASTUnit* { return cpp_ast_; }
   auto cpp_ast() const -> const clang::ASTUnit* { return cpp_ast_; }
   // TODO: When the AST can be created before creating `File`, initialize the
@@ -228,8 +222,8 @@ class File : public Printable<File> {
   auto types() const -> const TypeStore& { return types_; }
   auto insts() -> InstStore& { return insts_; }
   auto insts() const -> const InstStore& { return insts_; }
-  auto vtables() -> ValueStore<VtableId>& { return vtables_; }
-  auto vtables() const -> const ValueStore<VtableId>& { return vtables_; }
+  auto vtables() -> VtableStore& { return vtables_; }
+  auto vtables() const -> const VtableStore& { return vtables_; }
   auto constant_values() -> ConstantValueStore& { return constant_values_; }
   auto constant_values() const -> const ConstantValueStore& {
     return constant_values_;
@@ -239,15 +233,15 @@ class File : public Printable<File> {
   auto constants() -> ConstantStore& { return constants_; }
   auto constants() const -> const ConstantStore& { return constants_; }
 
-  auto expr_regions() -> ValueStore<ExprRegionId>& { return expr_regions_; }
-  auto expr_regions() const -> const ValueStore<ExprRegionId>& {
-    return expr_regions_;
-  }
+  auto expr_regions() -> ExprRegionStore& { return expr_regions_; }
+  auto expr_regions() const -> const ExprRegionStore& { return expr_regions_; }
 
-  auto clang_source_locs() -> ValueStore<ClangSourceLocId>& {
+  using ClangSourceLocStore =
+      ValueStore<ClangSourceLocId, clang::SourceLocation>;
+  auto clang_source_locs() -> ClangSourceLocStore& {
     return clang_source_locs_;
   }
-  auto clang_source_locs() const -> const ValueStore<ClangSourceLocId>& {
+  auto clang_source_locs() const -> const ClangSourceLocStore& {
     return clang_source_locs_;
   }
 
@@ -297,16 +291,16 @@ class File : public Printable<File> {
   EntityNameStore entity_names_;
 
   // Storage for callable objects.
-  ValueStore<FunctionId> functions_;
+  FunctionStore functions_;
 
   // Storage for classes.
-  ValueStore<ClassId> classes_;
+  ClassStore classes_;
 
   // Storage for interfaces.
-  ValueStore<InterfaceId> interfaces_;
+  InterfaceStore interfaces_;
 
   // Storage for associated constants.
-  ValueStore<AssociatedConstantId> associated_constants_;
+  AssociatedConstantStore associated_constants_;
 
   // Storage for facet types.
   CanonicalValueStore<FacetTypeId> facet_types_;
@@ -328,14 +322,14 @@ class File : public Printable<File> {
   SpecificStore specifics_;
 
   // Related IRs. There are some fixed entries at the start; see ImportIRId.
-  ValueStore<ImportIRId> import_irs_;
+  ImportIRStore import_irs_;
 
   // Related IR instructions. These are created for LocIds for instructions
   // that are import-related.
-  ValueStore<ImportIRInstId> import_ir_insts_;
+  ImportIRInstStore import_ir_insts_;
 
   // List of Cpp imports.
-  ValueStore<ImportCppId> import_cpps_;
+  ImportCppStore import_cpps_;
 
   // The Clang AST to use when looking up `Cpp` names. Null if there are no
   // `Cpp` imports.
@@ -350,7 +344,7 @@ class File : public Printable<File> {
   // instructions.
   InstStore insts_ = InstStore(this);
 
-  ValueStore<VtableId> vtables_;
+  VtableStore vtables_;
 
   // Storage for name scopes.
   NameScopeStore name_scopes_ = NameScopeStore(this);
@@ -380,10 +374,10 @@ class File : public Printable<File> {
 
   // Single-entry/single-exit regions that are referenced as units, e.g. because
   // they represent expressions.
-  ValueStore<ExprRegionId> expr_regions_;
+  ExprRegionStore expr_regions_;
 
   // C++ source locations for C++ interop.
-  ValueStore<ClangSourceLocId> clang_source_locs_;
+  ClangSourceLocStore clang_source_locs_;
 };
 
 }  // namespace Carbon::SemIR

+ 3 - 0
toolchain/sem_ir/function.h

@@ -6,6 +6,7 @@
 #define CARBON_TOOLCHAIN_SEM_IR_FUNCTION_H_
 
 #include "clang/AST/Decl.h"
+#include "toolchain/base/value_store.h"
 #include "toolchain/sem_ir/builtin_function_kind.h"
 #include "toolchain/sem_ir/clang_decl.h"
 #include "toolchain/sem_ir/entity_with_params_base.h"
@@ -157,6 +158,8 @@ struct Function : public EntityWithParamsBase,
   }
 };
 
+using FunctionStore = ValueStore<FunctionId, Function>;
+
 class File;
 
 struct CalleeFunction : public Printable<CalleeFunction> {

+ 2 - 2
toolchain/sem_ir/generic.cpp

@@ -19,7 +19,7 @@ class SpecificStore::KeyContext : public TranslatingKeyContext<KeyContext> {
     friend auto operator==(const Key&, const Key&) -> bool = default;
   };
 
-  explicit KeyContext(const ValueStore<SpecificId>* specifics)
+  explicit KeyContext(const ValueStore<SpecificId, Specific>* specifics)
       : specifics_(specifics) {}
 
   auto TranslateKey(SpecificId id) const -> Key {
@@ -28,7 +28,7 @@ class SpecificStore::KeyContext : public TranslatingKeyContext<KeyContext> {
   }
 
  private:
-  const ValueStore<SpecificId>* specifics_;
+  const ValueStore<SpecificId, Specific>* specifics_;
 };
 
 auto SpecificStore::GetOrAdd(GenericId generic_id, InstBlockId args_id)

+ 3 - 3
toolchain/sem_ir/generic.h

@@ -53,7 +53,7 @@ struct Generic : public Printable<Generic> {
 };
 
 // Provides storage for generics.
-class GenericStore : public ValueStore<GenericId> {
+class GenericStore : public ValueStore<GenericId, Generic> {
  public:
   // Get the self specific for a generic, or `None` if the `id` is `None`.
   auto GetSelfSpecific(GenericId id) const -> SpecificId {
@@ -139,7 +139,7 @@ class SpecificStore : public Yaml::Printable<SpecificStore> {
       -> void;
 
   auto values() const [[clang::lifetimebound]]
-  -> ValueStore<SpecificId>::Range {
+  -> ValueStore<SpecificId, Specific>::Range {
     return specifics_.values();
   }
   auto size() const -> size_t { return specifics_.size(); }
@@ -151,7 +151,7 @@ class SpecificStore : public Yaml::Printable<SpecificStore> {
   // Context for hashing keys.
   class KeyContext;
 
-  ValueStore<SpecificId> specifics_;
+  ValueStore<SpecificId, Specific> specifics_;
   Carbon::Set<SpecificId, 0, KeyContext> lookup_table_;
 };
 

+ 19 - 55
toolchain/sem_ir/ids.h

@@ -14,42 +14,17 @@
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/parse/node_ids.h"
 
-// NOLINTNEXTLINE(readability-identifier-naming)
-namespace clang {
-
-// Forward declare indexed types, for integration with ValueStore.
-class SourceLocation;
-
-}  // namespace clang
-
 namespace Carbon::SemIR {
 
 // Forward declare indexed types, for integration with ValueStore.
 class File;
-class ImportIRInst;
-class Inst;
-class NameScope;
-struct AssociatedConstant;
-struct Class;
-struct EntityName;
-struct ExprRegion;
 struct FacetTypeInfo;
-struct Function;
-struct Generic;
-struct Specific;
 struct SpecificInterface;
-struct ImportCpp;
-struct ImportIR;
-struct Impl;
-struct Interface;
 struct StructTypeField;
-struct TypeInfo;
-struct Vtable;
 
-// The ID of an instruction.
+// The ID of an `Inst`.
 struct InstId : public IdBase<InstId> {
   static constexpr llvm::StringLiteral Label = "inst";
-  using ValueType = Inst;
 
   // The maximum ID, inclusive.
   static constexpr int Max = std::numeric_limits<int32_t>::max();
@@ -65,7 +40,7 @@ struct InstId : public IdBase<InstId> {
 
 constexpr InstId InstId::InitTombstone = InstId(NoneIndex - 1);
 
-// And InstId whose value is a type. The fact it's a type is CHECKed on
+// An InstId whose value is a type. The fact it's a type is CHECKed on
 // construction, and this allows that check to be represented in the type
 // system.
 struct TypeInstId : public InstId {
@@ -239,10 +214,9 @@ struct ConstantId : public IdBase<ConstantId> {
 
 constexpr ConstantId ConstantId::NotConstant = ConstantId(NotConstantIndex);
 
-// The ID of a EntityName.
+// The ID of a `EntityName`.
 struct EntityNameId : public IdBase<EntityNameId> {
   static constexpr llvm::StringLiteral Label = "entity_name";
-  using ValueType = EntityName;
 
   using IdBase::IdBase;
 };
@@ -267,10 +241,9 @@ struct CallParamIndex : public IndexBase<CallParamIndex> {
   using IndexBase::IndexBase;
 };
 
-// The ID of a function.
+// The ID of a `Function`.
 struct FunctionId : public IdBase<FunctionId> {
   static constexpr llvm::StringLiteral Label = "function";
-  using ValueType = Function;
 
   using IdBase::IdBase;
 };
@@ -288,33 +261,30 @@ struct CheckIRId : public IdBase<CheckIRId> {
 
 constexpr CheckIRId CheckIRId::Cpp = CheckIRId(NoneIndex - 1);
 
-// The ID of a class.
+// The ID of a `Class`.
 struct ClassId : public IdBase<ClassId> {
   static constexpr llvm::StringLiteral Label = "class";
-  using ValueType = Class;
 
   using IdBase::IdBase;
 };
 
+// The ID of a `Vtable`.
 struct VtableId : public IdBase<VtableId> {
   static constexpr llvm::StringLiteral Label = "vtable";
-  using ValueType = Vtable;
 
   using IdBase::IdBase;
 };
 
-// The ID of an interface.
+// The ID of an `Interface`.
 struct InterfaceId : public IdBase<InterfaceId> {
   static constexpr llvm::StringLiteral Label = "interface";
-  using ValueType = Interface;
 
   using IdBase::IdBase;
 };
 
-// The ID of an associated constant.
+// The ID of an `AssociatedConstant`.
 struct AssociatedConstantId : public IdBase<AssociatedConstantId> {
   static constexpr llvm::StringLiteral Label = "assoc_const";
-  using ValueType = AssociatedConstant;
 
   using IdBase::IdBase;
 };
@@ -334,31 +304,28 @@ struct IdentifiedFacetTypeId : public IdBase<IdentifiedFacetTypeId> {
   using IdBase::IdBase;
 };
 
-// The ID of an impl.
+// The ID of an `Impl`.
 struct ImplId : public IdBase<ImplId> {
   using DiagnosticType = Diagnostics::TypeInfo<std::string>;
 
   static constexpr llvm::StringLiteral Label = "impl";
-  using ValueType = Impl;
 
   using IdBase::IdBase;
 };
 
-// The ID of a generic.
+// The ID of a `Generic`.
 struct GenericId : public IdBase<GenericId> {
   static constexpr llvm::StringLiteral Label = "generic";
-  using ValueType = Generic;
 
   using IdBase::IdBase;
 };
 
-// The ID of a specific, which is the result of specifying the generic arguments
-// for a generic.
+// The ID of a `Specific`, which is the result of specifying the generic
+// arguments for a generic.
 struct SpecificId : public IdBase<SpecificId> {
   using DiagnosticType = Diagnostics::TypeInfo<std::string>;
 
   static constexpr llvm::StringLiteral Label = "specific";
-  using ValueType = Specific;
 
   using IdBase::IdBase;
 };
@@ -423,17 +390,17 @@ struct GenericInstIndex : public IndexBase<GenericInstIndex> {
 constexpr GenericInstIndex GenericInstIndex::None =
     GenericInstIndex::MakeNone();
 
+// The ID of an `ImportCpp`.
 struct ImportCppId : public IdBase<ImportCppId> {
   static constexpr llvm::StringLiteral Label = "import_cpp";
-  using ValueType = ImportCpp;
 
   using IdBase::IdBase;
 };
 
-// The ID of an IR within the set of imported IRs, both direct and indirect.
+// The ID of an `ImportIR` within the set of imported IRs, both direct and
+// indirect.
 struct ImportIRId : public IdBase<ImportIRId> {
   static constexpr llvm::StringLiteral Label = "ir";
-  using ValueType = ImportIR;
 
   // The implicit `api` import, for an `impl` file. A null entry is added if
   // there is none, as in an `api`, in which case this ID should not show up in
@@ -600,10 +567,9 @@ constexpr int NameId::NonIndexValueCount =
     1 CARBON_SPECIAL_NAME_ID(CARBON_SPECIAL_NAME_ID_FOR_COUNT);
 #undef CARBON_SPECIAL_NAME_ID_FOR_COUNT
 
-// The ID of a name scope.
+// The ID of a `NameScope`.
 struct NameScopeId : public IdBase<NameScopeId> {
   static constexpr llvm::StringLiteral Label = "name_scope";
-  using ValueType = NameScope;
 
   // The package (or file) name scope, guaranteed to be the first added.
   static const NameScopeId Package;
@@ -739,11 +705,11 @@ class LabelId : public InstBlockId {
   using InstBlockId::InstBlockId;
 };
 
+// The ID of an `ExprRegion`.
 // TODO: Move this out of sem_ir and into check, if we don't wind up using it
 // in the SemIR for expression patterns.
 struct ExprRegionId : public IdBase<ExprRegionId> {
   static constexpr llvm::StringLiteral Label = "region";
-  using ValueType = ExprRegion;
 
   using IdBase::IdBase;
 };
@@ -793,10 +759,9 @@ struct TypeId : public IdBase<TypeId> {
   auto Print(llvm::raw_ostream& out) const -> void;
 };
 
-// The ID of a Clang Source Location.
+// The ID of a `clang::SourceLocation`.
 struct ClangSourceLocId : public IdBase<ClangSourceLocId> {
   static constexpr llvm::StringLiteral Label = "clang_source_loc";
-  using ValueType = clang::SourceLocation;
 
   using IdBase::IdBase;
 };
@@ -835,10 +800,9 @@ struct LibraryNameId : public IdBase<LibraryNameId> {
 constexpr LibraryNameId LibraryNameId::Default = LibraryNameId(NoneIndex - 1);
 constexpr LibraryNameId LibraryNameId::Error = LibraryNameId(NoneIndex - 2);
 
-// The ID of an ImportIRInst.
+// The ID of an `ImportIRInst`.
 struct ImportIRInstId : public IdBase<ImportIRInstId> {
   static constexpr llvm::StringLiteral Label = "import_ir_inst";
-  using ValueType = ImportIRInst;
 
   // The maximum ID, non-inclusive. This is constrained to fit inside LocId.
   static constexpr int Max =

+ 3 - 2
toolchain/sem_ir/impl.h

@@ -192,7 +192,8 @@ class ImplStore {
     mem_usage.Collect(MemUsage::ConcatLabel(label, "lookup_"), lookup_);
   }
 
-  auto values() const [[clang::lifetimebound]] -> ValueStore<ImplId>::Range {
+  auto values() const [[clang::lifetimebound]]
+  -> ValueStore<ImplId, Impl>::Range {
     return values_.values();
   }
   auto size() const -> size_t { return values_.size(); }
@@ -202,7 +203,7 @@ class ImplStore {
 
  private:
   File& sem_ir_;
-  ValueStore<ImplId> values_;
+  ValueStore<ImplId, Impl> values_;
   Map<std::tuple<InstId, InterfaceId, SpecificId>, ImplOrLookupBucketId>
       lookup_;
   // Buckets with at least 2 entries, which will be rare; see LookupBucketRef.

+ 2 - 0
toolchain/sem_ir/import_cpp.h

@@ -20,6 +20,8 @@ struct ImportCpp : Printable<ImportCpp> {
   StringLiteralValueId library_id;
 };
 
+using ImportCppStore = ValueStore<ImportCppId, ImportCpp>;
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_IMPORT_CPP_H_

+ 5 - 0
toolchain/sem_ir/import_ir.h

@@ -6,6 +6,7 @@
 #define CARBON_TOOLCHAIN_SEM_IR_IMPORT_IR_H_
 
 #include "llvm/ADT/FoldingSet.h"
+#include "toolchain/base/value_store.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst.h"
 
@@ -25,6 +26,8 @@ struct ImportIR : public Printable<ImportIR> {
 
 static_assert(sizeof(ImportIR) == 8 + sizeof(uintptr_t), "Unexpected size");
 
+using ImportIRStore = ValueStore<ImportIRId, ImportIR>;
+
 // A reference to an instruction in an imported IR. Used for diagnostics with
 // LocId. For a `Cpp` import, points to a Clang source location.
 class ImportIRInst : public Printable<ImportIRInst> {
@@ -70,6 +73,8 @@ class ImportIRInst : public Printable<ImportIRInst> {
   };
 };
 
+using ImportIRInstStore = ValueStore<ImportIRInstId, ImportIRInst>;
+
 // Returns the canonical `File` and `InstId` for an entity, tracing imported
 // instructions. Note the returned `File` might not be directly imported by the
 // input `sem_ir`.

+ 3 - 2
toolchain/sem_ir/inst.h

@@ -592,7 +592,8 @@ class InstStore {
     mem_usage.Collect(MemUsage::ConcatLabel(label, "values_"), values_);
   }
 
-  auto values() const [[clang::lifetimebound]] -> ValueStore<InstId>::Range {
+  auto values() const [[clang::lifetimebound]]
+  -> ValueStore<InstId, Inst>::Range {
     return values_.values();
   }
   auto size() const -> int { return values_.size(); }
@@ -614,7 +615,7 @@ class InstStore {
 
   File* file_;
   llvm::SmallVector<LocId> loc_ids_;
-  ValueStore<InstId> values_;
+  ValueStore<InstId, Inst> values_;
 };
 
 // Adapts BlockValueStore for instruction blocks.

+ 3 - 0
toolchain/sem_ir/interface.h

@@ -5,6 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_INTERFACE_H_
 #define CARBON_TOOLCHAIN_SEM_IR_INTERFACE_H_
 
+#include "toolchain/base/value_store.h"
 #include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
 
@@ -48,6 +49,8 @@ struct Interface : public EntityWithParamsBase,
   }
 };
 
+using InterfaceStore = ValueStore<InterfaceId, Interface>;
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_INTERFACE_H_

+ 3 - 3
toolchain/sem_ir/name.h

@@ -5,7 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_NAME_H_
 #define CARBON_TOOLCHAIN_SEM_IR_NAME_H_
 
-#include "toolchain/base/canonical_value_store.h"
+#include "toolchain/base/shared_value_stores.h"
 #include "toolchain/base/value_ids.h"
 #include "toolchain/sem_ir/ids.h"
 
@@ -26,7 +26,7 @@ namespace Carbon::SemIR {
 class NameStoreWrapper {
  public:
   explicit NameStoreWrapper(
-      const CanonicalValueStore<IdentifierId>* identifiers)
+      const SharedValueStores::IdentifierStore* identifiers)
       : identifiers_(identifiers) {}
 
   // Returns the requested name as a string, if it is an identifier name. This
@@ -51,7 +51,7 @@ class NameStoreWrapper {
   auto GetIRBaseName(NameId name_id) const -> llvm::StringRef;
 
  private:
-  const CanonicalValueStore<IdentifierId>* identifiers_;
+  const SharedValueStores::IdentifierStore* identifiers_;
 };
 
 }  // namespace Carbon::SemIR

+ 1 - 1
toolchain/sem_ir/name_scope.h

@@ -367,7 +367,7 @@ class NameScopeStore {
 
  private:
   const File* file_;
-  ValueStore<NameScopeId> values_;
+  ValueStore<NameScopeId, NameScope> values_;
 };
 
 }  // namespace Carbon::SemIR

+ 3 - 0
toolchain/sem_ir/vtable.h

@@ -5,6 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_VTABLE_H_
 #define CARBON_TOOLCHAIN_SEM_IR_VTABLE_H_
 
+#include "toolchain/base/value_store.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::SemIR {
@@ -28,6 +29,8 @@ struct Vtable : public VtableFields, public Printable<Vtable> {
   }
 };
 
+using VtableStore = ValueStore<VtableId, Vtable>;
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_VTABLE_H_