Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions docs/architecture/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ There are no bare files at either level: an entry point lives inside the root it
transpiler, so nothing in `cnx/`, its tests included, imports them. 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'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
program: where an ADR's rule fired, and where each toolchain requirement was asked for
(2.2 decides every requirement key, read off the plan, #1928 and #1997; Render reports
only which forms it emitted, and the sites travel beside the plan's keys). 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.
Expand All @@ -148,15 +148,15 @@ There are no bare files at either level: an entry point lives inside the root it
**"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: C++20.` into a generated `.cpp` that uses a designated initializer
(`cpp-designated-initializer`, recorded where `CodeGenWalker` emits it), and `captureEmissionFacts` carries
(`cpp-designated-initializer`, decided by 2.2 from the form Render reports, #1997), 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
reads as violated: `EmissionPlan` branches on render state -- the `needs*` and
`emitted*` facts and `debugMode`, never a site -- and the sites travel beside them as
attribution that no decision consults. So no generated CODE differs because of
instrumentation; a comment and a diagnostic's file:line do.

`scripts/__tests__/passes-hold-no-mutable-state.test.ts` scans this root **and** the
layers, with its four holders listed by name. It used not to scan here at all, which
layers, with its three holders listed by name. It used not to scan here at all, which
made the exemption geographic: the four mutable statics #1452 relocated into this root
left the guard unable to fail on the state that card was about. A list is a thing a
reviewer can disagree with; a directory boundary is not.
Expand Down
5 changes: 2 additions & 3 deletions scripts/__tests__/passes-hold-no-mutable-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..");
const PASS_ROOTS = ["src/cnx/PARSE", "src/cnx/TRANSPILE", "src/cnx/WRITE"];

/**
* Scanned alongside the pass roots, with its four holders listed below.
* Scanned alongside the pass roots, with its three holders listed below.
*
* The `instrumentation/` root did not exist when this file was written, and #1452
* created it -- so the four mutable statics this card RELOCATED there sat
Expand Down Expand Up @@ -100,8 +100,7 @@ const INSTRUMENTATION_ROOT = "src/cnx/instrumentation";
const INSTRUMENTATION_HOLDERS = [
"src/cnx/instrumentation/AdrProvenance.ts:58",
"src/cnx/instrumentation/AdrProvenance.ts:59",
"src/cnx/instrumentation/ToolchainRequirements.ts:65",
"src/cnx/instrumentation/ToolchainRequirements.ts:78",
"src/cnx/instrumentation/ToolchainRequirements.ts:49",
];

/**
Expand Down
25 changes: 9 additions & 16 deletions src/cli/Transpiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1018,19 +1018,12 @@ class Transpiler {
this.headerEmissionFactsByPath.set(sourcePath, headerFacts);
}

// 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.
// Issue #1143: the file's one requirement list, a view of the emission
// plan, which decides every key (#1928, #1997). EmissionPlan.build froze
// that plan inside generate(), so which line reads it here no longer
// matters: the boundary is the plan build. A form emitted after it would
// lose its cost from the banner and from this result alike, so
// TranspileState's note* methods refuse such a write instead.
const requirements = this.codeGenerator.getToolchainRequirements();

return this.buildSuccessResult(
Expand Down Expand Up @@ -1133,9 +1126,9 @@ class Transpiler {
//
// The comment here used to claim it stopped a run that plans nothing from
// reporting the previous run's cost. It cannot: every reader runs after
// `generate()`'s own `reset()` -- `collect()` from `buildBanner` inside
// `generate()` and from `_transpileFile` immediately after it,
// `takeDeferredSites()` from inside `generate()` -- so a run that plans no
// `generate()`'s own `reset()` -- `takeDeferredSites()` from inside
// `generate()`, whose sites the plan carries to `buildBanner` and to
// `_transpileFile` immediately after it -- so a run that plans no
// file never reads the ledger at all. Verified by deleting this line: 7438
// unit tests and 1263 fixtures stay green, which is why no regression test
// could be written for it.
Expand Down
35 changes: 35 additions & 0 deletions src/cli/__tests__/Transpiler.requirementSites.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* #1997: the atomic requirement's site travels from where Render notes the
* LDREX/STREX loop to the result through `ToolchainRequirements`. No fixture
* can see it -- the banner prints no sites -- so this is the one check that
* fails if the note and the take stop meeting.
*/
import { describe, it, expect } from "vitest";
import Transpiler from "../Transpiler";
import MockFileSystem from "./MockFileSystem";

describe("Transpiler -- requirement sites (#1997)", () => {
it("attributes atomic-ldrex-cmsis to the file that emitted the loop", async () => {
const fs = new MockFileSystem();
fs.addFile(
"/project/src/main.cnx",
"atomic u32 n <- 0;\nvoid bump() { n +<- 1; }\n",
);
const transpiler = new Transpiler(
{
input: "/project/src/main.cnx",
outDir: "/project/build",
noCache: true,
target: "teensy41",
},
fs,
);

const result = await transpiler.transpile({ kind: "files" });

expect(result.requirements).toContainEqual({
key: "atomic-ldrex-cmsis",
sites: [{ sourcePath: "/project/src/main.cnx", line: null }],
});
});
});
28 changes: 27 additions & 1 deletion src/cli/__tests__/cnx/TRANSPILE/CodeGenWalker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,33 @@ describe("CodeGenWalker", () => {

// Debug mode affects helper generation
expect(code).toContain("cnx_clamp_add_u8");
// #1997: 2.2's one panic decision drives both the helper Render emits
// and the cost the banner reports.
expect(code).toContain("abort();");
expect(code).toContain("hosted libc");
});

it("clamps without a hosted-libc cost outside debug mode", () => {
const source = `
clamp u8 val;
void main() {
val +<- 1;
}
`;
const { tree, program } = CNextSourceParser.parse(source);
const generator = new CodeGenWalker(new CodeGenerator());
const symbols = TSymbolInfoAdapter.convert(declareAndResolve(tree));

const code = generateWithProgram(generator, tree, program, {
cppMode: false,
symbolInfo: symbols,
sourcePath: "test.cnx",
debugMode: false,
});

expect(code).toContain("cnx_clamp_add_u8");
expect(code).not.toContain("abort();");
expect(code).not.toContain("hosted libc");
});

it("should generate C++ output when cppMode is enabled", () => {
Expand Down Expand Up @@ -304,7 +331,6 @@ describe("CodeGenWalker", () => {
expect(input.knownStructs).toBeInstanceOf(Set);
expect(input.callbackTypes).toBeInstanceOf(Map);
expect(input.callbackFieldTypes).toBeInstanceOf(Map);
expect(typeof input.debugMode).toBe("boolean");
});
});

Expand Down
75 changes: 67 additions & 8 deletions src/cnx/TRANSPILE/2-Plan/EmissionPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ class EmissionPlan {
facts.needsISR &&
!HeaderOwnership.ownsDeclarations(facts.selfIncludeAdded),
clampOps: [...facts.clampOps],
panicOverflowHelpers: EmissionPlan.decidePanicOverflowHelpers(facts),
safeDivOps: [...facts.safeDivOps],
castHelpers: [...facts.castHelpers].sort((a, b) => a.localeCompare(b)),
formRequirements: EmissionPlan.decideFormRequirements(facts),
};

return Object.freeze(plan);
Expand All @@ -122,22 +124,79 @@ class EmissionPlan {
} = {
floatStaticAssert: true,
irqWrappers: true,
panicOverflowHelpers: 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.
* Every requirement the plan decided: the forms' and the blocks', each with
* the sites that asked for it. A view of decisions already in the plan, not
* a new one. Copied on the way out: `Object.freeze` is shallow, the banner
* and `ITranspilerResult` both read this list, and the IRQ block's four keys
* share one `sites` array.
*/
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 }));
});
return [
...plan.formRequirements.map((entry) => ({
key: entry.key,
sites: [...entry.sites],
})),
...fields.flatMap((field) => {
const block = plan[field] ?? null;
return block === null
? []
: block.requirements.map((key) => ({ key, sites: [...block.sites] }));
}),
];
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
}

/**
* What the forms Render emitted cost, beside the planned blocks (#1997).
*
* Render reports the form -- it wrote a designated initializer -- and this
* decides the cost: C99 and free in C, C++20 in C++. Each branch reads an
* `emitted*` fact; the LDREX sites ride along as attribution and decide
* nothing (README §1: no decision consults instrumentation).
*/
private static decideFormRequirements(
facts: IEmissionFacts,
): readonly IRecordedRequirement[] {
const decided: IRecordedRequirement[] = [
{ key: facts.cppMode ? "baseline-cpp" : "baseline-c", sites: [] },
];
if (facts.cppMode && facts.emittedDesignatedInitializer) {
decided.push({ key: "cpp-designated-initializer", sites: [] });
}
if (facts.cppMode && facts.emittedCompoundLiteral) {
decided.push({ key: "cpp-compound-literal", sites: [] });
}
if (facts.emittedLdrexStrexLoop) {
decided.push({
key: "atomic-ldrex-cmsis",
sites: [...facts.ldrexStrexSites],
});
}
return decided;
}

/**
* Whether the overflow helpers panic, with what that costs (#1997).
*
* Render reads this block to choose the panic templates, so the hosted-libc
* key and the `<stdio.h>` it pays for are one decision rather than two
* readings of `debugMode` that happen to agree.
*/
private static decidePanicOverflowHelpers(
facts: IEmissionFacts,
): IPlannedBlock | null {
if (!facts.debugMode || facts.clampOps.size === 0) return null;
return {
keyword: null,
requirements: ["overflow-panic-hosted-libc"],
sites: [],
};
}

/**
Expand Down
Loading
Loading