fix(#1928): banner reads planned requirements off the emission plan

docs/architecture/README.md

@@ -123,7 +123,9 @@ files at the root: an entry point lives inside the root it starts.
   construct the pipeline. Each is entered through its own `index.ts`: the command-line
   tool and the library are separate concerns and do not share a starting point.
 - **Instrumentation** -- `instrumentation/`. Records facts about the RUN, never about the
-  program: where an ADR's rule fired, which toolchain features a run required. Written
+  program: where an ADR's rule fired, which toolchain features a run's Render emitters
+  required (a 2.2 planned block's features are 2.2's decision, read off the plan, #1928;
+  that Render still authors the rest, against the Ownership table, is tracked by #1997). Written
   from any layer, read once by the host, delivered on the run's result. It authors no fact
   the program has, so it is not a state container, and it decides nothing, so it is not a
   pass.
@@ -141,7 +143,8 @@ files at the root: an entry point lives inside the root it starts.
 
   **"Read once at the end" is not the test -- "nothing branches on it" is**, and the
   distinction matters because the ledger IS read back. `CodeGenWalker.buildBanner` puts
-  ` * Requires: C11.` into the generated `.c`, and `captureEmissionFacts` carries
+  ` * Requires: C++20.` into a generated `.cpp` that uses a designated initializer
+  (`cpp-designated-initializer`, recorded where `CodeGenWalker` emits it), and `captureEmissionFacts` carries
   `takeDeferredSites(...)` into `IEmissionFacts`. Checked, because a rule stated loosely
   reads as violated: `EmissionPlan` branches on `facts.needsFloatStaticAssert` and
   `facts.needsIrqWrappers` -- both render state -- and the sites travel beside them as

scripts/__tests__/passes-hold-no-mutable-state.test.ts

@@ -100,8 +100,8 @@ const INSTRUMENTATION_ROOT = "src/instrumentation";
 const INSTRUMENTATION_HOLDERS = [
   "src/instrumentation/AdrProvenance.ts:58",
   "src/instrumentation/AdrProvenance.ts:59",
-  "src/instrumentation/ToolchainRequirements.ts:56",
-  "src/instrumentation/ToolchainRequirements.ts:67",
+  "src/instrumentation/ToolchainRequirements.ts:65",
+  "src/instrumentation/ToolchainRequirements.ts:78",
 ];
 
 /**

src/TRANSPILE/2-Plan/EmissionPlan.ts

@@ -47,6 +47,7 @@
 import type IEmissionFacts from "../../types/IEmissionFacts";
 import type IEmissionPlan from "../../types/IEmissionPlan";
 import type IPlannedBlock from "../../types/IPlannedBlock";
+import type IRecordedRequirement from "../../types/IRecordedRequirement";
 import type TRequirementKey from "../../types/TRequirementKey";
 import SYSTEM_INCLUDE_TARGETS from "./SYSTEM_INCLUDE_TARGETS";
 import CTypeIncludes from "./CTypeIncludes";
@@ -106,6 +107,39 @@ class EmissionPlan {
     return Object.freeze(plan);
   }
 
+  /**
+   * Every `IEmissionPlan` field that holds a planned block, nullable or
+   * optional. A block added to the plan and not listed here fails to compile,
+   * instead of rendering with its cost missing from the banner -- #1143's
+   * failure.
+   */
+  private static readonly PLANNED_BLOCKS: {
+    readonly [K in keyof IEmissionPlan as NonNullable<
+      IEmissionPlan[K]
+    > extends IPlannedBlock
+      ? K
+      : never]-?: true;
+  } = {
+    floatStaticAssert: true,
+    irqWrappers: true,
+  };
+
+  /**
+   * The requirements the plan's blocks carry, each with the sites that asked
+   * for its block. A view of decisions already in the plan, not a new one.
+   */
+  static requirements(plan: IEmissionPlan): readonly IRecordedRequirement[] {
+    const fields = Object.keys(
+      EmissionPlan.PLANNED_BLOCKS,
+    ) as (keyof typeof EmissionPlan.PLANNED_BLOCKS)[];
+    return fields.flatMap((field) => {
+      const block = plan[field] ?? null;
+      return block === null
+        ? []
+        : block.requirements.map((key) => ({ key, sites: block.sites }));
+    });
+  }
+
   /**
    * Which system headers the implementation file emits, already deduplicated
    * against what its own source includes.
@@ -151,7 +185,7 @@ class EmissionPlan {
     return {
       keyword: facts.cppMode ? "static_assert" : "_Static_assert",
       requirements: [facts.cppMode ? "float-assert-cpp11" : "float-assert-c11"],
-      sites: facts.floatAssertSites,
+      sites: [...facts.floatAssertSites],
     };
   }
 
@@ -163,7 +197,7 @@ class EmissionPlan {
     return {
       keyword: null,
       requirements: IRQ_WRAPPER_REQUIREMENTS,
-      sites: facts.irqWrapperSites,
+      sites: [...facts.irqWrapperSites],
     };
   }
 }

src/TRANSPILE/2-Plan/__tests__/EmissionPlan.test.ts

@@ -155,6 +155,58 @@ describe("EmissionPlan (2.2 Plan)", () => {
     });
   });
 
+  describe("requirements -- read off the blocks, never copied (#1928)", () => {
+    it("lists every planned block's keys with that block's sites", () => {
+      const site = { sourcePath: "a.cnx", line: 3 };
+      const plan = EmissionPlan.build(
+        facts({
+          needsFloatStaticAssert: true,
+          needsIrqWrappers: true,
+          floatAssertSites: [site],
+        }),
+      );
+
+      expect(EmissionPlan.requirements(plan)).toEqual([
+        { key: "float-assert-c11", sites: [site] },
+        { key: "critical-arm-gnu", sites: [] },
+        { key: "critical-arduino", sites: [] },
+        { key: "critical-avr-libc", sites: [] },
+        { key: "critical-cmsis-fallback", sites: [] },
+      ]);
+    });
+
+    // `takeDeferredSites` hands the facts `ToolchainRequirements.deferred`'s
+    // own arrays without copying them, so they can still grow after
+    // `build()`; a site noted after the decision must not join a block it did
+    // not cause.
+    it("keeps the sites as decided when the facts' lists grow later", () => {
+      const early = { sourcePath: "a.cnx", line: 3 };
+      const irqWrapperSites = [early];
+      const floatAssertSites = [early];
+      const plan = EmissionPlan.build(
+        facts({
+          needsIrqWrappers: true,
+          irqWrapperSites,
+          needsFloatStaticAssert: true,
+          floatAssertSites,
+        }),
+      );
+
+      irqWrapperSites.push({ sourcePath: "a.cnx", line: 9 });
+      floatAssertSites.push({ sourcePath: "a.cnx", line: 9 });
+
+      for (const entry of EmissionPlan.requirements(plan)) {
+        expect(entry.sites).toEqual([early]);
+      }
+    });
+
+    it("is empty when the plan has no block that costs anything", () => {
+      expect(EmissionPlan.requirements(EmissionPlan.build(NOTHING))).toEqual(
+        [],
+      );
+    });
+  });
+
   describe("ISR typedef", () => {
     it.each([
       [false, true],

src/TRANSPILE/3-Render/codegen/CodeGenerator.ts

@@ -74,6 +74,8 @@ import PassByValueAnalyzer from "../../2-Plan/PassByValueAnalyzer";
 // Issue #797: Centralized C-style name generation
 import type IRecordedRequirement from "../../../types/IRecordedRequirement";
 import ToolchainRequirements from "../../../instrumentation/ToolchainRequirements";
+import RequirementAggregator from "../../../utils/RequirementAggregator";
+import EmissionPlan from "../../2-Plan/EmissionPlan";
 import TranspileState from "../../TranspileState";
 import StructDefaultInitializer from "./helpers/StructDefaultInitializer";
 import StructDefault from "../../../utils/StructDefault";
@@ -232,14 +234,19 @@ export default class CodeGenerator implements IOrchestrator {
   }
 
   /**
-   * Issue #1143: Snapshot the toolchain requirements recorded during the last
-   * generate() call.
+   * Issue #1143: what the last generate() call's output requires.
    *
-   * Must be read before the next file's this.state.reset(), which clears the
-   * recording map.
+   * Two sources, each the owner of its facts: what Render recorded at the
+   * sites that emitted text, and what 2.2's planned blocks carry, read off the
+   * plan (#1928). The banner and the file result both read this one list.
+   *
+   * Must be read before the next file's this.state.reset(), which clears both.
    */
   getToolchainRequirements(): readonly IRecordedRequirement[] {
-    return ToolchainRequirements.collect();
+    return RequirementAggregator.union([
+      ToolchainRequirements.collect(),
+      EmissionPlan.requirements(this.state.emissionPlan()),
+    ]);
   }
 
   /**

src/TRANSPILE/CodeGenWalker.ts

@@ -1650,11 +1650,9 @@ class CodeGenWalker {
       this.captureEmissionFacts(sourceIncludeTargets),
     );
 
-    // The plan decided WHICH toolchain capabilities these helpers need and at
-    // which sites; registering them is part of that decision, not part of
-    // formatting. `addGeneratedHelpers` used to do it while pushing the text,
-    // which made the renderer a writer of state something downstream reads.
-    CodeGenWalker.registerPlannedToolchain(plan);
+    // #1928: the banner and the file result read the planned blocks'
+    // requirements off the plan itself, so nothing copies them anywhere.
+    this.host.state.emissionPlanOrNull = plan;
 
     // 2.3 Render: format the plan. These two decide nothing.
     this.addAutoIncludes(output, plan);
@@ -1887,9 +1885,6 @@ class CodeGenWalker {
     return match ? match[1] : null;
   }
 
-  /**
-   * Add generated helpers (static asserts, IRQ wrappers, typedefs, etc.).
-   */
   /**
    * Print the deferred blocks and helpers the plan decided on.
    *
@@ -1898,31 +1893,7 @@ class CodeGenWalker {
    * the two cannot disagree -- #1143 kept them in step by computing both from
    * one ternary at this site; the plan keeps them in step by making them two
    * fields of one record, and the ternary is gone from here.
-   *
-   * Requirements are still recorded into `CodeGenState` rather than read off
-   * the plan by the banner, because the banner also carries requirements this
-   * plan does not yet own (the mode baseline, C++ initializer forms, atomics).
-   * Recording a decision someone else made is transcription, not derivation.
    */
-  /**
-   * Register the toolchain capabilities the plan's helper blocks need.
-   *
-   * Separated from `addGeneratedHelpers` so that rendering a block and
-   * declaring what the block requires are not the same act: the plan already
-   * holds both the keys and the sites, and a renderer that writes them is a
-   * renderer making state visible downstream. Order is unchanged -- this runs
-   * immediately after the plan is built, and nothing between reads a toolchain
-   * requirement.
-   */
-  private static registerPlannedToolchain(plan: IEmissionPlan): void {
-    for (const block of [plan.floatStaticAssert, plan.irqWrappers]) {
-      if (block === null) continue;
-      for (const key of block.requirements) {
-        ToolchainRequirements.record(key, block.sites);
-      }
-    }
-  }
-
   private addGeneratedHelpers(output: string[], plan: IEmissionPlan): void {
     const floatAssert = plan.floatStaticAssert;
     if (floatAssert !== null) {

src/TRANSPILE/TranspileState.ts

@@ -20,6 +20,7 @@ import type ITypeBindingDeps from "../types/ITypeBindingDeps";
 import type IProgram from "../types/IProgram";
 import type ITypingContext from "../types/ITypingContext";
 import type IDeclarationPlan from "../types/IDeclarationPlan";
+import type IEmissionPlan from "../types/IEmissionPlan";
 import type IFunctionSignature from "../types/IFunctionSignature";
 import invariant from "../utils/invariant";
 import emittedLocalName from "../utils/emittedLocalName";
@@ -140,12 +141,12 @@ class TranspileState {
    */
   requireInclude(header: TIncludeHeader, line: number | null = null): void {
     // Issue #1143: three of these "headers" are really deferred code-emission
-    // requests. Record where they were asked for, so the emitter that finally
-    // produces the block can attribute its requirement to a .cnx line. No
+    // requests. Record where they were asked for, so the 2.2 plan block that
+    // decides the emission carries them as its requirements' sites (#1928). No
     // requirement is recorded here -- the code does not exist yet, and
     // recording a requirement for text that may never be emitted is exactly
     // the mistake that made #1141's guard fire on files without the construct.
-    // Only the two headers that have a claiming emitter. "isr" was noted here
+    // Only the two headers whose sites a plan block carries. "isr" was noted here
     // and never read: ToolchainRequirements.takeDeferredSites is called for
     // float_static_assert and irq_wrappers alone, and the ISR typedef carries
     // no requirement. Keeping
@@ -309,6 +310,14 @@ class TranspileState {
    * silently-wrong default.
    */
   declarationPlanOrNull: IDeclarationPlan | null = null;
+  /**
+   * 2.2 Plan's emission decisions for the file being generated.
+   *
+   * Held for the rest of the file's turn so the requirements its planned
+   * blocks carry are read off the plan, never copied into
+   * `ToolchainRequirements` (#1928). Null until the declarations have rendered.
+   */
+  emissionPlanOrNull: IEmissionPlan | null = null;
   /** ADR-013: Track function parameter const-ness for call-site validation */
   functionSignatures: Map<string, IFunctionSignature> = new Map();
   /** Callback field types: "Struct.field" -> callbackTypeName */
@@ -363,6 +372,21 @@ class TranspileState {
     );
     return plan;
   }
+  /**
+   * 2.2 Plan's emission decisions, asserted present.
+   *
+   * Same reason as `declarationPlan()`: answering "no planned block costs
+   * anything" before the plan exists would drop the interrupt-masking and C11
+   * lines from the banner without failing.
+   */
+  emissionPlan(): IEmissionPlan {
+    const plan = this.emissionPlanOrNull;
+    invariant(
+      plan !== null,
+      "2.2 Plan builds the emission plan before its requirements are read",
+    );
+    return plan;
+  }
   /**
    * Compute unmodified parameters for all functions on-demand.
    * Returns a map of function name -> Set of parameter names NOT modified.
@@ -1219,6 +1243,7 @@ class TranspileState {
     this.callbackTypeReferences = new Set();
     this.publicCallbackTypeReferences = new Set();
     this.declarationPlanOrNull = null;
+    this.emissionPlanOrNull = null;
     this.usedClampOps = new Set();
     this.usedSafeDivOps = new Set();
     this.usedCastHelpers = new Set();

src/TRANSPILE/__tests__/CodeGenWalker.test.ts

@@ -17715,3 +17715,11 @@ describe("CodeGenWalker", () => {
     });
   });
 });
+
+describe("CodeGenerator.getToolchainRequirements (#1928)", () => {
+  it("refuses to answer before 2.2 has built the emission plan", () => {
+    expect(() => new CodeGenerator().getToolchainRequirements()).toThrow(
+      "2.2 Plan builds the emission plan before its requirements are read",
+    );
+  });
+});

src/TRANSPILE/__tests__/TranspileState.includes.test.ts

@@ -5,7 +5,7 @@
  * `CodeGenState.test.ts` while the members were statics; the behavior they
  * pin is unchanged, and the negative controls came with them -- the include
  * funnel must raise ONE flag rather than blanket them, and only the two
- * headers with a claiming emitter record a deferred site.
+ * headers whose sites a plan block carries record a deferred site.
  */
 import { describe, it, expect, beforeEach } from "vitest";
 import TranspileState from "../TranspileState";
@@ -49,8 +49,8 @@ describe("TranspileState", () => {
       expect(state.needsIrqWrappers).toBe(false);
     });
 
-    // #1143: only the two headers with a claiming emitter are recorded as
-    // deferred sites. "isr" is deliberately NOT one --
+    // #1143: only the two headers whose sites a plan block carries are
+    // recorded as deferred sites. "isr" is deliberately NOT one --
     // ToolchainRequirements.takeDeferredSites is called for
     // float_static_assert and irq_wrappers alone.
     //

src/cli/Transpiler.ts

@@ -1018,17 +1018,19 @@ class Transpiler {
         this.headerEmissionFactsByPath.set(sourcePath, headerFacts);
       }
 
-      // Issue #1143: read after header-facts CAPTURE, and before the next
-      // file's TranspileState.reset() clears the recording map. This covers a
-      // requirement that capturing a header's facts triggers (e.g. through
-      // HeaderEmissionCapture.convertToHeaderSymbols) -- it does NOT cover one the RENDER might
-      // trigger, since #1323 moved rendering to Stage 5.5, after every file's
-      // requirements have already been read here and reset() has run N times.
-      // Currently unreachable rather than wrong: TranspileState.requireToolchain
-      // has no caller under output/headers/, so no render path records one --
-      // but this read does not guarantee that stays true, and #1143 is
-      // precisely the bug class where an ordering assumption like that broke
-      // under a later refactor.
+      // Issue #1143: the file's one requirement list -- Render's records plus
+      // what the emission plan's blocks carry (#1928). Read after header-facts
+      // CAPTURE, and before the next file's TranspileState.reset() clears both.
+      // This covers a requirement that capturing a header's facts triggers
+      // (e.g. through HeaderEmissionCapture.convertToHeaderSymbols) -- it does
+      // NOT cover one the header RENDER might trigger, since #1323 moved
+      // rendering to Stage 5.5, after every file's requirements have already
+      // been read here and reset() has run N times. Currently unreachable
+      // rather than wrong: nothing under TRANSPILE/3-Render/headers/ calls
+      // ToolchainRequirements, so no header render records one -- but this
+      // read does not guarantee that stays true, and #1143 is precisely the
+      // bug class where an ordering assumption like that broke under a later
+      // refactor.
       const requirements = this.codeGenerator.getToolchainRequirements();
 
       return this.buildSuccessResult(