Consolidate post-check logic (#5003)

Right now, some post-run logic is does in `Run()`
(`CheckRequiredDefinitions();` and
`context_.sem_ir().set_has_errors(unit_and_imports_->err_tracker.seen_error());`)
whereas other parts are done by `Finalize`. Noting the goal to move
things off `Context`, this consolidates into a new `FinishRun`. Note
#4962 is adding another bit of post-run that can be consolidated in;
this seems likely to keep growing slowly.

Note this also creates more parity with mutation source, like the
`context_.scope_stack().Pop();` matches the push done by
`CheckUnit::ImportCurrentPackage` and
`context_.inst_block_stack().Pop()` was pushed in `CheckUnit::Run()`.

Also makes `exports()` more consistent with other Context APIs. Makes
`VerifyOnFinish` `const` so that it can't accidentally mutate state, and
is instead only validating that the Context is in its expected
configuration at completion.

toolchain/check/check_unit.cpp

@@ -76,22 +76,7 @@ auto CheckUnit::Run() -> void {
     return;
   }
 
-  CheckRequiredDefinitions();
-
-  CheckRequiredDeclarations();
-
-  context_.Finalize();
-
-  context_.VerifyOnFinish();
-
-  context_.sem_ir().set_has_errors(unit_and_imports_->err_tracker.seen_error());
-
-#ifndef NDEBUG
-  if (auto verify = context_.sem_ir().Verify(); !verify.ok()) {
-    CARBON_FATAL("{0}Built invalid semantics IR: {1}\n", context_.sem_ir(),
-                 verify.error());
-  }
-#endif
+  FinishRun();
 }
 
 auto CheckUnit::InitPackageScopeAndImports() -> void {
@@ -505,4 +490,26 @@ auto CheckUnit::CheckRequiredDefinitions() -> void {
   }
 }
 
+auto CheckUnit::FinishRun() -> void {
+  CheckRequiredDeclarations();
+  CheckRequiredDefinitions();
+
+  // Pop information for the file-level scope.
+  context_.sem_ir().set_top_inst_block_id(context_.inst_block_stack().Pop());
+  context_.scope_stack().Pop();
+
+  // Finalizes the list of exports on the IR.
+  context_.inst_blocks().Set(SemIR::InstBlockId::Exports, context_.exports());
+  // Finalizes the ImportRef inst block.
+  context_.inst_blocks().Set(SemIR::InstBlockId::ImportRefs,
+                             context_.import_ref_ids());
+  // Finalizes __global_init.
+  context_.global_init().Finalize();
+
+  context_.sem_ir().set_has_errors(unit_and_imports_->err_tracker.seen_error());
+
+  // Verify that Context cleanly finished.
+  context_.VerifyOnFinish();
+}
+
 }  // namespace Carbon::Check

toolchain/check/check_unit.h

@@ -153,16 +153,20 @@ class CheckUnit {
   // Imports all other Carbon packages (excluding the current package).
   auto ImportOtherPackages(SemIR::TypeId namespace_type_id) -> void;
 
+  // Checks that each required declaration is available. This applies for
+  // declarations that should exist in an owning library, for which an extern
+  // declaration exists that assigns ownership to the current API.
+  auto CheckRequiredDeclarations() -> void;
+
   // Checks that each required definition is available. If the definition can be
   // generated by resolving a specific, does so, otherwise emits a diagnostic
   // for each declaration in context.definitions_required() that doesn't have a
   // definition.
   auto CheckRequiredDefinitions() -> void;
 
-  // Checks that each required declaration is available. This applies for
-  // declarations that should exist in an owning library, for which an extern
-  // declaration exists that assigns ownership to the current API.
-  auto CheckRequiredDeclarations() -> void;
+  // Does work after processing the parse tree, such as finishing the IR and
+  // checking for missing contents.
+  auto FinishRun() -> void;
 
   // Loops over all nodes in the tree. On some errors, this may return early,
   // for example if an unrecoverable state is encountered.

toolchain/check/context.cpp

@@ -75,7 +75,7 @@ auto Context::TODO(SemIRLoc loc, std::string label) -> bool {
   return false;
 }
 
-auto Context::VerifyOnFinish() -> void {
+auto Context::VerifyOnFinish() const -> void {
   // Information in all the various context objects should be cleaned up as
   // various pieces of context go out of scope. At this point, nothing should
   // remain.
@@ -89,19 +89,13 @@ auto Context::VerifyOnFinish() -> void {
   // decl_introducer_state_stack_.
   scope_stack_.VerifyOnFinish();
   // TODO: Add verification for generic_region_stack_.
-}
-
-auto Context::Finalize() -> void {
-  // Pop information for the file-level scope.
-  sem_ir().set_top_inst_block_id(inst_block_stack().Pop());
-  scope_stack().Pop();
 
-  // Finalizes the list of exports on the IR.
-  inst_blocks().Set(SemIR::InstBlockId::Exports, exports_);
-  // Finalizes the ImportRef inst block.
-  inst_blocks().Set(SemIR::InstBlockId::ImportRefs, import_ref_ids_);
-  // Finalizes __global_init.
-  global_init_.Finalize();
+#ifndef NDEBUG
+  if (auto verify = sem_ir_->Verify(); !verify.ok()) {
+    CARBON_FATAL("{0}Built invalid semantics IR: {1}\n", sem_ir_,
+                 verify.error());
+  }
+#endif
 }
 
 auto Context::PrintForStackDump(llvm::raw_ostream& output) const -> void {

toolchain/check/context.h

@@ -65,12 +65,7 @@ class Context {
   auto TODO(SemIRLoc loc, std::string label) -> bool;
 
   // Runs verification that the processing cleanly finished.
-  auto VerifyOnFinish() -> void;
-
-  // Adds an exported name.
-  auto AddExport(SemIR::InstId inst_id) -> void { exports_.push_back(inst_id); }
-
-  auto Finalize() -> void;
+  auto VerifyOnFinish() const -> void;
 
   // Prints information for a stack dump.
   auto PrintForStackDump(llvm::raw_ostream& output) const -> void;
@@ -146,6 +141,8 @@ class Context {
 
   auto vtable_stack() -> InstBlockStack& { return vtable_stack_; }
 
+  auto exports() -> llvm::SmallVector<SemIR::InstId>& { return exports_; }
+
   auto check_ir_map() -> llvm::MutableArrayRef<SemIR::ImportIRId> {
     return check_ir_map_;
   }

toolchain/check/decl_name_stack.cpp

@@ -158,7 +158,7 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id,
         // scope. Otherwise, it's in some other entity, such as a class.
         if (access_kind == SemIR::AccessKind::Public &&
             name_context.initial_scope_index == ScopeIndex::Package) {
-          context_->AddExport(target_id);
+          context_->exports().push_back(target_id);
         }
 
         name_scope.AddRequired({.name_id = name_context.name_id,

toolchain/check/handle_export.cpp

@@ -75,7 +75,7 @@ auto HandleParseNode(Context& context, Parse::ExportDeclId node_id) -> bool {
                                  {.type_id = import_ref->type_id,
                                   .entity_name_id = import_ref->entity_name_id,
                                   .value_id = inst_id});
-  context.AddExport(export_id);
+  context.exports().push_back(export_id);
 
   // Replace the ImportRef in name lookup, both for the above duplicate
   // diagnostic and so that cross-package imports can find it easily.

toolchain/check/param_and_arg_refs_stack.h

@@ -69,7 +69,7 @@ class ParamAndArgRefsStack {
   }
 
   // Runs verification that the processing cleanly finished.
-  auto VerifyOnFinish() -> void { stack_.VerifyOnFinish(); }
+  auto VerifyOnFinish() const -> void { stack_.VerifyOnFinish(); }
 
   // Prints the stack for a stack dump.
   auto PrintForStackDump(int indent, llvm::raw_ostream& output) const -> void {

toolchain/check/scope_stack.cpp

@@ -9,7 +9,7 @@
 
 namespace Carbon::Check {
 
-auto ScopeStack::VerifyOnFinish() -> void {
+auto ScopeStack::VerifyOnFinish() const -> void {
   CARBON_CHECK(scope_stack_.empty(), "{0}", scope_stack_.size());
 }
 

toolchain/check/scope_stack.h

@@ -155,7 +155,7 @@ class ScopeStack {
   auto Restore(SuspendedScope scope) -> void;
 
   // Runs verification that the processing cleanly finished.
-  auto VerifyOnFinish() -> void;
+  auto VerifyOnFinish() const -> void;
 
   auto return_scope_stack() -> llvm::SmallVector<ReturnScope>& {
     return return_scope_stack_;