Skip to content

Make the formatter's meaning guard honest, and fix the two defects it was hiding - #222

Open
MelbourneDeveloper wants to merge 3 commits into
mainfrom
fmt-guard-and-bench-refresh
Open

Make the formatter's meaning guard honest, and fix the two defects it was hiding#222
MelbourneDeveloper wants to merge 3 commits into
mainfrom
fmt-guard-and-bench-refresh

Conversation

@MelbourneDeveloper

Copy link
Copy Markdown
Collaborator

osprey fmt was a silent no-op on 58 of the corpus's ~250 files, and nothing could notice: when the meaning-preservation guard rejected the formatter's own output, format_source returned the input verbatim, which is indistinguishable from "this file was already formatted". crates/osprey-fmt/tests/corpus.rs asserted idempotency against that, so the assertion was partly self-fulfilling.

A rejection is now the error osprey_fmt::DECLINED, and the corpus test collects every failing file rather than stopping at the first. Turning it on exposed two separate defects.

The guard compared source positions. Program's derived PartialEq includes Position, so any reindentation that moved a positioned node failed the guard that exists to permit it. 56 of the 58 diverged on nothing but line/column. The guard now compares osprey_ast::canonical::without_positions, one shared position-blind rendering that also replaced the two hand-copied scrub_positions helpers in module_equiv.rs and cross_flavor_equiv.rs. Being exact where those were sloppy, it immediately caught an asymmetry they had masked: Default records a span on a nested fn TypeExpr in a signature and ML does not.

The scanner reindented the inside of multi-line string literals. scan.rs scanned line by line and lost the "still inside a string" fact at the newline, so continuation lines were treated as code — indentation rewritten, interior double spaces collapsed. That changes what the program prints; the only thing that ever stopped it reaching disk was the silent bail. scan_source now merges the physical lines a literal spans into one logical line, so the literal moves as a unit and only the line that opens it is indented.

Extension stdin hang. runCompiler never closed the child's stdin, so a program reading input blocked until the run was cancelled. Fixed in source against the test that pins it.

Also: the positional-table scoping guard gained two mutation-proven tests; make bench was re-run across 22 cases × 11 languages and website/src/benchmarks.md rewritten to what the fresh data supports (three claims on it were false — Osprey wins no CPU or memory row, at 11.59× Rust and 13.14× C; ARC is above C in 21 of 22 cases, median 1.06×). Plan 0019 is retired and deleted.

Verification

make ci green end-to-end (deslop 0.27.0 installed locally to run the duplication gate rather than skip it), plus every other job CI runs: the full rust job, the wasm job (126 corpus programs vs native goldens), website E2E (93), bank e2e (17), extension tests (269), and the webcompiler API test.

Two gaps I could not close on this host, both stated rather than skipped:

  • Windows Core Build & Smoke Testwindows-latest only; the closest local equivalent is the three-backend corpus, which passes.
  • Web Compiler Docker test — Docker Desktop is not installed here. I ran webcompiler/test.sh against the server started directly under Node, which passed; the container build and its memory limits are untested locally.

Defects found and deliberately left open

  • let r = xs [0] silently parses as two statements instead of an index — one space changes the program's meaning with no diagnostic.
  • ackermann on wasm32 prints nothing where native prints 8189 (excluded from benchmark timing, so published numbers are unaffected).

Both are recorded in the retired plan row in docs/plans/README.md.

`runCompiler` spawned every child with an open stdin pipe and never ended it,
so a program that reads input — `input()`, or anything waiting on EOF — blocked
forever on a pipe nobody writes to, and the run only ended when the caller
cancelled it. The child's stdin is now closed immediately after spawn, which is
what the `runCompiler closes child stdin so input reaches EOF` test asserts.

`tests/regressions/basics/files/test_output.txt` and `test_stale_reason.txt`
are written by `file_io_json_workflow.test.osp`, not inputs to it. .gitignore
already says why a committed copy is wrong — "a stale copy is a test input
nobody wrote on purpose" — but only anchored the repo-root paths. Both are
removed and their in-corpus paths ignored, fully anchored so no bare name can
hide a real fixture elsewhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant