fix(typecheck): resolve use before checking, under the backends contract - #3413
Merged
Conversation
…ntract Closes #3412 Every `gen-*` path splices imported declarations in before compiling. `typecheck` read the raw source. So a type arriving through an import read as undeclared, and the unknown-type check added one pass ago in #3409 warned about types the spec correctly imports -- my own defect, found by measuring my own output. Measured over all 651 specs: unknown type warnings 1283 -> 1045 all warnings 1775 -> 1570 exit-code changes 0 specs/base/ternary_add.t27 alone goes from 10 warnings to 0: it writes `use base::types;` and every type it names comes from there. Counted a second way before the repair -- asking whether each warned name appears as a declaration in the RESOLVED output -- 41 distinct names across 29 of the 169 flagged files were false, against 228 true. The naive repair is wrong, and the reason is already written down. run_gen carries the contract: Safety contract ... this may only ADD declarations, never break a spec. If the spliced source stops compiling, the original is used and the spec generates exactly what it generated before. Resolving without that fallback took specs/nn/hslm.t27 from exit 0 to `Expected RParen, got Eof at line 652:1`, while all four backends still compiled it -- because they fall back and this did not. The splice can produce source the parser rejects; that is a handled condition, not a verdict about the spec. With the contract mirrored, exactly 1 of 651 specs takes the fallback and says so on stderr rather than silently checking something other than what it claims to. Mutation-checked: disabling the resolve fails the new test with the measured number, "it warned 10 times". compiler.rs is untouched, so FROZEN_HASH is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gHashTag
enabled auto-merge (squash)
September 7, 2026 20:53
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3412
Every
gen-*path splices imported declarations in before compiling.typecheckread the raw source. So a type arriving through an import read as undeclared, and the unknown-type check I shipped one pass ago in #3409 warned about types the spec correctly imports.This is my own defect, found by measuring my own output rather than by anyone reporting it.
Measured, all 651 specs
unknown typewarningsspecs/base/ternary_add.t27alone goes from 10 warnings to 0: it writesuse base::types;and every type it names comes from there.Counted a second way before the repair, by asking whether each warned name appears as a declaration in the resolved output: 41 distinct names across 29 of the 169 flagged files were false, against 228 true. Precision was about 85% — not broken, but noise in a check whose only job is to be believed.
The naive repair is wrong, and the reason was already written down
run_gencarries the contract:Resolving without that fallback took
specs/nn/hslm.t27from exit 0 towhile all four backends still compiled it — because they fall back and my first version did not. The splice can produce source the parser rejects; that is a handled condition, not a verdict about the spec.
With the contract mirrored: exactly 1 of 651 specs takes the fallback, and it says so on stderr rather than silently checking something other than what it claims to.
Test
One new, seven green in the file. It runs the shipped binary on
specs/base/ternary_add.t27and asserts zero unknown-type warnings, skipping loudly if the spec is absent.Mutation-checked: disabling the resolve fails it with the measured number — "it warned 10 times".
Named, not done
Tritis declared by four specs aspub const Trit = enum(i8)and simply not imported by the files that use it —specs/ar/restraint.t27uses it with nousestatement at all. Adding one line there takes its emitted Rust from 30 rustc errors to 19 and bringspub enum Tritinto the output. That is a spec-side repair with a measured payoff and deserves its own change rather than riding along here.compiler.rsis untouched, soFROZEN_HASHis unchanged.