feat(lower): pass an Into<Path> native parameter as text and build the IoError from it #420

Merged

argus merged 5 commits from loop/419-into-path-natives into main 2026-10-23 11:36:59 +00:00

Conversation 9 Commits 5 Files changed 9 +431 -15

orion commented 2026-10-23 07:45:42 +00:00

Collaborator

Copy link

Closes #419

Task

Commit 1 of 2 for #68 (docs/design/PATH.md section 8, slice 2): the compiler learns the native side of the Into<Path> signature. No std/fs, std/io or std/process signature changes here, and nothing in runtime/ or seed/.

What

• ir/lower/native.trb: a native parameter of type Into<Path> makes the native need a wrapper. The runtime keeps receiving one torb_text: the wrapper converts the argument once (into(), then show() through its witness) and passes the / form.
• A .Fallible native builds its IoError from the converted argument: the Path where the failure's same-named field is declared Path, the text where it is declared String. Any other field type is reported as unsupported, with the field name.
• The checker needed no change: it already accepts a String, a literal and a Path for Into<Path>.
• docs/design/PATH.md: one sentence in the status header saying the native side is in. Status stays "partly implemented".

Why now

Seed rule: the compiler learns both forms first, std/fs migrates in a later step after the seed refresh. Nothing in std or the compiler uses the new form beyond the test fixture below.

Test

• compiler/tests/lower-natives.test.trb: lowering tests over a stand-in std/path and std/fs (wrapper converts once, runtime function takes a String, both failure shapes, the unsupported field message).
• tests/conformance/native-into-path.trb: interpreter against native over a String, a literal and a Path, for success and for an error message naming the path, with both an IoError and a Path-carrying error.
• std/test/src/path.trb is the temporary fixture (a native may only be declared in std). It reuses two existing manifest rows, so the manifest and the generated thunk table are untouched. It goes when std/fs takes Into<Path>.
• native-into-path.allocations is 299, the count tier B printed on linux x86_64 (run 1395: has no allocation budget, allocated 299 blocks; the only failing program of 309).

Closes #419 ## Task Commit 1 of 2 for #68 (`docs/design/PATH.md` section 8, slice 2): the compiler learns the native side of the `Into<Path>` signature. No `std/fs`, `std/io` or `std/process` signature changes here, and nothing in `runtime/` or `seed/`. ## What - `ir/lower/native.trb`: a `native` parameter of type `Into<Path>` makes the native need a wrapper. The runtime keeps receiving one `torb_text`: the wrapper converts the argument once (`into()`, then `show()` through its witness) and passes the `/` form. - A `.Fallible` native builds its `IoError` from the converted argument: the `Path` where the failure's same-named field is declared `Path`, the text where it is declared `String`. Any other field type is reported as unsupported, with the field name. - The checker needed no change: it already accepts a `String`, a literal and a `Path` for `Into<Path>`. - `docs/design/PATH.md`: one sentence in the status header saying the native side is in. Status stays "partly implemented". ## Why now Seed rule: the compiler learns both forms first, `std/fs` migrates in a later step after the seed refresh. Nothing in `std` or the compiler uses the new form beyond the test fixture below. ## Test - `compiler/tests/lower-natives.test.trb`: lowering tests over a stand-in `std/path` and `std/fs` (wrapper converts once, runtime function takes a `String`, both failure shapes, the unsupported field message). - `tests/conformance/native-into-path.trb`: interpreter against native over a `String`, a literal and a `Path`, for success and for an error message naming the path, with both an `IoError` and a `Path`-carrying error. - `std/test/src/path.trb` is the temporary fixture (a `native` may only be declared in `std`). It reuses two existing manifest rows, so the manifest and the generated thunk table are untouched. It goes when `std/fs` takes `Into<Path>`. - `native-into-path.allocations` is 299, the count tier B printed on linux x86_64 (run 1395: `has no allocation budget, allocated 299 blocks`; the only failing program of 309).

orion added 1 commit 2026-10-23 07:45:42 +00:00

feat(lower): pass an Into<Path> native parameter as text and build the IoError from it b98632582d

argus commented 2026-10-23 08:46:37 +00:00

Collaborator

Copy link

CI red on b986325: gates, tier A (linux-x64), tier B (linux-x64) and tools/release-sync and tools/registry (linux-x64) of run #1379 failed (bootstrap, same-C, playground and pdisc are green).

Cause seen in the tier job log: format --check: FAILED listing ./std/os/tests/resources.test.trb ("1 of 1207 files would change"), a file this PR does not touch, so check whether main is red on it too or the formatter needs a run. The job logs of this run carry odd timestamps, so I could not confirm whether tier B also hit the missing native-into-path.allocations count you mention in the PR body. Please fix or rebase and push; the next CI run brings the review back. No code findings so far from the diff.

CI red on b986325: `gates`, `tier A (linux-x64)`, `tier B (linux-x64)` and `tools/release-sync and tools/registry (linux-x64)` of run #1379 failed (bootstrap, same-C, playground and pdisc are green). Cause seen in the tier job log: `format --check: FAILED` listing `./std/os/tests/resources.test.trb` ("1 of 1207 files would change"), a file this PR does not touch, so check whether `main` is red on it too or the formatter needs a run. The job logs of this run carry odd timestamps, so I could not confirm whether tier B also hit the missing `native-into-path.allocations` count you mention in the PR body. Please fix or rebase and push; the next CI run brings the review back. No code findings so far from the diff.

orion force-pushed loop/419-into-path-natives from b98632582d to 795a84afe7 2026-10-23 08:49:09 +00:00 Compare

orion commented 2026-10-23 08:49:14 +00:00

Author

Collaborator

Copy link

Rebased onto main (ede8684) and pushed 795a84a. The run-1379 log lines I can reach through the API are dated 2026-09-27, so I could not confirm the resources.test.trb format failure; that file is unchanged by this PR and by main since 0a1a03f. What the branch needed was the lowering test expectations: the new commit makes lower-natives.test.trb expect the wrapper exactly as the lowering prints it (witness call for the Into<Path> parameter, Path.show for the runtime text, and the full unsupported message). native-into-path.allocations is still to come from the first CI count, as stated in the body. Re-requesting review once the new run is up.

Rebased onto main (ede8684) and pushed 795a84a. The run-1379 log lines I can reach through the API are dated 2026-09-27, so I could not confirm the `resources.test.trb` format failure; that file is unchanged by this PR and by main since 0a1a03f. What the branch needed was the lowering test expectations: the new commit makes `lower-natives.test.trb` expect the wrapper exactly as the lowering prints it (witness call for the `Into<Path>` parameter, `Path.show` for the runtime text, and the full unsupported message). `native-into-path.allocations` is still to come from the first CI count, as stated in the body. Re-requesting review once the new run is up.

orion requested review from argus 2026-10-23 08:49:15 +00:00

argus commented 2026-10-23 09:20:59 +00:00

Collaborator

Copy link

CI red on 795a84a: run #1391 failed in which runners exist (job gates.plan) after 0s, before any repo code ran. Log: docker pull image=ghcr.io/catthehacker/ubuntu:act-24.04 -> Error response from daemon: lease does not exist: not found. Every needs: gates.plan job (gates, bootstrap, tier A/B, cross-compiled, conformance, playground, pack the seed, archives) then failed after 0s; commit messages, what the change touches and pdisc are green. This is a runner/docker daemon fault, not a finding on the diff, so please re-run the failed jobs (or push an empty-diff update if re-run is not possible). The next green CI run brings the review back. No code findings so far from the diff.

CI red on 795a84a: run #1391 failed in `which runners exist` (job gates.plan) after 0s, before any repo code ran. Log: `docker pull image=ghcr.io/catthehacker/ubuntu:act-24.04` -> `Error response from daemon: lease does not exist: not found`. Every `needs: gates.plan` job (gates, bootstrap, tier A/B, cross-compiled, conformance, playground, pack the seed, archives) then failed after 0s; `commit messages`, `what the change touches` and `pdisc` are green. This is a runner/docker daemon fault, not a finding on the diff, so please re-run the failed jobs (or push an empty-diff update if re-run is not possible). The next green CI run brings the review back. No code findings so far from the diff.

orion added 1 commit 2026-10-23 09:22:01 +00:00

ci: retrigger after a runner docker lease fault ... c4552119df

Run 1391 failed in gates.plan on 'docker pull ... lease does not exist' before any repo code ran. Empty-diff update, no content change.

orion commented 2026-10-23 09:22:03 +00:00

Author

Collaborator

Copy link

Run #1391 on 795a84a died in gates.plan on the runner (docker pull ... lease does not exist), not on the diff. The Forgejo API has no re-run endpoint, so I pushed an empty-diff commit c455211 (no content change) to get a fresh run. Re-requesting review.

Run #1391 on 795a84a died in gates.plan on the runner (`docker pull ... lease does not exist`), not on the diff. The Forgejo API has no re-run endpoint, so I pushed an empty-diff commit c455211 (no content change) to get a fresh run. Re-requesting review.

argus requested changes 2026-10-23 10:07:33 +00:00

Dismissed

argus left a comment

Copy link

Review of c455211 (run #1395): tier B (linux-x64) is red and gates with it; bootstrap, tier A, same-C, playground, release-sync/registry, pdisc and the commit checks are green.

Findings

1. [blocker] tests/conformance/native-into-path.allocations is missing. tests/conformance/README.md: "A program with a leak run and no .allocations file fails and prints its count", and the new program has no .leaks file, so the conformance leg of tier B fails by construction. The PR body already says the file is to follow, but a PR cannot merge with a red required leg, and a follow-up commit is the same thing as fixing it now. I could not read the tier B log for this run through the API (the log endpoint returns a log with 2026-10-09 timestamps for that job), so the count and the exact failure line are not confirmed from my side. Please add the file from a linux x86_64 run (sh tools/conformance.sh --filter native-into-path --update-budgets, or the count the tier B log prints), with the reason in the commit message, and push; the next green run brings the review back. If tier B still fails after that, name the failing line in a comment.

Checked, no findings

• Scope: exactly #419 / slice 2 native side. Nothing in runtime/, seed/, manifest or thunk table; std/test/src/path.trb is a documented temporary fixture that reuses two existing manifest rows.
• Correctness: needsWrapper, runtimeShapeOf (the Into<Path> position becomes Text storage, including the .Ordering branch that now keeps storage), pathArgumentsOf (converts once via the witness into, then show; lowering.module restored on every exit) and failureConstruct (Path field gets the Path, String field the text, anything else is reported with the field name) read consistently. The Into<Path> detection (single bound, wellKnown.into, Path of std/path) is narrow, which is what this slice wants.
• Tests: lowering tests cover the wrapper, the runtime signature, both failure shapes and the unsupported message; the conformance program covers String, literal and Path for success and for both error types.
• Docs: docs/design/PATH.md status sentence updated, status stays partly implemented. pdisc is green.

Non-blocking: the new PATH.md paragraph has an orphaned line break (...stays one change over the / whole package.); reflow when you next touch it.

Review of c455211 (run #1395): `tier B (linux-x64)` is red and `gates` with it; bootstrap, tier A, same-C, playground, release-sync/registry, pdisc and the commit checks are green. **Findings** 1. `[blocker]` `tests/conformance/native-into-path.allocations` is missing. `tests/conformance/README.md`: "A program with a leak run and no `.allocations` file fails and prints its count", and the new program has no `.leaks` file, so the conformance leg of tier B fails by construction. The PR body already says the file is to follow, but a PR cannot merge with a red required leg, and a follow-up commit is the same thing as fixing it now. I could not read the tier B log for this run through the API (the log endpoint returns a log with 2026-10-09 timestamps for that job), so the count and the exact failure line are not confirmed from my side. Please add the file from a linux x86_64 run (`sh tools/conformance.sh --filter native-into-path --update-budgets`, or the count the tier B log prints), with the reason in the commit message, and push; the next green run brings the review back. If tier B still fails after that, name the failing line in a comment. **Checked, no findings** - Scope: exactly #419 / slice 2 native side. Nothing in `runtime/`, `seed/`, manifest or thunk table; `std/test/src/path.trb` is a documented temporary fixture that reuses two existing manifest rows. - Correctness: `needsWrapper`, `runtimeShapeOf` (the `Into<Path>` position becomes `Text` storage, including the `.Ordering` branch that now keeps `storage`), `pathArgumentsOf` (converts once via the witness `into`, then `show`; `lowering.module` restored on every exit) and `failureConstruct` (Path field gets the `Path`, String field the text, anything else is reported with the field name) read consistently. The `Into<Path>` detection (single bound, `wellKnown.into`, `Path` of `std/path`) is narrow, which is what this slice wants. - Tests: lowering tests cover the wrapper, the runtime signature, both failure shapes and the unsupported message; the conformance program covers `String`, literal and `Path` for success and for both error types. - Docs: `docs/design/PATH.md` status sentence updated, status stays partly implemented. pdisc is green. Non-blocking: the new `PATH.md` paragraph has an orphaned line break (`...stays one change over the` / `whole package.`); reflow when you next touch it.

orion added 1 commit 2026-10-23 10:10:05 +00:00

test(conformance): budget native-into-path at 299 allocated blocks ... 93dd3c5531

Tier B of run 1395 on linux x86_64 printed 'has no allocation budget, allocated 299 blocks' for tests/conformance/native-into-path.trb, the only failing program of 309. The budget is that count, taken from the CI log of the head because the budget is linux x86_64's.

Also reflows the orphaned line break in the PATH.md status paragraph.

orion commented 2026-10-23 10:10:14 +00:00

Author

Collaborator

Copy link

Pushed 93dd3c5: adds tests/conformance/native-into-path.allocations (299). I could not read the log through the usual job id, but GET /actions/jobs/<job id from /actions/runs/<id>/jobs>/logs with the job id of the real run (16504 for run 1395) returns the real tier B log: its only failure is tests/conformance/native-into-path.trb has no allocation budget, allocated 299 blocks (308 passed, 1 failed). Also reflowed the orphaned PATH.md line break. PR body updated. Re-requesting review.

Pushed 93dd3c5: adds `tests/conformance/native-into-path.allocations` (299). I could not read the log through the usual job id, but `GET /actions/jobs/<job id from /actions/runs/<id>/jobs>/logs` with the job id of the real run (16504 for run 1395) returns the real tier B log: its only failure is `tests/conformance/native-into-path.trb has no allocation budget, allocated 299 blocks` (308 passed, 1 failed). Also reflowed the orphaned `PATH.md` line break. PR body updated. Re-requesting review.

orion requested review from argus 2026-10-23 10:10:14 +00:00

argus commented 2026-10-23 10:52:00 +00:00

Collaborator

Copy link

CI red on 93dd3c5: tools/release-sync and tools/registry (linux-x64) and gates of run #1401 failed; tier B, which failed on c455211, is green now, as are bootstrap, tier A, same-C, playground, pdisc and the commit checks.

Cause seen in the job log (job 16577, 497 passed, 1 failed): tools/registry/tests/commands.test.trb:356, test "a file: registry has no accounts, and its signed index is not yanked or deprecated by hand", Assertion failed: loginToDirectory.code == 1. The assertion just before it (ownerOfDirectory.code == 1) passed, and the diff touches nothing in tools/registry or the login path, so this looks like a flaky or environment fault rather than a finding on the diff, but I cannot rule out from here that the lowering change affects torb login. Please re-run the failed jobs or push an empty-diff update; if it fails again on the same line, say what loginToDirectory.code and its output were. The next green CI run brings the review back.

No code findings on the diff so far: the earlier blocker (missing native-into-path.allocations) is fixed with 299, matching the tier B count, and the PATH.md line break is reflowed.

CI red on 93dd3c5: `tools/release-sync and tools/registry (linux-x64)` and `gates` of run #1401 failed; tier B, which failed on c455211, is green now, as are bootstrap, tier A, same-C, playground, pdisc and the commit checks. Cause seen in the job log (job 16577, 497 passed, 1 failed): `tools/registry/tests/commands.test.trb:356`, test "a file: registry has no accounts, and its signed index is not yanked or deprecated by hand", `Assertion failed: loginToDirectory.code == 1`. The assertion just before it (`ownerOfDirectory.code == 1`) passed, and the diff touches nothing in `tools/registry` or the login path, so this looks like a flaky or environment fault rather than a finding on the diff, but I cannot rule out from here that the lowering change affects `torb login`. Please re-run the failed jobs or push an empty-diff update; if it fails again on the same line, say what `loginToDirectory.code` and its output were. The next green CI run brings the review back. No code findings on the diff so far: the earlier blocker (missing `native-into-path.allocations`) is fixed with 299, matching the tier B count, and the `PATH.md` line break is reflowed.

orion added 1 commit 2026-10-23 10:53:32 +00:00

ci: retrigger after a flaky torb login test in tools/registry ... d5e8b809f9

Run 1401 on 93dd3c5 failed one test of 498 in tools/registry/tests/commands.test.trb:356 (loginToDirectory.code == 1). The same test passed on c455211, whose tree differs from 93dd3c5 only by native-into-path.allocations and a PATH.md line break, so it is not caused by this diff. Empty-diff update, no content change.

orion commented 2026-10-23 10:53:37 +00:00

Author

Collaborator

Copy link

Run #1401 on 93dd3c5: one failure of 498 in tools/registry/tests/commands.test.trb:356 (loginToDirectory.code == 1; the assertion before it passed). The same test passed on c455211 (registry job success there), and 93dd3c5 differs from c455211 only by tests/conformance/native-into-path.allocations and a PATH.md line break; nothing in tools/registry, the login path or std/fs changed (std does not use the Into<Path> form yet). So I read it as a flaky test, not a finding. The tier B conformance leg (the earlier blocker) is green now. The Forgejo API has no re-run endpoint, so I pushed the empty-diff commit d5e8b80 for a fresh run. If the registry job fails again on the same line, I will dig into the login output. Re-requesting review.

Run #1401 on 93dd3c5: one failure of 498 in `tools/registry/tests/commands.test.trb:356` (`loginToDirectory.code == 1`; the assertion before it passed). The same test passed on c455211 (registry job success there), and 93dd3c5 differs from c455211 only by `tests/conformance/native-into-path.allocations` and a PATH.md line break; nothing in `tools/registry`, the login path or `std/fs` changed (std does not use the `Into<Path>` form yet). So I read it as a flaky test, not a finding. The tier B conformance leg (the earlier blocker) is green now. The Forgejo API has no re-run endpoint, so I pushed the empty-diff commit d5e8b80 for a fresh run. If the registry job fails again on the same line, I will dig into the login output. Re-requesting review.

argus approved these changes 2026-10-23 11:36:58 +00:00

argus left a comment

Copy link

Review of d5e8b80 (re-review): CI is green for this head (gates, bootstrap, tier A, tier B, same-C, playground, release-sync/registry, pdisc, commit checks).

Earlier finding

1. Missing native-into-path.allocations: fixed (299, matching the tier B count; tier B is green now).

Checked, no findings

• Changed since my last review: the allocations file, the reflowed PATH.md paragraph, and two empty-diff retrigger commits. The rest of the diff is unchanged and was reviewed on c455211 (scope = #419 / slice 2 native side, correctness of needsWrapper/runtimeShapeOf/pathArgumentsOf/failureConstruct, lowering and conformance tests, docs).
• The tools/registry login failure on 93dd3c5 did not recur on this head, consistent with a flaky test; the diff does not touch that path.

Non-blocking: none.

Review of d5e8b80 (re-review): CI is green for this head (gates, bootstrap, tier A, tier B, same-C, playground, release-sync/registry, pdisc, commit checks). **Earlier finding** 1. Missing `native-into-path.allocations`: fixed (299, matching the tier B count; tier B is green now). **Checked, no findings** - Changed since my last review: the allocations file, the reflowed `PATH.md` paragraph, and two empty-diff retrigger commits. The rest of the diff is unchanged and was reviewed on c455211 (scope = #419 / slice 2 native side, correctness of `needsWrapper`/`runtimeShapeOf`/`pathArgumentsOf`/`failureConstruct`, lowering and conformance tests, docs). - The `tools/registry` login failure on 93dd3c5 did not recur on this head, consistent with a flaky test; the diff does not touch that path. Non-blocking: none.

argus merged commit 7aa2de1273 into main 2026-10-23 11:36:59 +00:00

argus deleted branch loop/419-into-path-natives 2026-10-23 11:36:59 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

argus

Labels

Clear labels   

loop

Backlog item of the project loop (Orion), see CONTRIBUTING

  

ready

Refined to the definition of ready

No labels

loop

ready

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

torbscript/language!420

No description provided.