Skip to content

feat(ts): a byte local whose address is taken, in a buffer of its own - #640

Merged
kixelated merged 11 commits into
mainfrom
quest/m3/ts-unsafe/lone-byte-buffers
Nov 7, 2026
Merged

kixelated merged 11 commits into
mainfrom
quest/m3/ts-unsafe/lone-byte-buffers

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Claims quest/m3/ts-unsafe/lone-byte-buffers.md, planned in moq-dev/rust2#624, and deletes it.

Problem

JS kept a u8 local in no array of bytes, so a raw or mutable slice of it could not be a view of where it is. moq-dev/rust2#624 refused such slices, and casts of them to *const [u8; N], only on the routes its raw_start tracer followed, and kept missing routes (&raw *s, NonNull::from, NonNull::new(p).unwrap()). The rest threw at run time.

Approach

A u8 or MaybeUninit<u8> local whose &raw the body takes, or whose reference made in the body is cast to a raw pointer (&mut b as *mut u8, let p: *const u8 = &b), is element 0 of a Uint8Array(1) of its own (Slot::Byte). Every read and write is b[0], and every pointer to it is rs.arrayAsPtr(b). Its raw, mutable, and shared slices, and casts of them to *const [u8; N], are views of that buffer on every route, and offset_from between its pointers measures them. Its pointers' origin is Array::Byte, a buffer of length 1, so a pointer, slice, copy, or write_bytes the body shows reaching past it is still refused at translation, as for any value in no array. A parameter keeps its name in the signature and is copied into the buffer on entry.

The lone-byte refusals are removed: Fx::lone_byte_array with its raw_start tracer, and stray's refusal of a raw or mutable slice of a lone byte. Array::Copy stays for bytes still in no array, where a shared slice is still an exact copy.

Proven by the differential case pointers::lone_bytes against native Rust. It covers each slice and cast route, a parameter, &mut b as *mut u8, a coercion from &c, and a MaybeUninit<u8>.

Decisions

  • Bytes in fields (2026-11-07, maintainer): ✅ leave the old lone-byte refusals removed. A raw or mutable slice of a byte stored in a field (a struct's, a tuple's, a closure's capture, or one an async body keeps across an .await) throws at run time. Throw only where Rust panics records the gap and owns replacing it with a translation or a refusal: a buffer for each address-taken u8 field. Rejected: keeping the refusals for fields only, which is partial route tracing that would be deleted later. Rejected: giving field bytes buffers in this PR.

Choices made without asking

  • The quest asks for locals whose &raw the body takes. A &raw of *r, where r is a reference to the local made once in the body, buffers the local too, unless the body takes a &mut to r (or &raw mut r), which could redirect it to another byte. That is what &mut b as *mut u8 and a coercion to a raw pointer compile to, and without it the most common spelling kept throwing. Calls such as ptr::from_mut(&mut b) are not traced and still throw, as they did before this PR (origin never traced them). ts-runtime-throws.md lists that as well.
  • Merging moq-dev/rust2#642's write_bytes showed that a buffered byte's origin, Array::Owned, had no known length. That silenced its refusal of (&raw mut b).write_bytes(0xff, 2) (fixtures/reject/src/pointers.rs:written_bytes), and every other "reaching past" refusal for a buffered byte. At run time AtIndex.reach would still have thrown. The origin is now Array::Byte, of length 1, which keeps the refusals and main's expected lines unchanged. A write within the buffer is exact, but a write past it has no native meaning (it is UB), so the refusal stays.
  • MaybeUninit<u8> locals are buffered too, since crate::lower::byte counts them as bytes.

Overlaps

  • moq-dev/rust2#621 translates &raw const *r for any value JS copies. This PR handles it only for a buffered byte, in the same RawPtr arm of rvalue.rs. Whichever lands second resolves the conflict and keeps both: the buffered byte's arm first, then feat(ts): translate a raw pointer behind a reference #621's general one.
  • moq-dev/rust2#608 also edits quest/m3/ts-runtime-throws.md's gap list, and plans slice::from_mut(&mut b) as a quest of its own (that call is refused, not thrown).

Caused by

  • moq-dev/rust2#624 - refused casts of a lone byte's raw slice only on the routes raw_start traced
  • moq-dev/rust2#400 - kept a byte local in no array of bytes, so its raw and mutable slices could not be views

Impact

  • The translator's output: such a local is now a Uint8Array in generated code, and constructs the reject fixture refused (lone_byte_slices, lone_byte_arrays) now translate. No runtime or wire change.

Alternatives

  • Keep lone_byte_array for bytes in fields: rejected, see Decisions.
  • Buffer every byte local that any &mut borrows, which would also cover call routes like ptr::from_mut(&mut b): not taken. It changes the output of every &mut u8 local for routes no consumer uses.

Validation

  • quest gate just ci origin/main on head de3b678 (origin/main at 76f9955ca merged in): passed. That includes lang::pointers' new differential case pointers::lone_bytes against native Rust (89 fixture tests).
  • With moq-dev/rust2#633 on main, UPDATE_EXPECT=1 just ts test left every snapshot unchanged, dyns and panics/hook included.
  • Review round 1 found a reference redirected through &mut r being traced to its first borrow. The new redirect case reproduced it (145 differences against native Rust) before the fix and passes after.
  • Merging origin/main at 6e0ce8ac2: one conflict, in ts/README.md's pointer row. Resolved by keeping this PR's row and applying moq-dev/rust2#642's write_bytes edits to it. reject and reject_census then failed on the lost written_bytes refusal, fixed above. The rebound lines in that failure were only a shifted diff window: expected and actual agree once written_bytes is refused, and no snapshot was regenerated.
  • quest gate nix develop -c just ci after the fix: passed (89 fixture tests, including pointers::lone_bytes against native Rust). Run again after the conflict-free merge of origin/main at f82788274, which round 3 reviewed against: passed.
  • Merging origin/main at ed45ff031 (moq-dev/rust2#634, str::as_ptr): three conflicts, resolved by hand. ts/README.md's pointer row is this PR's row plus feat(ts): translate str::as_ptr as a read-only pointer into its UTF-8 #634's str::as_ptr sentence. fixtures/reject/src/pointers.rs keeps feat(ts): translate str::as_ptr as a read-only pointer into its UTF-8 #634's text_apart and text_before and drops lone_byte_slices and lone_byte_arrays as before. quest/m3/ts-runtime-throws.md's gap list keeps both the substring-read gap and the field-byte gap. origin.rs auto-merged feat(ts): translate str::as_ptr as a read-only pointer into its UTF-8 #634's Array::Text beside Array::Byte. quest gate nix develop -c just ci passed afterwards.
  • The last merge, of origin/main at 132d0ad0d, was conflict-free and touched only ffi/ and quest/. quest check --fix: ok. Review round 4 covered this head.
  • quest check --fix: ok.

Follow-ups

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, high effort

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, high effort
  • Session: 01a1b680-72a7-7f00-93a9-494d8a478eed, round 4
  • Head: 7a18933
  • Base: origin/main at 132d0ad
  • Score: 10/10, The merge resolutions preserve byte-buffer and text-pointer behavior, and differential reruns passed 22,358 cases with no differences.
  • Findings: none

@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

A `u8` or `MaybeUninit<u8>` local whose `&raw` the body takes, or that of a
reference to it made there, is element 0 of a `Uint8Array` of its own, so
its raw, mutable, and shared slices, and arrays cast from them, view it.
The lone-byte refusals in origin.rs go; a byte in a field still throws at
run time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A byte local whose address is taken, in a buffer of its own feat(ts): a byte local whose address is taken, in a buffer of its own Nov 7, 2026
@kixelated
kixelated marked this pull request as ready for review November 7, 2026 15:15
@coderabbitai

coderabbitai Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0949d0d9-6419-40a9-bcef-c69908d0847d
Commits

Reviewing files that changed from the base of the PR and between 132d0ad and 7a18933.

Files selected for processing: 17

  • quest/m3/ts-runtime-throws.md
  • quest/m3/ts-unsafe/README.md
  • quest/m3/ts-unsafe/lone-byte-buffers.md
  • ts/README.md
  • ts/fixtures/lang/elements/expected/elements.ts
  • ts/fixtures/lang/pointers/expected/lib.ts
  • ts/fixtures/lang/pointers/expected/pointers.ts
  • ts/fixtures/lang/pointers/src/lib.rs
  • ts/fixtures/lang/pointers/src/pointers.rs
  • ts/fixtures/reject/census.txt
  • ts/fixtures/reject/expected.txt
  • ts/fixtures/reject/src/pointers.rs
  • ts/fixtures/reject/ts.toml
  • ts/src/lower/mir/mod.rs
  • ts/src/lower/mir/origin.rs
  • ts/src/lower/mir/place.rs
  • ts/src/lower/mir/rvalue.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit de3b678 against base 76f9955.

No actionable findings identified in this static review. Giving address-taken byte locals stable one-byte buffers fixes their slice/view representation at its source. I checked Slot::Byte reads and writes, parameter entry copies without changing the numeric ABI, alias/place conversion, pointer origins, the guard against tracing redirected references, and the registered lone_bytes fixture and generated output. A separate inspection also checked pending-read ordering and stable buffer identity without finding a concrete regression.

Direction: retain the new differential and redirection coverage. The explicitly documented field-byte and call-only ptr::from_mut limitations remain runtime gaps; this review does not claim those routes are now supported.

Verification limits: GitHub source/diff inspection only. I did not execute the translator, TypeScript checks, native differential fixtures, quest checks, or just ci. Reported passing tests and mutation reproductions have not been independently reproduced. This is a COMMENT, not an approval.

(Written by review (OpenAI))

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit 7a18933 against base 132d0ad, comparing the changes since my review of de3b678 while accounting for merged main changes.

No new actionable findings identified. The substantive follow-up replaces the buffered byte's unknown-length Array::Owned origin with Array::Byte(Local), retaining stable identity while making its length one. This restores body-visible bounds refusals, including write_bytes past the byte, without treating a shared slice of the buffered byte as a copied lone byte. The merged text-pointer origin remains separate. The other lowering and differential-test changes retain the previously reviewed behavior.

Direction: retain the written_bytes rejection coverage alongside lone_bytes and redirected-reference cases. The documented field-byte and call-only ptr::from_mut gaps remain outside this change.

Verification limits: GitHub source/diff inspection only; no translator, native differential tests, TypeScript checks, or CI executed. Reported passing runs were not independently reproduced. This is a COMMENT, not an approval.

(Written by review (OpenAI))

@kixelated
kixelated added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit f20b505 Nov 7, 2026
7 checks passed
@kixelated
kixelated deleted the quest/m3/ts-unsafe/lone-byte-buffers branch November 7, 2026 17:01
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