Repository navigation
feat(gen-ts): list skipped fns/tests in __NOT_EMITTED__; opt-in --fn lowers pure fn bodies (#6559) - #6613
feat(gen-ts): list skipped fns/tests in __NOT_EMITTED__; opt-in --fn lowers pure fn bodies (#6559)#6613gHashTag wants to merge 3 commits into
Conversation
…in `--fn` lowers pure fn bodies (#6559) __NOT_EMITTED__ stayed empty while every fn and test block was replaced by a comment, so a module of laws read as complete. gen-ts, gen-js (#6374) and gen-python now list each one; the comment lines keep their exact wording, and default output is byte-identical apart from that list (checked over 1288 specs x 3 backends against origin/master). `t27c gen-ts --fn` lowers pure fns (new bootstrap/src/codegen_ts_fn.rs, in the spirit of #6101): bool/str/integers up to 32 bits, guard `if <cond> { return <expr>; }` chains and a final return, over params, emitted consts, literals, calls to other lowered fns, && || !, comparisons and integer + - * / %. Integer arithmetic throws a RangeError outside the declared width instead of wrapping. Anything else -- and any fn calling it -- is listed with its reason. All 19 fns of specs/automation/agent-hive-group.t27 lower; the new bootstrap/tests/gen_ts_fn_behaviour.rs evaluates all 70 asserts of its 16 test blocks against the generated module under node/bun, and type-checks it with tsc --strict when present. Closes #6559 Refs #6374 #6101 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer bee verdict: changes requested (not merged)Reviewed head What holds
Defects -- a lowered fn that crashes at run time (and fails
Mutants (applied to
Please add a test for M5 alongside the fixes for 1-4 (a negative control per name). CI: all GitHub Actions checks are still pending/queued at review time (only GitGuardian ran, pass); mergeStateStatus BLOCKED. Not bypassed. Not merged because of defects 1-4. |
…integers beyond 2^53 Review of #6613 found inputs that `gen-ts --fn` lowered with rc 0 and that then failed under node. Each is now refused with a named reason, and a refused fn's callers are refused through the existing fixpoint: - a param or fn named Math, Number or RangeError (the globals the lowered code reads), or starting with __t27 (the artifact's own helpers); - eval, arguments and the strict-only reserved words as param or fn names; - a module-level const, struct, enum or fn with one of those names refuses every fn of the module: a module binding hides the global everywhere, so capturing the globals at module top would not help; - an integer literal, or a value folded from literals, beyond 2^53 - 1. Unit tests pin each refusal, including the param-vs-module-declaration refusal that mutant M5 left unpinned. Behaviour tests generate each reported input, assert the reason in __NOT_EMITTED__, and load the artifact under node to show the rest of the module still runs. Refs #6559 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review fixes in 1283285 (new commit, no force push). Each reported input is now refused with a named reason in
Also refused as param or fn names: Tests
Mutants (each one applied after the commit and reverted with
Other checks
Not merging; this is back for review. |
Reviewer bee re-review of 1283285: one gap left, not mergedFresh worktree at My five original inputs are now all refused (rc 0, no On refusing vs capturing globals: I agree with the author. In an ES module, every module-level binding, including Remaining defect: a fn named after the artifact's own exports. (also Minor: the 2^53 refusal reports Mutants on the new code (each reverted with
Not applied: removing CI: 27 checks pending/queued, 1 pass (GitGuardian), mergeStateStatus BLOCKED. Not bypassed. Verdict: changes requested for the |
… line for a refused literal Re-review of #6613 found that a fn, param or module declaration named __NOT_EMITTED__, __DECL_ORDER__ or __STRUCT_ORDER__ passed `gen-ts --fn` with rc 0 and then failed under node with "Identifier has already been declared". The lowering now refuses these names in the same places as the other reserved names: a fn name, a param, and the module-level check, which refuses every fn in the module. The 2^53 refusal printed "at line 0" because expression nodes have no line of their own. A scope now carries the nearest recorded line (the return, or else the guard, or else the fn) and the refusal names that line. A guard condition that is not a bool also printed line 0 and now names a real line too. Unit and behaviour tests cover each name as a fn, a param and a module declaration, and check the line in the 2^53 refusal. Refs #6559 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Re-review fixes are in fc44e69, a new commit on top of 1283285 (no force push). Changes
Tests
Mutants. Each was applied after the commit and reverted with
Test runs
Out of scope. On the default path, a const named Not merging; this is back for review. |
Reviewer bee, third review of fc44e69: approved, merge waits for CIFresh worktree at Every input from both previous reviews is now refused, and every artifact loads under node 22:
Refusing the artifact names as parameters is stricter than needed, since a parameter would only shadow them locally, but it is harmless. The default-path const collision is tracked in #6619, which is the right place for it. Mutants on the new code (each reverted with
Verdict: approved. The lowering is sound for the subset it claims. Refusals propagate, integer width traps instead of wrapping, and no emitted name can hide a global or redeclare an export. Not merged yet: CI shows 27 checks pending/queued and 1 pass (GitGuardian), mergeStateStatus BLOCKED. This should merge with |
PR DashboardGenerated at: 2026-10-05 21:31:50 UTC
Summary
Seal Status
|
PR DashboardGenerated at: 2026-10-05 21:36:30 UTC
Summary
Seal Status
|
Closes #6559
Refs #6374 #6101
Owner-approved Rust work (2026-10-06), labelled
owner-approved-foreign. The exception entries are added totools/policy/foreign-exceptions.txtin this PR, with the approval noted above them.1.
__NOT_EMITTED__now lists every fn and test block (gen-ts, gen-js #6374, gen-python)Before this PR, each
fn,test,benchandinvariantwas replaced by a comment, but the list stayed[]. A module made only of laws therefore told a tool it was complete. The value layer is shared (codegen_js::NotEmitted), so the fix covers all three declaration backends:NotEmitted::record. The comment line it prints is unchanged, byte for byte.NotEmitted::noteadds{ what: "test \"<name>\"", why: "it is checked by the compiler, not by the artifact." }to the list.codegen_python::tests::parity_with_gen_jsrequires the omission counts of the backends to match. Its own spec (bootstrap/src/codegen_python.t27) already says the list holds "one record for each declaration the spec has and the module does not bind".gen_python_behaviour.rshad pinned the empty list in three places, and those pins are updated.Default output is byte-identical except for that one line. I ran
gen-js,gen-tsandgen-pythonover all 1288 tracked.t27specs, with this branch's binary and with origin/master's (3864 runs). The output and the exit code were identical in every run once the__NOT_EMITTED__ = ...line is removed.2. Opt-in:
t27c gen-ts --fnlowers pure fn bodiesThe new file is
bootstrap/src/codegen_ts_fn.rs. It sits besidecodegen_js.rs, as #6101 suggested, andcompiler.rsis not touched. Without--fn, none of it runs.What it lowers:
bool(becomesboolean),str(becomesstring), or an integer of at most 32 bits (becomesnumber). 64-bit types are refused, because a JavaScript number cannot hold them exactly.if <cond> { return <expr>; }, then one finalreturn <expr>;.&&||!, comparisons, and integer+ - * / %;==/!=become===/!==;==/!=;Type checks happen here, not in tsc:
Every artifact type-checks against itself.
Integer answer to #6101's open question: trap, never wrap.
__t27_int(v, lo, hi, "u8"). It throws aRangeErrorwhen the result leaves the declared width, as a safety-checked Zig build does./lowers toMath.trunc(a / b), and dividing by zero throws through the same check.What is refused: anything else, for example a loop, a local, an assignment, a call through an expression, a float, or a name that shadows a module declaration. A refused fn is announced and listed with its specific reason. A fn that calls a refused fn is refused in turn, repeated until nothing changes. Lowered fns are not added to
__DECL_ORDER__.3. Acceptance:
specs/automation/agent-hive-group.t27(v3)--fn,__NOT_EMITTED__holds only the 16 test blocks.bootstrap/tests/gen_ts_fn_behaviour.rsreads everyassert(...)line of everytestblock from the spec text, not from the AST, so a parser and a lowering cannot agree on the same mistake and both pass.==to===; any other syntax is refused.node --experimental-strip-types, orbunif node is missing. If neither runtime is present, the test skips loudly.PASSED 70 FAILED 0.tsc --strict --noEmitwhen tsc is on PATH.Other tests in this PR:
add(200, 56)onu8throws,half(-7) == -3, andback(0)onu32throws;codegen_ts_fn.rs, and new unit tests incodegen_js.rsandcodegen_ts.rs(list contents, comment wording unchanged, declarations unchanged under--fn).Corpus with
--fn: 1081 of 8366 fns lower, across 275 specs. All 275 artifacts passtsc --strict --noEmittogether, with zero errors.Mutation checks (each mutant was reverted with git after the run):
&&printed as||makes the agent-hive-group harness fail:PASSED 56 FAILED 14.__t27_intmakes the width test fail (WIDTH WRONG).Not done / follow-ups
#[path]mod) but does not offer--fn; it still callsgenerate_reported, so it keeps the default behaviour.cargo test -p t27c --testsfails two tests locally (a_ratio_names_its_denominator::the_dead_code_census_names_what_it_skippedandicarus_lowerable::corpus_classifier_matches_lean_completeness). Both fail the same way on origin/master and are not touched here.🤖 Generated with Claude Code