Fix compiler, LSP and documentation regressions - #238
Merged
Merged
Conversation
A function value that returned a tagged collection lost its element type.
The lowered `FnSig` carried parameter slots, a return `LType`, a `Result`
inner type and a `FiberSig` — but no return OWNER. A named function
recovers one through `fn_ret_owner`; a call through a closure cell has only
the signature, so `|| => [0.5, 1.25]` handed back an untagged `i8*` and
`listGet` on it met an `i64` payload against a `double` default:
codegen: invalid program: match arms disagree on type: `i64` and `double`
Truthful about the representation, but naming no user construct, so there
was nothing in the message to act on. The same loss reached a captured
value, a lambda passed as a function-value parameter, and a generic HOF.
The common cause is that the tag was reconstructed at each consumer instead
of travelling with the value: `stmt.rs` rebuilt it via `elem_tag`,
`ctor_field_handle` via `handle_elem_owner`, and the return route via
`fn_ret_owner`, which answers `None` for a handle. Every route with no
declaration to consult bound `None`.
`FnSig` gains a fifth slot, the return owner, filled by `fn_value_sig` and
applied in `closure::returned`; `monofn`'s `Abi` return slot carries it too.
`FiberSig` gains `elem_owner`/`elem_payload_owner` so `restore` is the one
place all four pieces land, which deletes the per-consumer reconstruction in
`cast.rs`, `stmt.rs` and `ctor_field_handle`.
One trap on the way: tagging the return `List#double` turned a returned list
LITERAL into a segfault rather than a rejection, because a literal leaves the
body as a flat `{length, data}` header. `ret_as_sig` now converts at the
escape through `listlit::escaping`, the seam `fit_lambda_return` already uses.
Tests, both flavors sharing one golden, green under default/gc/arc:
- `a function value returning a collection keeps its element type`
(functional_showcase) — verified to FAIL on the pre-fix compiler
- `a channel reached through a closure keeps its element descriptor`
(nested_generic_collections_fibers) — FIFO order, outer and inner lengths,
exact edge values
- `a block-bodied kernel keeps its internal let bindings` (gpu/buffers) —
byte-identical under OSPREY_GPU_KERNELS=extract and =inline
Plans:
- 0004 retired. Its last defect, `listGet` over a `List<string>`, no longer
reproduces in either face; re-measured across string/float/bool/int/record
elements and `mapValues`, and pinned by map_basics, string_edge_cases and
collect_all_errors. Its two remaining items were standing constraints
enforced by a build test and CLAUDE.md, not plan-tracked work.
- 0002: two items were stale — both verified against the parent commit — and
are now pinned by tests. The annotations they made load-bearing on
stress/mlkernels are dropped, byte-exact. Only the returned still-generic
lambda remains, and it fails with an actionable diagnostic.
Refs #227, #211
`fn pick() = |x| => x` followed by `let f = pick()` was rejected outright:
codegen: unsupported construct: a closure value with a still-generic type
One closure cell has one ABI and the binding may be used at several. The
lambda's own source position serves every instantiation, so its recorded type
stays generic and `lambda_value` had nothing concrete to emit against.
The fix is not a per-instantiation cell but the beta-reduction that already
makes a directly bound `let f = |x| => x` work at two instantiations:
`stmt::generic_returned_lambda` records the returned lambda for inline
application, so each call site specialises it at that site's real types. One
binding now serves `f(7)`, `f("os")`, `f(2.5)` and `f(true)`.
A lambda a generic function returns may also close over that call's parameters
(`fn constly(v) = |x| => v`). Two ways to get that wrong: re-evaluating the
argument expression per instantiation would duplicate its effects, and
resolving the callee's parameter NAMES at the inline site would read them from
whatever scope the body landed in — a silently wrong answer. So the arguments
are evaluated ONCE, at the binding, and carried as values in
`Codegen::lambda_prefix`, which `expr::apply_bound_lambda` prepends at each
call site. Those are SSA registers of the function being emitted, so the map is
saved and restored with the rest of the function frame and cleared per
function.
Three conditions keep the transform sound rather than merely permissive:
1. The callee's body must be syntactically the lambda, so calling it performs
no work of its own that inlining could duplicate or drop.
2. The argument count must match the callee's parameters.
3. A binding whose lambda READS those parameters is confined to the function
that evaluated the call, because the prefix values are that function's
registers. A file-scope one read from elsewhere keeps the ordinary path.
Condition 3 is not hypothetical. Saying a binding "materialises no value"
(`binds_no_value`) suppresses its module storage, so a capture-free file-scope
binding must be seeded into `file_lambdas` BEFORE any body is emitted — the
first version of this change was not, and a reader in another function emitted
`call @f` to a symbol no definition produces. That fails at LINK time, where
neither the type gate nor codegen would have caught it. Pinned by the
`sharedIdentity` assertions in the corpus test, which fail without the seeding.
The remaining refusal is permanent, not a gap: a still-generic lambda used as a
bare value has no call site to specialise against, so `print("${mk(1)}")` still
fails with the same truthful message. Recorded in spec 0004
[TYPE-GENERICS-FN] and pinned by
`a_still_generic_lambda_with_no_slot_at_all_is_rejected`.
Two tests pinned the limitation this removes, and are retargeted rather than
deleted:
- `generic_function_value_without_a_slot_is_rejected` becomes
`a_generic_functions_returned_lambda_is_inlined_per_call_site`, with its
rejection half kept as the boundary test above.
- `GENERIC_AS_VALUE` (cli_e2e) was a vehicle for reaching codegen's `Err` arms;
it now uses the shape that still fails, so `llvm_reports_a_codegen_error` and
`run_reports_a_codegen_error` keep testing what they were written to test.
New corpus coverage: `a generic function's returned lambda specialises per call
site` (functional_showcase, both flavors, one golden), whose `runs == 1`
assertion counts a real performed effect to prove the producing call is
evaluated once across three instantiations. Verified to FAIL pre-fix.
Plans 0002 and 0004 are retired: every checklist item is complete and named
tests prove it, so both files are deleted and their README rows struck. The one
permanent restriction moved to spec 0004 first. Fixed four dangling links to
the deleted plans, and a blog post whose plan link would 404 and whose "the
plan document tracks each of these" claim was no longer true.
124 codegen unit tests, the 116-fixture must-reject corpus and clippy at
pedantic are green; both flavors are byte-exact against one golden under
default, gc and arc.
The corpus ran SEVEN times per PR. `make test` ran it as TAP, again for the goldens, and again under GC and ARC, while a separate `rust` job ran all three backends a second time. `crates/run_test_corpus.sh` already makes both observations in ONE pass -- the in-language assertions AND the byte-exact golden -- so one invocation per memory backend is the entire matrix, and the TAP pass was a strictly weaker repeat of work the golden pass had done. The Rust suite ran twice for the same reason: once plain in `rust`, then re-executed under llvm-cov in `ci`. An instrumented run asserts everything an uninstrumented one does, so only the instrumented one survives. Four stages now. `build` performs every static check and produces every artifact -- C runtime archives, wasm archive, release binary, extension bundle, VSIX. Those upload once. Every suite then runs in parallel against them, collecting coverage where the language has a coverage story, and `coverage` enforces the per-project thresholds against reports the suites already produced rather than re-running a suite to measure it. setup-osprey-compiler gains `build: "false"` so a test job installs the toolchain without compiling the compiler a second time. The corpus jobs are three explicit jobs, not a matrix: the gate is verified from inside the tree by verify-branch-protection.mjs, which matches a required context against a literal job `name:`, and an interpolated matrix name matches nothing -- which that script rightly reports as a check that can never report. EXPECTED_CONTEXTS is updated to the new job names and now also requires "Detect changed areas (Windows)". That job was never required, yet `windows-core` skips on its output and a skipped check reports as PASSING -- so a failure there silently removed the entire Windows gate instead of blocking on it. Ruleset 6154907 must be updated to the same list. Until it is, the `changes` job fails, which is the intended loud behaviour: a gate that drifts silently is what that script exists to catch.
A job that `needs:` a failed job is SKIPPED, so making every suite depend on `changes` meant one failure in the gate-verification step left the PR with no test signal whatsoever -- twelve skipped checks and nothing actually run. The website suite is the one that is meaningful for a website-only PR and reads none of the change-detection outputs, so it depends on nothing, exactly as it did before the pipeline was staged.
Both comments pointed readers at tests/IOS_UNPORTABLE.txt, which has never existed — the pinned rejection set is tests/MOBILE_UNPORTABLE.txt, shared with Android because the two targets compile through one C ABI implementation and so have one set of holes. A comment that explains a gate and names a file that is not there sends the next reader looking for a manifest they cannot find. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two open checklist items read as though iOS were unverified on hardware. The signed application had already installed, launched and passed its full smoke on an iPhone 16; what is outstanding is re-running that smoke now that Markdown rendering has landed. Record that no hosted runner can ever close the item, and what stands in for it on every PR: the macos-15 job runs the whole corpus through the ios-sim C ABI and both application smokes on an iPhone 16 Pro simulator, and links the device archive's C host. The runner is ARM64, so the simulator executes the phone's instruction set -- a device adds signing, provisioning and device-only OS behaviour, not different code generation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release pipeline had five ways to publish nothing and still report success, and v0.17.0 went out through all of them intact. - The win32 build and VSIX legs were `continue-on-error`, so a Windows binary that never built showed as a warning inside a green run. - The runtime archives were staged with `cp ... || true`, so a build producing none would package, publish and reach Homebrew as a compiler that can link nothing. - The publish tokens were probed, and a missing one skipped its channel with a warning: a release that reached neither Homebrew, Scoop nor Open VSX passed while `brew install` kept serving the previous version. - `git commit ... || exit 0` in the tap jobs swallowed every commit failure along with the nothing-to-commit case, and skipped the push. - The web compiler deploy was non-fatal by design, which is how the live playground served a stale build for weeks behind green releases. All five are now failures. `fail-fast: false` stays on both matrices, so a broken leg still lets its siblings finish and one run shows every platform's result — it just ends red rather than cancelling anything. That leaves the failure mode none of it addresses: GitHub scores a *skipped* job as green, so a wrong `if:` or an unset scope output publishes nothing and still succeeds. The new `release-complete` job runs `if: always()` after every other job and fails unless each channel this tag requires actually succeeded, driven by the same four `scope` outputs the jobs' own conditions use so the two cannot drift apart. Also removes the `|| true` hiding a failed `cargo install cargo-llvm-cov` behind a confusing error several steps later, and the one on the Android `sdkmanager` lookup, which is now a directory test with a real diagnostic. How the tests prove it works: `scripts/test-release-gate.py` extracts the backstop's shell out of release.yml — the real one, not a copy — and runs it against 13 fabricated outcomes: Homebrew silently skipped, a build leg failed, a job cancelled, a website-only tag whose site never deployed, a vsix-only tag whose Marketplace publish skipped, and a prerelease correctly leaving the live site alone. Six mutations of the gate were each confirmed to turn it red, including one that first slipped through and exposed a missing case: a cancelled job the tag does not require is caught only by the failure sweep, so that case now pins the sweep. `scripts/verify-release-gates.mjs` fails the PR that reintroduces any of this: `continue-on-error`, `|| true` or `|| exit 0` in a run step, a release job absent from `release-complete`'s needs, a renamed or deleted backstop, or a reviewed tolerance gone stale. Eight regressions were each confirmed caught, with the tree checksummed back to clean after every probe. The one surviving `|| true` — the first-release tag lookup — is in that script's reviewed list with its reason. Both run in `make lint` and in the already-required "Build, Format & Analyse" job, so no required-check list or ruleset changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # .github/workflows/ci.yml # crates/osprey-cli/tests/cli_e2e.rs # crates/osprey-codegen/src/aggregate.rs # crates/osprey-codegen/src/builder.rs # crates/osprey-codegen/src/closure.rs # crates/osprey-codegen/src/effects.rs # crates/osprey-codegen/src/expr.rs # crates/osprey-codegen/src/lower.rs # crates/osprey-codegen/src/monofn.rs # crates/osprey-codegen/src/stmt.rs # docs/plans/0015-generics-and-variance.md # docs/specs/0004-TypeSystem.md
# Conflicts: # docs/plans/0029-ios-c-abi.md # docs/plans/0030-reactive-mobile-apps.md
# Conflicts: # Makefile
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
2 participants
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.
TL;DR
Fix compiler and tooling regressions so rejected programs report truthful errors, editor diagnostics survive incomplete sibling edits, and documentation examples and generated HTML are checked before shipping.
Details
Resultmatch arms, reject unsupported aggregate interpolation while retaining Fiber interpolation, and preserve operator positions through inference, escaped interpolation and project assembly.//!diagnostics consistent, and eliminate unused-binding warnings in all shipped examples and test programs.docs/fixes-branch-review.md.The large modules example still needs about six seconds for its initial analysis. Additional open files now share that analysis instead of repeating it; synthetic 10-open-file refresh time fell from 70.7 ms to 12.9 ms.
How Do The Automated Tests Prove It Works?