Skip to content

feat(ts): translate str::as_ptr as a read-only pointer into its UTF-8 - #634

Merged
moq-bot[bot] merged 10 commits into
mainfrom
quest/m2/ts-model/str-as-ptr
Nov 7, 2026
Merged

moq-bot[bot] merged 10 commits into
mainfrom
quest/m2/ts-model/str-as-ptr

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Closes the quest quest/m2/ts-model/str-as-ptr.md.

Decisions

  • Reads past a substring's own bytes but inside its text, such as s[..1].as_ptr().add(1) read forward and s[1..].as_ptr().wrapping_sub(1) read back, keep failing loudly at run time with a rust2-ts: error. ✅ Merge as is (rejected: tracking str lengths and wrapping offsets through to reads at translation; a substring sharing its text's encoding, which reverses moq-dev/rust2#574; refusing str::as_ptr unless the body proves every read in bounds). Decided by the maintainer on 2026-11-07 at the round-3 design checkpoint. These reads are UB under Stacked Borrows and allowed under Tree Borrows, and Rust's memory model is unsettled, so a loud failure never silently differs from Rust. The constant sub refusal from round 1 is kept. The future fix is recorded in quest/m3/ts-runtime-throws.md's gap list: if Rust rules these reads defined, a substring's pointer carries its text's encoding and offset.

Problem

str::as_ptr was an unsupported call, so arrayvec's ArrayString::try_push_str, which copies a &str into its buffer through the pointer, refused translation. At the moq pin that is 1 refusal in the model's census and 3 in moq-net whole (arrayvec's, plus two in zlib-rs). It was the last child of quest/m2/ts-model/README.md that this repo owns. moq-dev/rust2#574 settled that a &str stays a JS string, so the bytes behind the pointer are an encoding of the text.

Approach

  • rs.strAsPtr(s) encodes the text's UTF-8 and returns a pointer to its first byte. The pointer is a TextAt, a private AtIndex subclass. Reads work as they do for any byte pointer: *p, read, arithmetic, offset_from within one encoding, ptr::copy/copy_nonoverlapping from it, and slice::from_raw_parts.
  • A write fails loudly. Rust forbids writing through a &str's pointer. Here such a write would land in a copy the text never reads, so it fails at run time instead. That covers *p = x, a copy into it, and a mutable slice (from_raw_parts_mut, or derefBytes(false) of a raw slice). Arithmetic keeps the pointer read-only: AtIndex gains an at(index) method that ptrAdd and ptrWrappingAdd call, and TextAt overrides it.
  • Origins. Each str::as_ptr call is its own origin (Array::Text) in mir/origin.rs. So offset_from between two calls' pointers is refused with a span wherever the body shows both. A typical case is a substring's pointer against its text's, which Rust allows but JS can't match. A pointer starts at index 0 of its own encoding, so a constant add/sub/offset the body shows moving it back before its first byte is refused with a span, as one off an Array::Copy is. That covers a substring's pointer moved back into its text. Anything the body doesn't show is left to the runtime, which fails on a read outside the encoding.

Validation

  • Differential, lang/strs' new strs::pointers: text of every UTF-8 width and a substring from each char boundary, against native Rust. It copies into a Vec's room and between bytes, reads every tail through from_raw_parts, walks forward and back by *, NonNull, and offset_from/offset_from_unsigned, and pushes into a [MaybeUninit<u8>; 12] as ArrayString does.
  • Differential, stock's new fixed::strings: the real arrayvec::ArrayString<12> (0.7.8, the version moq's lock has). It covers try_push, try_push_str with its errors, push_str, pop, truncate, remove, from, and clear over chars of each width.
  • Runtime, tests/pointers.ts: reads, arithmetic, and views pass. Writes, copies in, and mutable slices fail with a rust2-ts: error, and so does offset_from across two encodings.
  • Reject census: text_apart's two offset_froms and text_before's sub(1) are refused with spans.
  • Census at the moq pin, this branch against the same branch with the new mapping removed: model 52 → 51 and moq-net 356 → 353. The only lines that change are the three str::as_ptr refusals, and no new refusal appears.
  • just ci passes at 8fdffa85b, after merging origin/main at 049c5516d with moq-dev/rust2#633's TypeId-hash snapshot fix.

Impact

Public API (translator output):

  • str::as_ptr translates instead of refusing.
  • New runtime export: rs.strAsPtr.
  • New AtIndex.at(index) method.
  • The census quotes str and str::as_ptr verbatim in its constructs.
  • Fewer refusals: arrayvec's ArrayString::try_push_str, and zlib-rs' two str::as_ptrs.

Quests:

  • Deletes quest/m2/ts-model/str-as-ptr.md and its link.
  • Adds the substring reads past their own bytes to quest/m3/ts-runtime-throws.md's gaps, with the fix to take if Rust rules them defined.
  • quest/m2/ts-model/README.md loses its last child. It is sized [L] for its remaining end-to-end work, and its census paragraph is refreshed to 51.

Choices made without asking

  • Runtime write checks, not a translation-time refusal. A write needs cast_mut or as *mut of the pointer, and the result can flow anywhere, so only the runtime sees every write. That is the same split the README documents for pointers from elsewhere. The checks cost one instanceof per copy or mutable slice and nothing on other pointers' writes, because TextAt is its own class.
  • Two UB-only routes stay unchecked, since each needs a second forbidden step and checking them would cost every byte pointer:
    • a write through a *mut [u8; N] cast from a raw slice of the text;
    • a write through the as_ptr of a shared slice that from_raw_parts made of the text.
  • A new encoding per call. No cache, since JS strings can't be weak keys. That makes two calls' pointers different arrays, which is why they are refused or fail when measured against each other. Comparing *const u8s was already refused.
  • str::as_mut_ptr stays unsupported. Writes through it are defined in Rust and must reach the String, so it needs the bytes to be the string's own storage. Nothing in moq calls it, and it is outside this quest's Goal.
  • arrayvec added to the stock fixture's dependencies, at moq's locked version, so the real consumer crate is tested against native and not only a copy of its code.

Alternatives

  • Caching one encoding per string, so that ptr::eq and offset_from agree across calls. A strong Map of strings leaks, and a substring is a new JS string anyway, so its offset into its text still could not match.
  • Making the bytes read-only through an immutable ArrayBuffer (transferToImmutable). Node, Bun, Chrome, and Firefox don't all ship it, and writes to it fail silently outside strict mode.

Follow-ups

None.

Models

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

Review

mechanical fix by codex, gpt-6.1-sol, medium effort: Merged base and re·solved ts/fixtures/reject/census.txt by preserving both sides’ expected diagnostics; ran git diff --check --cached, just check base, and just test base successfully. (head 17055d7)

mechanical fix by codex, gpt-6.1-sol, medium effort: Merged base, preserved both documentation and fixture additions, and regenerated harness.ts; passed UPDATE_EXPECT=1 just ts test --test fixtures stock, just check base, and just test base. (head 2b9b9d4)

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1b52a-0b86-7ac1-8df0-8726554271f2, round 5
  • Head: 8fdffa8
  • Base: origin/main at 049c551
  • Score: 10/10, The branch’s added and deleted lines match the previously approved diff, the README merge preserves both changes, and pointer runtime tests pass; no new defects were found.
  • Findings: none

7335b88da adds only a conflict-free merge of origin/main (moq-dev/rust2#646, bench and quest files) over the reviewed head; just check passes on it.

@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

kixelated and others added 2 commits November 7, 2026 00:24
A `&str` stays a JS string, so `rs.strAsPtr` encodes its UTF-8 and points
at the first byte. Reads, arithmetic, copies from it, and `from_raw_parts`
read what Rust reads; a write, a copy into it, or a mutable slice of it,
which Rust forbids, fails at run time. `offset_from` between two calls'
pointers is refused where the body shows both.

arrayvec's `ArrayString` now translates: the stock fixture runs it against
native Rust.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A string's bytes through its pointer feat(ts): translate str::as_ptr as a read-only pointer into its UTF-8 Nov 7, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 7, 2026 15:08
@coderabbitai

coderabbitai Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

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: 8ef5cb6e-1afe-4732-bf52-32ae74197762
  • 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 7335b88 against base c0f5344.

No actionable findings identified in this static review. TextAt preserves the read-only pointer through normal and wrapping arithmetic, and the copy/mutable-slice paths reject writes into its encoding. I checked the call mapping, origin-based refusals, bounds and offset handling, the registered UTF-8 pointer cases and their generated output, and the ArrayString consumer fixture. Using the existing pointer machinery with a distinct text origin is a coherent direction for this scoped support.

The documented limits remain: separate as_ptr calls produce separate encodings, and substring reads beyond their own bytes can fail at runtime. This review does not treat those explicitly recorded design limits as resolved or settle the Rust memory-model question. The two documented UB-only write routes are also outside the added guards.

Verification limits: GitHub source/diff inspection only. I did not execute runtime tests, native differential fixtures, translator builds, the census, or just ci; the reported passing results and refusal reductions have not been 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
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 7, 2026
@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown

quest land watch: the merge queue removed this pull request at 7335b88 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 7, 2026 15:44
@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown

quest land watch: armed auto-merge on this pull request at 2b9b9d4, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex, gpt-6.1-sol, medium effort): Merged base, preserved both documentation and fixture additions, and regenerated harness.ts; passed UPDATE_EXPECT=1 just ts test --test fixtures stock, just check base, and just test base..

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 7, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 7, 2026
@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown

quest land watch: the merge queue removed this pull request at 2b9b9d4 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 7, 2026 16:12
@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown

quest land watch: armed auto-merge on this pull request at 17055d7, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex, gpt-6.1-sol, medium effort): Merged base and re·solved ts/fixtures/reject/census.txt by preserving both sides’ expected diagnostics; ran git diff --check --cached, just check base, and just test base successfully..

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit ed45ff0 Nov 7, 2026
7 checks passed
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