Automatically update constant_values() when adding a constant to the constant store. (#3618)

As requested in review of #3611.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

toolchain/check/context.cpp

@@ -103,15 +103,8 @@ auto Context::AddPlaceholderInst(SemIR::ParseNodeAndInst parse_node_and_inst)
 
 auto Context::AddConstant(SemIR::Inst inst, bool is_symbolic)
     -> SemIR::ConstantId {
-  auto [inst_id, added] = constants().GetOrAdd(inst);
-  auto const_id = is_symbolic ? SemIR::ConstantId::ForSymbolicConstant(inst_id)
-                              : SemIR::ConstantId::ForTemplateConstant(inst_id);
-  if (added) {
-    // TODO: Should `ConstantStore` do this for us?
-    constant_values().Set(inst_id, const_id);
-  }
-
-  CARBON_VLOG() << "AddConstantInst: " << inst << "\n";
+  auto const_id = constants().GetOrAdd(inst, is_symbolic);
+  CARBON_VLOG() << "AddConstant: " << inst << "\n";
   return const_id;
 }
 

toolchain/sem_ir/value_stores.cpp

@@ -10,30 +10,41 @@
 
 namespace Carbon::SemIR {
 
-auto ConstantStore::GetOrAdd(Inst inst) -> std::pair<InstId, bool> {
+auto ConstantStore::GetOrAdd(Inst inst, bool is_symbolic) -> ConstantId {
   // Compute the instruction's profile.
-  ConstantNode node = {.inst = inst, .inst_id = InstId::Invalid};
+  ConstantNode node = {.inst = inst, .constant_id = ConstantId::NotConstant};
   llvm::FoldingSetNodeID id;
   node.Profile(id, constants_.getContext());
 
   // Check if we have already created this constant.
   void* insert_pos;
   if (ConstantNode* found = constants_.FindNodeOrInsertPos(id, insert_pos)) {
-    return {found->inst_id, false};
+    CARBON_CHECK(found->constant_id.is_constant())
+        << "Found non-constant in constant store for " << inst;
+    CARBON_CHECK(found->constant_id.is_symbolic() == is_symbolic)
+        << "Mismatch in phase for constant " << inst;
+    return found->constant_id;
   }
 
   // Create the new inst and insert the new node.
-  node.inst_id = constants_.getContext()->insts().AddInNoBlock(
+  auto inst_id = constants_.getContext()->insts().AddInNoBlock(
       ParseNodeAndInst::Untyped(Parse::NodeId::Invalid, inst));
+  auto constant_id = is_symbolic
+                         ? SemIR::ConstantId::ForSymbolicConstant(inst_id)
+                         : SemIR::ConstantId::ForTemplateConstant(inst_id);
+  node.constant_id = constant_id;
   constants_.InsertNode(new (*allocator_) ConstantNode(node), insert_pos);
-  return {node.inst_id, true};
+
+  // The constant value of any constant instruction is that instruction itself.
+  constants_.getContext()->constant_values().Set(inst_id, constant_id);
+  return constant_id;
 }
 
 auto ConstantStore::GetAsVector() const -> llvm::SmallVector<InstId, 0> {
   llvm::SmallVector<InstId, 0> result;
   result.reserve(constants_.size());
   for (const ConstantNode& node : constants_) {
-    result.push_back(node.inst_id);
+    result.push_back(node.constant_id.inst_id());
   }
   // For stability, put the results into index order. This happens to also be
   // insertion order.

toolchain/sem_ir/value_stores.h

@@ -141,9 +141,11 @@ class ConstantStore {
       : allocator_(&allocator), constants_(&sem_ir) {}
 
   // Adds a new constant instruction, or gets the existing constant with this
-  // value. Returns the instruction ID and a bool indicating whether a new
-  // instruction was added.
-  auto GetOrAdd(Inst inst) -> std::pair<InstId, bool>;
+  // value. Returns the ID of the constant.
+  //
+  // This updates `sem_ir.insts()` and `sem_ir.constant_values()` if the
+  // constant is new.
+  auto GetOrAdd(Inst inst, bool is_symbolic) -> ConstantId;
 
   // Returns a copy of the constant IDs as a vector, in an arbitrary but
   // stable order. This should not be used anywhere performance-sensitive.
@@ -159,7 +161,7 @@ class ConstantStore {
   // impact on compile time from that change.
   struct ConstantNode : llvm::FoldingSetNode {
     Inst inst;
-    InstId inst_id;
+    ConstantId constant_id;
 
     auto Profile(llvm::FoldingSetNodeID& id, File* sem_ir) -> void;
   };