Split IdentifierId and StringLiteralId from StringId (#3352)

Following up on discussion yesterday regarding this split.

Note, I'm expecting #3341 to do IdentifierId -> NameId in SemIR. It
might be worth adding NameId creation directly to StringStore if you're
content with this setup though.

language_server/language_server.cpp

@@ -82,7 +82,7 @@ static auto GetName(const SharedValueStores& value_stores,
                     Parse::Node node) -> std::optional<llvm::StringRef> {
   for (auto ch : p.children(node)) {
     if (p.node_kind(ch) == Parse::NodeKind::Name) {
-      return value_stores.strings().Get(
+      return value_stores.identifiers().Get(
           tokens.GetIdentifier(p.node_token(node)));
     }
   }

toolchain/base/value_store.h

@@ -84,6 +84,29 @@ struct StringId : public IndexBase, public Printable<StringId> {
 };
 constexpr StringId StringId::Invalid(StringId::InvalidIndex);
 
+// Adapts StringId for identifiers.
+struct IdentifierId : public IndexBase, public Printable<IdentifierId> {
+  static const IdentifierId Invalid;
+  using IndexBase::IndexBase;
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "strId";
+    IndexBase::Print(out);
+  }
+};
+constexpr IdentifierId IdentifierId::Invalid(IdentifierId::InvalidIndex);
+
+// Adapts StringId for string literals.
+struct StringLiteralId : public IndexBase, public Printable<StringLiteralId> {
+  static const StringLiteralId Invalid;
+  using IndexBase::IndexBase;
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "strLit";
+    IndexBase::Print(out);
+  }
+};
+constexpr StringLiteralId StringLiteralId::Invalid(
+    StringLiteralId::InvalidIndex);
+
 namespace Internal {
 // Used as a parent class for non-printable types. This is just for
 // std::conditional, not as an API.
@@ -98,9 +121,6 @@ class ValueStore
                               Yaml::Printable<ValueStore<IdT, ValueT>>,
                               Internal::ValueStoreNotPrintable> {
  public:
-  using PrintFn =
-      llvm::function_ref<void(llvm::raw_ostream&, const ValueT& val)>;
-
   // Stores the value and returns an ID to reference it.
   auto Add(ValueT value) -> IdT {
     IdT id = IdT(values_.size());
@@ -153,9 +173,6 @@ class ValueStore
 template <>
 class ValueStore<StringId> : public Yaml::Printable<ValueStore<StringId>> {
  public:
-  using PrintFn =
-      llvm::function_ref<void(llvm::raw_ostream&, const llvm::StringRef& val)>;
-
   // Returns an ID to reference the value. May return an existing ID if the
   // string was previously added.
   auto Add(llvm::StringRef value) -> StringId {
@@ -186,16 +203,55 @@ class ValueStore<StringId> : public Yaml::Printable<ValueStore<StringId>> {
   llvm::SmallVector<llvm::StringRef> values_;
 };
 
+// A thin wrapper around a `ValueStore<StringId>` that provides a different IdT,
+// while using a unified storage for values. This avoids potentially
+// duplicative string hash maps, which are expensive.
+template <typename IdT>
+class StringStoreWrapper : public Printable<StringStoreWrapper<IdT>> {
+ public:
+  explicit StringStoreWrapper(ValueStore<StringId>* values) : values_(values) {}
+
+  auto Add(llvm::StringRef value) -> IdT {
+    return IdT(values_->Add(value).index);
+  }
+
+  auto Get(IdT id) const -> llvm::StringRef {
+    return values_->Get(StringId(id.index));
+  }
+
+  auto Print(llvm::raw_ostream& out) const -> void { out << *values_; }
+
+ private:
+  ValueStore<StringId>* values_;
+};
+
 // Stores that will be used across compiler phases for a given compilation unit.
 // This is provided mainly so that they don't need to be passed separately.
 class SharedValueStores : public Yaml::Printable<SharedValueStores> {
  public:
+  explicit SharedValueStores()
+      : identifiers_(&strings_), string_literals_(&strings_) {}
+
+  // Not copyable or movable.
+  SharedValueStores(const SharedValueStores&) = delete;
+  auto operator=(const SharedValueStores&) -> SharedValueStores& = delete;
+
+  auto identifiers() -> StringStoreWrapper<IdentifierId>& {
+    return identifiers_;
+  }
+  auto identifiers() const -> const StringStoreWrapper<IdentifierId>& {
+    return identifiers_;
+  }
   auto integers() -> ValueStore<IntegerId>& { return integers_; }
   auto integers() const -> const ValueStore<IntegerId>& { return integers_; }
   auto reals() -> ValueStore<RealId>& { return reals_; }
   auto reals() const -> const ValueStore<RealId>& { return reals_; }
-  auto strings() -> ValueStore<StringId>& { return strings_; }
-  auto strings() const -> const ValueStore<StringId>& { return strings_; }
+  auto string_literals() -> StringStoreWrapper<StringLiteralId>& {
+    return string_literals_;
+  }
+  auto string_literals() const -> const StringStoreWrapper<StringLiteralId>& {
+    return string_literals_;
+  }
 
   auto OutputYaml(std::optional<llvm::StringRef> filename = std::nullopt) const
       -> Yaml::OutputMapping {
@@ -215,11 +271,19 @@ class SharedValueStores : public Yaml::Printable<SharedValueStores> {
  private:
   ValueStore<IntegerId> integers_;
   ValueStore<RealId> reals_;
+
   ValueStore<StringId> strings_;
+  StringStoreWrapper<IdentifierId> identifiers_;
+  StringStoreWrapper<StringLiteralId> string_literals_;
 };
 
 }  // namespace Carbon
 
+// Support use of IdentifierId as DenseMap/DenseSet keys.
+// TODO: Remove once NameId is used in checking.
+template <>
+struct llvm::DenseMapInfo<Carbon::IdentifierId>
+    : public Carbon::IndexMapInfo<Carbon::IdentifierId> {};
 // Support use of StringId as DenseMap/DenseSet keys.
 template <>
 struct llvm::DenseMapInfo<Carbon::StringId>

toolchain/base/value_store_test.cpp

@@ -63,19 +63,20 @@ TEST(ValueStore, String) {
   std::string a = "a";
   std::string b = "b";
   SharedValueStores value_stores;
-  StringId a_id = value_stores.strings().Add(a);
-  StringId b_id = value_stores.strings().Add(b);
+  auto a_id = value_stores.identifiers().Add(a);
+  auto b_id = value_stores.string_literals().Add(b);
 
   ASSERT_TRUE(a_id.is_valid());
   ASSERT_TRUE(b_id.is_valid());
 
-  EXPECT_THAT(a_id, Not(Eq(b_id)));
-  EXPECT_THAT(value_stores.strings().Get(a_id), Eq(a));
-  EXPECT_THAT(value_stores.strings().Get(b_id), Eq(b));
+  EXPECT_THAT(a_id.index, Not(Eq(b_id.index)));
+  EXPECT_THAT(value_stores.identifiers().Get(a_id), Eq(a));
+  EXPECT_THAT(value_stores.string_literals().Get(b_id), Eq(b));
 
-  // Adding the same string again should return the same id.
-  EXPECT_THAT(value_stores.strings().Add(a), Eq(a_id));
-  EXPECT_THAT(value_stores.strings().Add(b), Eq(b_id));
+  // Adding the same string again, even with a different Id type, should return
+  // the same id.
+  EXPECT_THAT(value_stores.string_literals().Add(a).index, Eq(a_id.index));
+  EXPECT_THAT(value_stores.identifiers().Add(b).index, Eq(b_id.index));
 }
 
 auto MatchSharedValues(testing::Matcher<Yaml::MappingValue> integers,
@@ -102,7 +103,7 @@ TEST(ValueStore, PrintVals) {
   value_stores.integers().Add(apint);
   value_stores.reals().Add(
       Real{.mantissa = apint, .exponent = apint, .is_decimal = true});
-  value_stores.strings().Add("foo'\"baz");
+  value_stores.string_literals().Add("foo'\"baz");
   TestRawOstream out;
   value_stores.Print(out);
 

toolchain/check/context.cpp

@@ -81,11 +81,11 @@ auto Context::DiagnoseDuplicateName(Parse::Node parse_node,
       .Emit();
 }
 
-auto Context::DiagnoseNameNotFound(Parse::Node parse_node, StringId name_id)
+auto Context::DiagnoseNameNotFound(Parse::Node parse_node, IdentifierId name_id)
     -> void {
   CARBON_DIAGNOSTIC(NameNotFound, Error, "Name `{0}` not found.",
                     llvm::StringRef);
-  emitter_->Emit(parse_node, NameNotFound, strings().Get(name_id));
+  emitter_->Emit(parse_node, NameNotFound, identifiers().Get(name_id));
 }
 
 auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
@@ -105,7 +105,7 @@ auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
   }
 }
 
-auto Context::AddNameToLookup(Parse::Node name_node, StringId name_id,
+auto Context::AddNameToLookup(Parse::Node name_node, IdentifierId name_id,
                               SemIR::InstId target_id) -> void {
   if (current_scope().names.insert(name_id).second) {
     name_lookup_[name_id].push_back(target_id);
@@ -114,7 +114,7 @@ auto Context::AddNameToLookup(Parse::Node name_node, StringId name_id,
   }
 }
 
-auto Context::LookupName(Parse::Node parse_node, StringId name_id,
+auto Context::LookupName(Parse::Node parse_node, IdentifierId name_id,
                          SemIR::NameScopeId scope_id, bool print_diagnostics)
     -> SemIR::InstId {
   if (scope_id == SemIR::NameScopeId::Invalid) {
@@ -126,7 +126,7 @@ auto Context::LookupName(Parse::Node parse_node, StringId name_id,
       return SemIR::InstId::BuiltinError;
     }
     CARBON_CHECK(!it->second.empty())
-        << "Should have been erased: " << strings().Get(name_id);
+        << "Should have been erased: " << identifiers().Get(name_id);
 
     // TODO: Check for ambiguous lookups.
     return it->second.back();

toolchain/check/context.h

@@ -49,12 +49,12 @@ class Context {
   auto AddInstAndPush(Parse::Node parse_node, SemIR::Inst inst) -> void;
 
   // Adds a name to name lookup. Prints a diagnostic for name conflicts.
-  auto AddNameToLookup(Parse::Node name_node, StringId name_id,
+  auto AddNameToLookup(Parse::Node name_node, IdentifierId name_id,
                        SemIR::InstId target_id) -> void;
 
   // Performs name lookup in a specified scope, returning the referenced
   // instruction. If scope_id is invalid, uses the current contextual scope.
-  auto LookupName(Parse::Node parse_node, StringId name_id,
+  auto LookupName(Parse::Node parse_node, IdentifierId name_id,
                   SemIR::NameScopeId scope_id, bool print_diagnostics)
       -> SemIR::InstId;
 
@@ -63,7 +63,8 @@ class Context {
       -> void;
 
   // Prints a diagnostic for a missing name.
-  auto DiagnoseNameNotFound(Parse::Node parse_node, StringId name_id) -> void;
+  auto DiagnoseNameNotFound(Parse::Node parse_node, IdentifierId name_id)
+      -> void;
 
   // Adds a note to a diagnostic explaining that a class is incomplete.
   auto NoteIncompleteClass(SemIR::ClassId class_id, DiagnosticBuilder& builder)
@@ -246,9 +247,14 @@ class Context {
   }
 
   // Directly expose SemIR::File data accessors for brevity in calls.
+  auto identifiers() -> StringStoreWrapper<IdentifierId>& {
+    return sem_ir().identifiers();
+  }
   auto integers() -> ValueStore<IntegerId>& { return sem_ir().integers(); }
   auto reals() -> ValueStore<RealId>& { return sem_ir().reals(); }
-  auto strings() -> ValueStore<StringId>& { return sem_ir().strings(); }
+  auto string_literals() -> StringStoreWrapper<StringLiteralId>& {
+    return sem_ir().string_literals();
+  }
   auto functions() -> ValueStore<SemIR::FunctionId, SemIR::Function>& {
     return sem_ir().functions();
   }
@@ -299,7 +305,7 @@ class Context {
 
     // Names which are registered with name_lookup_, and will need to be
     // deregistered when the scope ends.
-    llvm::DenseSet<StringId> names;
+    llvm::DenseSet<IdentifierId> names;
 
     // TODO: This likely needs to track things which need to be destructed.
   };
@@ -382,7 +388,7 @@ class Context {
   // reference.
   //
   // Names which no longer have lookup results are erased.
-  llvm::DenseMap<StringId, llvm::SmallVector<SemIR::InstId>> name_lookup_;
+  llvm::DenseMap<IdentifierId, llvm::SmallVector<SemIR::InstId>> name_lookup_;
 
   // Cache of the mapping from instructions to types, to avoid recomputing the
   // folding set ID.

toolchain/check/convert.cpp

@@ -421,7 +421,7 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type,
   }
 
   // Prepare to look up fields in the source by index.
-  llvm::SmallDenseMap<StringId, int32_t> src_field_indexes;
+  llvm::SmallDenseMap<IdentifierId, int32_t> src_field_indexes;
   if (src_type.fields_id != dest_type.fields_id) {
     for (auto [i, field_id] : llvm::enumerate(src_elem_fields)) {
       auto [it, added] = src_field_indexes.insert(
@@ -462,7 +462,7 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type,
               llvm::StringRef);
           context.emitter().Emit(value.parse_node(),
                                  StructInitMissingFieldInLiteral,
-                                 sem_ir.strings().Get(dest_field.name_id));
+                                 sem_ir.identifiers().Get(dest_field.name_id));
         } else {
           CARBON_DIAGNOSTIC(StructInitMissingFieldInConversion, Error,
                             "Cannot convert from struct type `{0}` to `{1}`: "
@@ -472,7 +472,7 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type,
                                  StructInitMissingFieldInConversion,
                                  sem_ir.StringifyType(value.type_id()),
                                  sem_ir.StringifyType(target.type_id),
-                                 sem_ir.strings().Get(dest_field.name_id));
+                                 sem_ir.identifiers().Get(dest_field.name_id));
         }
         return SemIR::InstId::BuiltinError;
       }

toolchain/check/declaration_name_stack.cpp

@@ -13,7 +13,7 @@ auto DeclarationNameStack::MakeEmptyNameContext() -> NameContext {
 }
 
 auto DeclarationNameStack::MakeUnqualifiedName(Parse::Node parse_node,
-                                               StringId name_id)
+                                               IdentifierId name_id)
     -> NameContext {
   NameContext context = MakeEmptyNameContext();
   ApplyNameQualifierTo(context, parse_node, name_id);
@@ -86,13 +86,13 @@ auto DeclarationNameStack::AddNameToLookup(NameContext name_context,
 }
 
 auto DeclarationNameStack::ApplyNameQualifier(Parse::Node parse_node,
-                                              StringId name_id) -> void {
+                                              IdentifierId name_id) -> void {
   ApplyNameQualifierTo(declaration_name_stack_.back(), parse_node, name_id);
 }
 
 auto DeclarationNameStack::ApplyNameQualifierTo(NameContext& name_context,
                                                 Parse::Node parse_node,
-                                                StringId name_id) -> void {
+                                                IdentifierId name_id) -> void {
   if (CanResolveQualifier(name_context, parse_node)) {
     // For identifier nodes, we need to perform a lookup on the identifier.
     // This means the input instruction name_id is actually a string ID.

toolchain/check/declaration_name_stack.h

@@ -88,7 +88,7 @@ class DeclarationNameStack {
       SemIR::InstId resolved_inst_id = SemIR::InstId::Invalid;
 
       // The ID of an unresolved identifier.
-      StringId unresolved_name_id;
+      IdentifierId unresolved_name_id;
     };
   };
 
@@ -107,12 +107,12 @@ class DeclarationNameStack {
   // unqualified name in the current context. This is suitable for adding to
   // name lookup in situations where a qualified name is not permitted, such as
   // a pattern binding.
-  auto MakeUnqualifiedName(Parse::Node parse_node, StringId name_id)
+  auto MakeUnqualifiedName(Parse::Node parse_node, IdentifierId name_id)
       -> NameContext;
 
   // Applies a Name from the name stack to the top of the declaration name
   // stack.
-  auto ApplyNameQualifier(Parse::Node parse_node, StringId name_id) -> void;
+  auto ApplyNameQualifier(Parse::Node parse_node, IdentifierId name_id) -> void;
 
   // Adds a name to name lookup. Prints a diagnostic for name conflicts.
   auto AddNameToLookup(NameContext name_context, SemIR::InstId target_id)
@@ -129,7 +129,7 @@ class DeclarationNameStack {
 
   // Applies a Name from the name stack to given name context.
   auto ApplyNameQualifierTo(NameContext& name_context, Parse::Node parse_node,
-                            StringId name_id) -> void;
+                            IdentifierId name_id) -> void;
 
   // Returns true if the context is in a state where it can resolve qualifiers.
   // Updates name_context as needed.

toolchain/check/handle_class.cpp

@@ -54,7 +54,7 @@ static auto BuildClassDeclaration(Context& context)
         {.name_id = name_context.state ==
                             DeclarationNameStack::NameContext::State::Unresolved
                         ? name_context.unresolved_name_id
-                        : StringId::Invalid,
+                        : IdentifierId::Invalid,
          // `.self_type_id` depends on `class_id`, so is set below.
          .self_type_id = SemIR::TypeId::Invalid,
          .declaration_id = class_decl_id});
@@ -92,7 +92,7 @@ auto HandleClassDefinitionStart(Context& context, Parse::Node parse_node)
                       "Previous definition was here.");
     context.emitter()
         .Build(parse_node, ClassRedefinition,
-               context.strings().Get(class_info.name_id))
+               context.identifiers().Get(class_info.name_id))
         .Note(context.insts().Get(class_info.definition_id).parse_node(),
               ClassPreviousDefinition)
         .Emit();
@@ -110,7 +110,7 @@ auto HandleClassDefinitionStart(Context& context, Parse::Node parse_node)
   // HandleSelfTypeNameExpression.
   context.AddNameToLookup(
       parse_node,
-      context.strings().Add(
+      context.identifiers().Add(
           Lex::TokenKind::SelfTypeIdentifier.fixed_spelling()),
       context.sem_ir().GetTypeAllowBuiltinTypes(class_info.self_type_id));
 

toolchain/check/handle_function.cpp

@@ -101,7 +101,7 @@ static auto BuildFunctionDeclaration(Context& context, bool is_definition)
         {.name_id = name_context.state ==
                             DeclarationNameStack::NameContext::State::Unresolved
                         ? name_context.unresolved_name_id
-                        : StringId::Invalid,
+                        : IdentifierId::Invalid,
          .implicit_param_refs_id = implicit_param_refs_id,
          .param_refs_id = param_refs_id,
          .return_type_id = return_type_id,
@@ -174,7 +174,7 @@ auto HandleFunctionDefinitionStart(Context& context, Parse::Node parse_node)
                       "Previous definition was here.");
     context.emitter()
         .Build(parse_node, FunctionRedefinition,
-               context.strings().Get(function.name_id))
+               context.identifiers().Get(function.name_id))
         .Note(context.insts().Get(function.definition_id).parse_node(),
               FunctionPreviousDefinition)
         .Emit();
@@ -212,9 +212,9 @@ auto HandleFunctionDefinitionStart(Context& context, Parse::Node parse_node)
       // TODO: This will shadow a local variable named `r#self`, but should
       // not. See #2984 and the corresponding code in
       // HandleSelfTypeNameExpression.
-      context.AddNameToLookup(self_param->parse_node,
-                              context.strings().Add(SemIR::SelfParameter::Name),
-                              param_id);
+      context.AddNameToLookup(
+          self_param->parse_node,
+          context.identifiers().Add(SemIR::SelfParameter::Name), param_id);
     } else {
       CARBON_FATAL() << "Unexpected kind of parameter in function definition "
                      << param;
@@ -244,8 +244,8 @@ auto HandleReturnType(Context& context, Parse::Node parse_node) -> bool {
   auto type_id = ExpressionAsType(context, type_parse_node, type_inst_id);
   // TODO: Use a dedicated instruction rather than VarStorage here.
   context.AddInstAndPush(
-      parse_node,
-      SemIR::VarStorage{parse_node, type_id, context.strings().Add("return")});
+      parse_node, SemIR::VarStorage{parse_node, type_id,
+                                    context.identifiers().Add("return")});
   return true;
 }
 

toolchain/check/handle_name.cpp

@@ -56,7 +56,7 @@ static auto GetExpressionValueForLookupResult(Context& context,
 
 auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
     -> bool {
-  StringId name_id = context.node_stack().Pop<Parse::NodeKind::Name>();
+  IdentifierId name_id = context.node_stack().Pop<Parse::NodeKind::Name>();
   auto base_id = context.node_stack().PopExpression();
 
   // If the base is a name scope, such as a class or namespace, perform lookup
@@ -195,7 +195,7 @@ auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
                         llvm::StringRef);
       context.emitter().Emit(parse_node, QualifiedExpressionNameNotFound,
                              context.sem_ir().StringifyType(base_type_id),
-                             context.strings().Get(name_id));
+                             context.identifiers().Get(name_id));
       break;
     }
     // TODO: `ConstType` should support member access just like the
@@ -283,7 +283,7 @@ auto HandleSelfTypeNameExpression(Context& context, Parse::Node parse_node)
   // TODO: This will find a local variable declared with name `r#Self`, but
   // should not. See #2984 and the corresponding code in
   // HandleClassDefinitionStart.
-  auto name_id = context.strings().Add(
+  auto name_id = context.identifiers().Add(
       Lex::TokenKind::SelfTypeIdentifier.fixed_spelling());
   auto value_id =
       context.LookupName(parse_node, name_id, SemIR::NameScopeId::Invalid,
@@ -305,7 +305,7 @@ auto HandleSelfValueNameExpression(Context& context, Parse::Node parse_node)
   // TODO: This will find a local variable declared with name `r#self`, but
   // should not. See #2984 and the corresponding code in
   // HandleFunctionDefinitionStart.
-  auto name_id = context.strings().Add(SemIR::SelfParameter::Name);
+  auto name_id = context.identifiers().Add(SemIR::SelfParameter::Name);
   auto value_id =
       context.LookupName(parse_node, name_id, SemIR::NameScopeId::Invalid,
                          /*print_diagnostics=*/true);

toolchain/check/handle_struct.cpp

@@ -41,7 +41,7 @@ auto HandleStructFieldUnknown(Context& context, Parse::Node parse_node)
 auto HandleStructFieldValue(Context& context, Parse::Node parse_node) -> bool {
   auto [value_parse_node, value_inst_id] =
       context.node_stack().PopExpressionWithParseNode();
-  StringId name_id = context.node_stack().Pop<Parse::NodeKind::Name>();
+  IdentifierId name_id = context.node_stack().Pop<Parse::NodeKind::Name>();
 
   // Store the name for the type.
   context.args_type_info_stack().AddInst(SemIR::StructTypeField{

toolchain/check/node_stack.h

@@ -131,8 +131,8 @@ class NodeStack {
       RequireParseKind<RequiredParseKind>(back.first);
       return back;
     }
-    if constexpr (RequiredIdKind == IdKind::StringId) {
-      auto back = PopWithParseNode<StringId>();
+    if constexpr (RequiredIdKind == IdKind::IdentifierId) {
+      auto back = PopWithParseNode<IdentifierId>();
       RequireParseKind<RequiredParseKind>(back.first);
       return back;
     }
@@ -192,8 +192,8 @@ class NodeStack {
     if constexpr (RequiredIdKind == IdKind::ClassId) {
       return back.id<SemIR::ClassId>();
     }
-    if constexpr (RequiredIdKind == IdKind::StringId) {
-      return back.id<StringId>();
+    if constexpr (RequiredIdKind == IdKind::IdentifierId) {
+      return back.id<IdentifierId>();
     }
     if constexpr (RequiredIdKind == IdKind::TypeId) {
       return back.id<SemIR::TypeId>();
@@ -216,7 +216,7 @@ class NodeStack {
     InstBlockId,
     FunctionId,
     ClassId,
-    StringId,
+    IdentifierId,
     TypeId,
     // No associated ID type.
     SoloParseNode,
@@ -234,7 +234,7 @@ class NodeStack {
         : parse_node(parse_node), function_id(function_id) {}
     explicit Entry(Parse::Node parse_node, SemIR::ClassId class_id)
         : parse_node(parse_node), class_id(class_id) {}
-    explicit Entry(Parse::Node parse_node, StringId name_id)
+    explicit Entry(Parse::Node parse_node, IdentifierId name_id)
         : parse_node(parse_node), name_id(name_id) {}
     explicit Entry(Parse::Node parse_node, SemIR::TypeId type_id)
         : parse_node(parse_node), type_id(type_id) {}
@@ -254,7 +254,7 @@ class NodeStack {
       if constexpr (std::is_same<T, SemIR::ClassId>()) {
         return class_id;
       }
-      if constexpr (std::is_same<T, StringId>()) {
+      if constexpr (std::is_same<T, IdentifierId>()) {
         return name_id;
       }
       if constexpr (std::is_same<T, SemIR::TypeId>()) {
@@ -275,7 +275,7 @@ class NodeStack {
       SemIR::InstBlockId inst_block_id;
       SemIR::FunctionId function_id;
       SemIR::ClassId class_id;
-      StringId name_id;
+      IdentifierId name_id;
       SemIR::TypeId type_id;
     };
   };
@@ -320,7 +320,7 @@ class NodeStack {
       case Parse::NodeKind::ClassDefinitionStart:
         return IdKind::ClassId;
       case Parse::NodeKind::Name:
-        return IdKind::StringId;
+        return IdKind::IdentifierId;
       case Parse::NodeKind::ArrayExpressionSemi:
       case Parse::NodeKind::ClassIntroducer:
       case Parse::NodeKind::CodeBlockStart:
@@ -358,8 +358,8 @@ class NodeStack {
     if constexpr (std::is_same_v<IdT, SemIR::ClassId>) {
       return IdKind::ClassId;
     }
-    if constexpr (std::is_same_v<IdT, StringId>) {
-      return IdKind::StringId;
+    if constexpr (std::is_same_v<IdT, IdentifierId>) {
+      return IdKind::IdentifierId;
     }
     if constexpr (std::is_same_v<IdT, SemIR::TypeId>) {
       return IdKind::TypeId;

toolchain/check/testdata/basics/multifile_raw_and_textual_ir.carbon

@@ -19,7 +19,7 @@ fn B() {}
 // CHECK:STDOUT: sem_ir:
 // CHECK:STDOUT:   cross_reference_irs_size: 1
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: str0, param_refs: block0, body: [block1]}
+// CHECK:STDOUT:     function0:       {name: strId0, param_refs: block0, body: [block1]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     type0:           {inst: instFunctionType, value_rep: {kind: copy, type: type0}}
@@ -48,7 +48,7 @@ fn B() {}
 // CHECK:STDOUT: sem_ir:
 // CHECK:STDOUT:   cross_reference_irs_size: 1
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: str0, param_refs: block0, body: [block1]}
+// CHECK:STDOUT:     function0:       {name: strId0, param_refs: block0, body: [block1]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     type0:           {inst: instFunctionType, value_rep: {kind: copy, type: type0}}

toolchain/check/testdata/basics/multifile_raw_ir.carbon

@@ -19,7 +19,7 @@ fn B() {}
 // CHECK:STDOUT: sem_ir:
 // CHECK:STDOUT:   cross_reference_irs_size: 1
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: str0, param_refs: block0, body: [block1]}
+// CHECK:STDOUT:     function0:       {name: strId0, param_refs: block0, body: [block1]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     type0:           {inst: instFunctionType, value_rep: {kind: copy, type: type0}}
@@ -39,7 +39,7 @@ fn B() {}
 // CHECK:STDOUT: sem_ir:
 // CHECK:STDOUT:   cross_reference_irs_size: 1
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: str0, param_refs: block0, body: [block1]}
+// CHECK:STDOUT:     function0:       {name: strId0, param_refs: block0, body: [block1]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     type0:           {inst: instFunctionType, value_rep: {kind: copy, type: type0}}

toolchain/check/testdata/basics/raw_and_textual_ir.carbon

@@ -17,7 +17,7 @@ fn Foo(n: i32) -> (i32, f64) {
 // CHECK:STDOUT: sem_ir:
 // CHECK:STDOUT:   cross_reference_irs_size: 1
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: str0, param_refs: block1, return_type: type3, return_slot: inst+4, body: [block4]}
+// CHECK:STDOUT:     function0:       {name: strId0, param_refs: block1, return_type: type3, return_slot: inst+4, body: [block4]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     type0:           {inst: instIntegerType, value_rep: {kind: copy, type: type0}}
@@ -34,14 +34,14 @@ fn Foo(n: i32) -> (i32, f64) {
 // CHECK:STDOUT:       0:               type0
 // CHECK:STDOUT:       1:               type2
 // CHECK:STDOUT:   insts:
-// CHECK:STDOUT:     inst+0:          {kind: Parameter, arg0: str1, type: type0}
+// CHECK:STDOUT:     inst+0:          {kind: Parameter, arg0: strId1, type: type0}
 // CHECK:STDOUT:     inst+1:          {kind: TupleType, arg0: typeBlock0, type: typeTypeType}
 // CHECK:STDOUT:     inst+2:          {kind: TupleLiteral, arg0: block2, type: type1}
 // CHECK:STDOUT:     inst+3:          {kind: TupleType, arg0: typeBlock1, type: typeTypeType}
-// CHECK:STDOUT:     inst+4:          {kind: VarStorage, arg0: str2, type: type3}
+// CHECK:STDOUT:     inst+4:          {kind: VarStorage, arg0: strId2, type: type3}
 // CHECK:STDOUT:     inst+5:          {kind: PointerType, arg0: type3, type: typeTypeType}
 // CHECK:STDOUT:     inst+6:          {kind: FunctionDeclaration, arg0: function0, type: type5}
-// CHECK:STDOUT:     inst+7:          {kind: NameReference, arg0: str1, arg1: inst+0, type: type0}
+// CHECK:STDOUT:     inst+7:          {kind: NameReference, arg0: strId1, arg1: inst+0, type: type0}
 // CHECK:STDOUT:     inst+8:          {kind: IntegerLiteral, arg0: int3, type: type0}
 // CHECK:STDOUT:     inst+9:          {kind: BinaryOperatorAdd, arg0: inst+7, arg1: inst+8, type: type0}
 // CHECK:STDOUT:     inst+10:         {kind: RealLiteral, arg0: real0, type: type2}

toolchain/check/testdata/basics/raw_ir.carbon

@@ -17,7 +17,7 @@ fn Foo(n: i32) -> (i32, f64) {
 // CHECK:STDOUT: sem_ir:
 // CHECK:STDOUT:   cross_reference_irs_size: 1
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: str0, param_refs: block1, return_type: type3, return_slot: inst+4, body: [block4]}
+// CHECK:STDOUT:     function0:       {name: strId0, param_refs: block1, return_type: type3, return_slot: inst+4, body: [block4]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   types:
 // CHECK:STDOUT:     type0:           {inst: instIntegerType, value_rep: {kind: copy, type: type0}}
@@ -34,14 +34,14 @@ fn Foo(n: i32) -> (i32, f64) {
 // CHECK:STDOUT:       0:               type0
 // CHECK:STDOUT:       1:               type2
 // CHECK:STDOUT:   insts:
-// CHECK:STDOUT:     inst+0:          {kind: Parameter, arg0: str1, type: type0}
+// CHECK:STDOUT:     inst+0:          {kind: Parameter, arg0: strId1, type: type0}
 // CHECK:STDOUT:     inst+1:          {kind: TupleType, arg0: typeBlock0, type: typeTypeType}
 // CHECK:STDOUT:     inst+2:          {kind: TupleLiteral, arg0: block2, type: type1}
 // CHECK:STDOUT:     inst+3:          {kind: TupleType, arg0: typeBlock1, type: typeTypeType}
-// CHECK:STDOUT:     inst+4:          {kind: VarStorage, arg0: str2, type: type3}
+// CHECK:STDOUT:     inst+4:          {kind: VarStorage, arg0: strId2, type: type3}
 // CHECK:STDOUT:     inst+5:          {kind: PointerType, arg0: type3, type: typeTypeType}
 // CHECK:STDOUT:     inst+6:          {kind: FunctionDeclaration, arg0: function0, type: type5}
-// CHECK:STDOUT:     inst+7:          {kind: NameReference, arg0: str1, arg1: inst+0, type: type0}
+// CHECK:STDOUT:     inst+7:          {kind: NameReference, arg0: strId1, arg1: inst+0, type: type0}
 // CHECK:STDOUT:     inst+8:          {kind: IntegerLiteral, arg0: int3, type: type0}
 // CHECK:STDOUT:     inst+9:          {kind: BinaryOperatorAdd, arg0: inst+7, arg1: inst+8, type: type0}
 // CHECK:STDOUT:     inst+10:         {kind: RealLiteral, arg0: real0, type: type2}

toolchain/lex/tokenized_buffer.cpp

@@ -659,12 +659,12 @@ class [[clang::internal_linkage]] TokenizedBuffer::Lexer {
     }
 
     if (literal->is_terminated()) {
-      auto string_id = buffer_.value_stores_->strings().Add(
+      auto string_id = buffer_.value_stores_->string_literals().Add(
           literal->ComputeValue(buffer_.allocator_, emitter_));
       auto token = buffer_.AddToken({.kind = TokenKind::StringLiteral,
                                      .token_line = string_line,
                                      .column = string_column,
-                                     .string_id = string_id});
+                                     .string_literal_id = string_id});
       return token;
     } else {
       CARBON_DIAGNOSTIC(UnterminatedString, Error,
@@ -897,7 +897,8 @@ class [[clang::internal_linkage]] TokenizedBuffer::Lexer {
         {.kind = TokenKind::Identifier,
          .token_line = current_line(),
          .column = column,
-         .string_id = buffer_.value_stores_->strings().Add(identifier_text)});
+         .ident_id =
+             buffer_.value_stores_->identifiers().Add(identifier_text)});
   }
 
   auto LexKeywordOrIdentifierMaybeRaw(llvm::StringRef source_text,
@@ -933,7 +934,8 @@ class [[clang::internal_linkage]] TokenizedBuffer::Lexer {
         {.kind = TokenKind::Identifier,
          .token_line = current_line(),
          .column = column,
-         .string_id = buffer_.value_stores_->strings().Add(identifier_text)});
+         .ident_id =
+             buffer_.value_stores_->identifiers().Add(identifier_text)});
   }
 
   auto LexError(llvm::StringRef source_text, ssize_t& position) -> LexResult {
@@ -1326,13 +1328,13 @@ auto TokenizedBuffer::GetTokenText(Token token) const -> llvm::StringRef {
   }
 
   CARBON_CHECK(token_info.kind == TokenKind::Identifier) << token_info.kind;
-  return value_stores_->strings().Get(token_info.string_id);
+  return value_stores_->identifiers().Get(token_info.ident_id);
 }
 
-auto TokenizedBuffer::GetIdentifier(Token token) const -> StringId {
+auto TokenizedBuffer::GetIdentifier(Token token) const -> IdentifierId {
   const auto& token_info = GetTokenInfo(token);
   CARBON_CHECK(token_info.kind == TokenKind::Identifier) << token_info.kind;
-  return token_info.string_id;
+  return token_info.ident_id;
 }
 
 auto TokenizedBuffer::GetIntegerLiteral(Token token) const -> IntegerId {
@@ -1347,10 +1349,10 @@ auto TokenizedBuffer::GetRealLiteral(Token token) const -> RealId {
   return token_info.real_id;
 }
 
-auto TokenizedBuffer::GetStringLiteral(Token token) const -> StringId {
+auto TokenizedBuffer::GetStringLiteral(Token token) const -> StringLiteralId {
   const auto& token_info = GetTokenInfo(token);
   CARBON_CHECK(token_info.kind == TokenKind::StringLiteral) << token_info.kind;
-  return token_info.string_id;
+  return token_info.string_literal_id;
 }
 
 auto TokenizedBuffer::GetTypeLiteralSize(Token token) const
@@ -1506,7 +1508,8 @@ auto TokenizedBuffer::PrintToken(llvm::raw_ostream& output_stream, Token token,
       break;
     case TokenKind::StringLiteral:
       output_stream << ", value: `"
-                    << value_stores_->strings().Get(GetStringLiteral(token))
+                    << value_stores_->string_literals().Get(
+                           GetStringLiteral(token))
                     << "`";
       break;
     default:

toolchain/lex/tokenized_buffer.h

@@ -154,7 +154,7 @@ class TokenizedBuffer : public Printable<TokenizedBuffer> {
 
   // Returns the identifier associated with this token. The token kind must be
   // an `Identifier`.
-  [[nodiscard]] auto GetIdentifier(Token token) const -> StringId;
+  [[nodiscard]] auto GetIdentifier(Token token) const -> IdentifierId;
 
   // Returns the value of an `IntegerLiteral()` token.
   [[nodiscard]] auto GetIntegerLiteral(Token token) const -> IntegerId;
@@ -163,7 +163,7 @@ class TokenizedBuffer : public Printable<TokenizedBuffer> {
   [[nodiscard]] auto GetRealLiteral(Token token) const -> RealId;
 
   // Returns the value of a `StringLiteral()` token.
-  [[nodiscard]] auto GetStringLiteral(Token token) const -> StringId;
+  [[nodiscard]] auto GetStringLiteral(Token token) const -> StringLiteralId;
 
   // Returns the size specified in a `*TypeLiteral()` token.
   [[nodiscard]] auto GetTypeLiteralSize(Token token) const
@@ -300,7 +300,8 @@ class TokenizedBuffer : public Printable<TokenizedBuffer> {
           sizeof(Token) <= sizeof(int32_t),
           "Unable to pack token and identifier index into the same space!");
 
-      StringId string_id = StringId::Invalid;
+      IdentifierId ident_id = IdentifierId::Invalid;
+      StringLiteralId string_literal_id;
       IntegerId integer_id;
       RealId real_id;
       Token closing_token;

toolchain/lex/tokenized_buffer_test.cpp

@@ -453,7 +453,8 @@ TEST_F(LexerTest, MatchingGroups) {
     auto open_paren_token = *it++;
     auto open_curly_token = *it++;
 
-    ASSERT_EQ("x", value_stores_.strings().Get(buffer.GetIdentifier(*it++)));
+    ASSERT_EQ("x",
+              value_stores_.identifiers().Get(buffer.GetIdentifier(*it++)));
     auto close_curly_token = *it++;
     auto close_paren_token = *it++;
     EXPECT_EQ(close_paren_token,
@@ -467,7 +468,8 @@ TEST_F(LexerTest, MatchingGroups) {
 
     open_curly_token = *it++;
     open_paren_token = *it++;
-    ASSERT_EQ("y", value_stores_.strings().Get(buffer.GetIdentifier(*it++)));
+    ASSERT_EQ("y",
+              value_stores_.identifiers().Get(buffer.GetIdentifier(*it++)));
     close_paren_token = *it++;
     close_curly_token = *it++;
     EXPECT_EQ(close_curly_token,
@@ -483,7 +485,8 @@ TEST_F(LexerTest, MatchingGroups) {
     auto inner_open_curly_token = *it++;
     open_paren_token = *it++;
     auto inner_open_paren_token = *it++;
-    ASSERT_EQ("z", value_stores_.strings().Get(buffer.GetIdentifier(*it++)));
+    ASSERT_EQ("z",
+              value_stores_.identifiers().Get(buffer.GetIdentifier(*it++)));
     auto inner_close_paren_token = *it++;
     close_paren_token = *it++;
     auto inner_close_curly_token = *it++;

toolchain/lex/tokenized_buffer_test_helpers.h

@@ -124,7 +124,8 @@ MATCHER_P(HasTokens, raw_all_expected, "") {
     if (expected.string_contents &&
         actual_kind == Lex::TokenKind::StringLiteral) {
       llvm::StringRef actual_contents =
-          expected.value_stores->strings().Get(buffer.GetStringLiteral(token));
+          expected.value_stores->string_literals().Get(
+              buffer.GetStringLiteral(token));
       if (actual_contents != *expected.string_contents) {
         *result_listener << "\nToken " << index << " has contents `"
                          << actual_contents.str() << "`, expected `"

toolchain/lower/file_context.cpp

@@ -119,7 +119,7 @@ auto FileContext::BuildFunctionDeclaration(SemIR::FunctionId function_id)
     mangled_name = "main";
   } else {
     // TODO: Decide on a name mangling scheme.
-    mangled_name = sem_ir().strings().Get(function.name_id);
+    mangled_name = sem_ir().identifiers().Get(function.name_id);
   }
 
   llvm::FunctionType* function_type =
@@ -139,7 +139,8 @@ auto FileContext::BuildFunctionDeclaration(SemIR::FunctionId function_id)
     } else if (inst.Is<SemIR::SelfParameter>()) {
       arg.setName("self");
     } else {
-      arg.setName(sem_ir().strings().Get(inst.As<SemIR::Parameter>().name_id));
+      arg.setName(
+          sem_ir().identifiers().Get(inst.As<SemIR::Parameter>().name_id));
     }
   }
 

toolchain/lower/handle.cpp

@@ -255,7 +255,7 @@ static auto GetStructFieldName(FunctionContext& context,
           .fields_id);
   auto field = context.sem_ir().insts().GetAs<SemIR::StructTypeField>(
       fields[index.index]);
-  return context.sem_ir().strings().Get(field.name_id);
+  return context.sem_ir().identifiers().Get(field.name_id);
 }
 
 auto HandleClassFieldAccess(FunctionContext& context, SemIR::InstId inst_id,
@@ -558,7 +558,7 @@ auto HandleVarStorage(FunctionContext& context, SemIR::InstId inst_id,
   // TODO: Eventually this name will be optional, and we'll want to provide
   // something like `var` as a default. However, that's not possible right now
   // so cannot be tested.
-  auto name = context.sem_ir().strings().Get(inst.name_id);
+  auto name = context.sem_ir().identifiers().Get(inst.name_id);
   auto* alloca = context.builder().CreateAlloca(context.GetType(inst.type_id),
                                                 /*ArraySize=*/nullptr, name);
   context.SetLocal(inst_id, alloca);

toolchain/sem_ir/BUILD

@@ -81,6 +81,7 @@ cc_library(
     deps = [
         ":file",
         ":inst_kind",
+        "//toolchain/base:value_store",
         "//toolchain/lex:tokenized_buffer",
         "//toolchain/parse:tree",
         "@llvm-project//llvm:Support",

toolchain/sem_ir/entry_point.cpp

@@ -16,7 +16,7 @@ auto IsEntryPoint(const SemIR::File& file, SemIR::FunctionId function_id)
   const auto& function = file.functions().Get(function_id);
   // TODO: Check if `function` is in a namespace.
   return function.name_id.is_valid() &&
-         file.strings().Get(function.name_id) == EntryPointFunction;
+         file.identifiers().Get(function.name_id) == EntryPointFunction;
 }
 
 }  // namespace Carbon::SemIR

toolchain/sem_ir/file.cpp

@@ -287,7 +287,7 @@ auto File::StringifyTypeExpression(InstId outer_inst_id,
       case ClassType::Kind: {
         auto class_name_id =
             classes().Get(inst.As<ClassType>().class_id).name_id;
-        out << strings().Get(class_name_id);
+        out << identifiers().Get(class_name_id);
         break;
       }
       case ConstType::Kind: {
@@ -310,7 +310,7 @@ auto File::StringifyTypeExpression(InstId outer_inst_id,
         break;
       }
       case NameReference::Kind: {
-        out << strings().Get(inst.As<NameReference>().name_id);
+        out << identifiers().Get(inst.As<NameReference>().name_id);
         break;
       }
       case PointerType::Kind: {
@@ -343,7 +343,7 @@ auto File::StringifyTypeExpression(InstId outer_inst_id,
       }
       case StructTypeField::Kind: {
         auto field = inst.As<StructTypeField>();
-        out << "." << strings().Get(field.name_id) << ": ";
+        out << "." << identifiers().Get(field.name_id) << ": ";
         steps.push_back(
             {.inst_id = GetTypeAllowBuiltinTypes(field.field_type_id)});
         break;

toolchain/sem_ir/file.h

@@ -36,7 +36,7 @@ struct Function : public Printable<Function> {
   }
 
   // The function name.
-  StringId name_id;
+  IdentifierId name_id;
   // The definition, if the function has been defined or is currently being
   // defined. This is a FunctionDeclaration.
   InstId definition_id = InstId::Invalid;
@@ -73,7 +73,7 @@ struct Class : public Printable<Class> {
   // lifetime of the class.
 
   // The class name.
-  StringId name_id;
+  IdentifierId name_id;
   // The class type, which is the type of `Self` in the class definition.
   TypeId self_type_id;
   // The first declaration of the class. This is a ClassDeclaration.
@@ -258,6 +258,12 @@ class File : public Printable<File> {
       -> std::string;
 
   // Directly expose SharedValueStores members.
+  auto identifiers() -> StringStoreWrapper<IdentifierId>& {
+    return value_stores_->identifiers();
+  }
+  auto identifiers() const -> const StringStoreWrapper<IdentifierId>& {
+    return value_stores_->identifiers();
+  }
   auto integers() -> ValueStore<IntegerId>& {
     return value_stores_->integers();
   }
@@ -268,9 +274,11 @@ class File : public Printable<File> {
   auto reals() const -> const ValueStore<RealId>& {
     return value_stores_->reals();
   }
-  auto strings() -> ValueStore<StringId>& { return value_stores_->strings(); }
-  auto strings() const -> const ValueStore<StringId>& {
-    return value_stores_->strings();
+  auto string_literals() -> StringStoreWrapper<StringLiteralId>& {
+    return value_stores_->string_literals();
+  }
+  auto string_literals() const -> const StringStoreWrapper<StringLiteralId>& {
+    return value_stores_->string_literals();
   }
 
   auto functions() -> ValueStore<FunctionId, Function>& { return functions_; }

toolchain/sem_ir/formatter.cpp

@@ -8,6 +8,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "toolchain/base/value_store.h"
 #include "toolchain/lex/tokenized_buffer.h"
 #include "toolchain/parse/tree.h"
 
@@ -53,7 +54,8 @@ class InstNamer {
       auto fn_loc = Parse::Node::Invalid;
       GetScopeInfo(fn_scope).name = globals.AllocateName(
           *this, fn_loc,
-          fn.name_id.is_valid() ? sem_ir.strings().Get(fn.name_id).str() : "");
+          fn.name_id.is_valid() ? sem_ir.identifiers().Get(fn.name_id).str()
+                                : "");
       CollectNamesInBlock(fn_scope, fn.implicit_param_refs_id);
       CollectNamesInBlock(fn_scope, fn.param_refs_id);
       if (fn.return_slot_id.is_valid()) {
@@ -84,7 +86,7 @@ class InstNamer {
       GetScopeInfo(class_scope).name = globals.AllocateName(
           *this, class_loc,
           class_info.name_id.is_valid()
-              ? sem_ir.strings().Get(class_info.name_id).str()
+              ? sem_ir.identifiers().Get(class_info.name_id).str()
               : "");
       AddBlockLabel(class_scope, class_info.body_block_id, "class", class_loc);
       CollectNamesInBlock(class_scope, class_info.body_block_id);
@@ -387,10 +389,11 @@ class InstNamer {
         insts[inst_id.index] = {scope_idx, scope.insts.AllocateName(
                                                *this, inst.parse_node(), name)};
       };
-      auto add_inst_name_id = [&](StringId name_id,
+      auto add_inst_name_id = [&](IdentifierId name_id,
                                   llvm::StringRef suffix = "") {
         if (name_id.is_valid()) {
-          add_inst_name((sem_ir_.strings().Get(name_id).str() + suffix).str());
+          add_inst_name(
+              (sem_ir_.identifiers().Get(name_id).str() + suffix).str());
         } else {
           add_inst_name(suffix.str());
         }
@@ -593,7 +596,7 @@ class Formatter {
                        llvm::StringRef prefix) -> void {
     // Name scopes aren't kept in any particular order. Sort the entries before
     // we print them for stability and consistency.
-    llvm::SmallVector<std::pair<InstId, StringId>> entries;
+    llvm::SmallVector<std::pair<InstId, IdentifierId>> entries;
     for (auto [name_id, inst_id] : sem_ir_.name_scopes().Get(id)) {
       entries.push_back({inst_id, name_id});
     }
@@ -830,6 +833,12 @@ class Formatter {
 
   auto FormatArg(ClassId id) -> void { FormatClassName(id); }
 
+  auto FormatArg(IdentifierId id) -> void {
+    out_ << '"';
+    out_.write_escaped(sem_ir_.identifiers().Get(id), /*UseHexEscapes=*/true);
+    out_ << '"';
+  }
+
   auto FormatArg(IntegerId id) -> void {
     sem_ir_.integers().Get(id).print(out_, /*isSigned=*/false);
   }
@@ -861,9 +870,10 @@ class Formatter {
     out_ << (real.is_decimal ? 'e' : 'p') << real.exponent;
   }
 
-  auto FormatArg(StringId id) -> void {
+  auto FormatArg(StringLiteralId id) -> void {
     out_ << '"';
-    out_.write_escaped(sem_ir_.strings().Get(id), /*UseHexEscapes=*/true);
+    out_.write_escaped(sem_ir_.string_literals().Get(id),
+                       /*UseHexEscapes=*/true);
     out_ << '"';
   }
 
@@ -892,7 +902,9 @@ class Formatter {
     out_ << inst_namer_.GetLabelFor(scope_, id);
   }
 
-  auto FormatString(StringId id) -> void { out_ << sem_ir_.strings().Get(id); }
+  auto FormatString(IdentifierId id) -> void {
+    out_ << sem_ir_.identifiers().Get(id);
+  }
 
   auto FormatFunctionName(FunctionId id) -> void {
     out_ << inst_namer_.GetNameFor(id);

toolchain/sem_ir/typed_insts.h

@@ -101,7 +101,7 @@ struct BindName {
 
   Parse::Node parse_node;
   TypeId type_id;
-  StringId name_id;
+  IdentifierId name_id;
   InstId value_id;
 };
 
@@ -262,7 +262,7 @@ struct Field {
 
   Parse::Node parse_node;
   TypeId type_id;
-  StringId name_id;
+  IdentifierId name_id;
   MemberIndex index;
 };
 
@@ -300,7 +300,7 @@ struct NameReference {
 
   Parse::Node parse_node;
   TypeId type_id;
-  StringId name_id;
+  IdentifierId name_id;
   InstId value_id;
 };
 
@@ -324,7 +324,7 @@ struct Parameter {
 
   Parse::Node parse_node;
   TypeId type_id;
-  StringId name_id;
+  IdentifierId name_id;
 };
 
 struct PointerType {
@@ -383,7 +383,7 @@ struct StringLiteral {
 
   Parse::Node parse_node;
   TypeId type_id;
-  StringId string_id;
+  StringLiteralId string_literal_id;
 };
 
 struct StructAccess {
@@ -428,7 +428,7 @@ struct StructTypeField {
   // This instruction is an implementation detail of `StructType`, and doesn't
   // produce a value, so has no type, even though it declares a field with a
   // type.
-  StringId name_id;
+  IdentifierId name_id;
   TypeId field_type_id;
 };
 
@@ -556,7 +556,7 @@ struct VarStorage {
 
   Parse::Node parse_node;
   TypeId type_id;
-  StringId name_id;
+  IdentifierId name_id;
 };
 
 // HasParseNode<T> is true if T has a `Parse::Node parse_node` field.

toolchain/sem_ir/value_stores.h

@@ -53,19 +53,19 @@ class NameScopeStore {
 
   // Adds an entry to a name scope. Returns true on success, false on
   // duplicates.
-  auto AddEntry(NameScopeId scope_id, StringId name_id, InstId target_id)
+  auto AddEntry(NameScopeId scope_id, IdentifierId name_id, InstId target_id)
       -> bool {
     return values_.Get(scope_id).insert({name_id, target_id}).second;
   }
 
   // Returns the requested name scope.
   auto Get(NameScopeId scope_id) const
-      -> const llvm::DenseMap<StringId, InstId>& {
+      -> const llvm::DenseMap<IdentifierId, InstId>& {
     return values_.Get(scope_id);
   }
 
  private:
-  ValueStore<NameScopeId, llvm::DenseMap<StringId, InstId>> values_;
+  ValueStore<NameScopeId, llvm::DenseMap<IdentifierId, InstId>> values_;
 };
 
 // Provides a block-based ValueStore, which uses slab allocation of added