Skip to content

feat(ts): translate Debug of cells and locks, keeping a held value's effects in order - #484

Merged
moq-bot[bot] merged 6 commits into
mainfrom
quest/m3/ts-refcell-debug
Nov 3, 2026
Merged

moq-bot[bot] merged 6 commits into
mainfrom
quest/m3/ts-refcell-debug

Conversation

@kixelated

@kixelated kixelated commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

{:?} of a RefCell was an "unsupported call to <RefCell<..> as Debug>::fmt", and so was a Cell's, found writing Box<dyn Debug> (#445) and &mut dyn Debug (#453). Mutex and RwLock were refused the same way (the locks fixture hand-writes a Debug to avoid it), and RwLock and UnsafeCell share the RefCell and Cell lowering, so they are the same gap.

Writing the tests turned up a second bug that predates this PR: a held-back value could be reordered after a later effect. materialize flushed only the held values up to the one it needed into a pre buffer, and emit(pre) then flushed the later effectful ones ahead of it. On main, let free = format!("{lock:?}"); let _held = lock.0.lock().unwrap();, with a user Debug that calls try_lock, formatted free after the lock was taken. Native and translated output differed.

Approach

  • fmt::std_impl lowers Debug of RefCell, RwLock, Mutex, Cell, and UnsafeCell to new runtime helpers (refCellDebug, rwLockDebug, mutexDebug, cellDebug) that write std's output byte for byte:
    • RefCell { value: .. }, or <borrowed> unpadded while mutably borrowed.
    • RwLock { data: .., poisoned: false, .. }, or <locked> unpadded while written.
    • Mutex { data: "<locked>", .. } while held. It is quoted because std's Mutex writes &"<locked>" while RwLock writes format_args!.
    • Cell { value: .. } of what get gives.
    • UnsafeCell { .. }.
  • Like std, each helper holds a shared borrow, the lock, or a read while it writes the value, so a value whose Debug probes its own cell or lock sees what native sees. Ref/RefMut already forwarded to what they lend; they are now covered through nested cells.
  • materialize also flushes, in order, any later held value that does or reads something. emit would have flushed those first anyway, but in the wrong order.
  • Differential tests: borrows::printed and locks::shown cover plain, {:#?}, padded, and &dyn Debug forms; free, shared, and exclusive borrows; cells nested in cells, vectors, Arc, and derived Debug; guards of cells; and reentrant probes. Without the guard, the probes fail against native.

Impact

  • Translator output: these Debug impls now translate.
  • The materialize change reorders or refolds a few held temporaries in other snapshots (codec, stock buf, several lang modules). Most moves are back to MIR order or remove a temporary; a few keep one more const (for example iters::updates' copyWithin). Every differential still passes.
  • Runtime: new rt.refCellDebug, rt.rwLockDebug, rt.mutexDebug, and rt.cellDebug. Nothing breaks, so there is no MIGRATE.md entry.

Alternatives

  • Keeping strictly to the quest's RefCell/Cell: RwLock and UnsafeCell share their arms and would still need distinguishing, and Mutex is the same few lines, so the whole family is covered here.
  • A narrower materialize fix that only reorders when out is later emitted: the function can't tell, and the conservative flush is correct either way.

Follow-ups

  • LazyCell/LazyLock Debug was not checked; it is outside this family's representation.
  • A crate's own fmt::Write with placeholders is still refused, which is why the Cell read-before-write ordering has no differential test.

Review

mechanical fix by codex / Codex: Merged base cleanly and regenerated two stale TypeScript snapshots; passed cargo test --locked in ts/ and go/, cargo clippy --locked --all-targets -- -D warnings and cargo fmt --all --check in ts/, and tsc -p runtime in ts/. (head e4ed6c2)

  • Reviewer: Codex, gpt-6.1-sol, medium (quest review).
  • Round 1 on a18f620, two findings, both fixed:
    • Guard not held while formatting: fixed in 31cf8f5, with reentrant differential cases.
    • Cell value read after the prefix: fixed by passing get's copied value. Not differentially testable, since the translator refuses a crate's own fmt::Write with placeholders.
  • Round 2 on ca91845: no findings.

🤖 Generated with Claude Code

kixelated and others added 4 commits November 2, 2026 20:52
…effects in order

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alue

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Debug of a RefCell or a Cell feat(ts): translate Debug of cells and locks, keeping a held value's effects in order Nov 3, 2026
@kixelated
kixelated marked this pull request as ready for review November 3, 2026 05:29
@coderabbitai

coderabbitai Bot commented Nov 3, 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: 5a3f0d6d-f0f1-4bd9-af66-5b5932a65695
  • 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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@kixelated
kixelated added this pull request to the merge queue Nov 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 3, 2026
@moq-bot

moq-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

quest land watch: the merge queue removed this pull request at ca91845 because its merge_group build failed (failed_checks).

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.

Queue / Queue (job) failure at step Check and test the merge group:

…r loop over an `Option`. This is more readably written as an `if let` statement
  --> fixtures/task/src/drops/iters.rs:22:11
   |
22 |     for g in Some(guard(7)) {
   |              ^^^^^^^^^^^^^^
   |
   = note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
   |
22 -     for g in Some(guard(7)) {
22 +     while let Some(g) = Some(guard(7)) {
   |
help: consider using `if let` to clear intent
   |
22 -     for g in Some(guard(7)) {
22 +     if let Some(g) = Some(guard(7)) {
   |

test lang::singles ... ok
    Blocking waiting for file lock on build directory
test reject ... ok
    Blocking waiting for file lock on build directory
test stable_names ... ok
warning: `task-fixture` (lib) generated 12 warnings (run `cargo fix --lib -p task-fixture` to apply 1 suggestion)
    Finished `differential` profile [optimized + debuginfo] target(s) in 10.53s
   Compiling lang-separated-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/separated)
    Blocking waiting for file lock on build directory
test lang::statics has been running for over 60 seconds
    Blocking waiting for file lock on build directory
test lang::tuples has been running for over 60 seconds
test lang::utf8 has been running for over 60 seconds
    Blocking waiting for file lock on build directory
test lang::wrappers has been running for over 60 seconds
    Blocking waiting for file lock on build directory
    Blocking waiting for file lock on build directory
    Finished `differential` profile [optimized + debuginfo] target(s) in 18.86s
   Compiling lang-utf8-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/utf8)
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on build directory
test bind ... ok
    Finished `differential` profile [optimized + debuginfo] target(s) in 19.18s
   Compiling lang-slices-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/slices)
warning: irrefutable `if let` pattern
   --> fixtures/lang/slices/src/slices.rs:442:7
    |
442 |             if let [_, rest @ ..] = &mut array {
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
    = note: `#[warn(irrefutable_let_patterns)]` on by default

test lang::utf8 ... ok
test lang::separated ... ok
test lang::zeros has been running for over 60 seconds
test task ... ok
warning: `lang-slices-fixture` (lib) generated 1 warning
    Finished `differential` profile [optimized + debuginfo] target(s) in 21.67s
   Compiling lang-statics-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/statics)
warning: mutation of an interior mutable `const` item with call to `get`
   --> fixtures/lang/statics/src/statics.rs:198:27
    |
198 |     out.extend([tally.get(), TALLY.get(), PAIR.into_inner().1]);
    |                              -----^^^^^^
    |                              |
    |                              `TALLY` is a interior mutable `const` item of type `Cell<u8>`
    |
    = note: each usage of a `const` item creates a new temporary
    = note: only the temporaries and never the original `const TALLY` will be modified
    = help: for more details on interior mutability see <https://doc.rust-lang.org/reference/interior-mutability.html>
    = note: `#[warn(const_item_interior_mutations)]` on by default
help: for a shared instance of `TALLY`, consider making it a `static` item instead
    |
173 - const TALLY: Cell<u8> = Cell::new(250);
173 + static TALLY: Cell<u8> = Cell::new(250);
    |

warning: `lang-statics-fixture` (lib) generated 1 warning
    Finished `differential` profile [optimized + debuginfo] target(s) in 20.29s
   Compiling lang-shapes-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/shapes)
warning: unused import: `crate::generics::Wrap`
  --> fixtures/lang/shapes/src/fmt.rs:15:5
   |
15 | use crate::generics::Wrap;
   |     ^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

test share has been running for over 60 seconds
test lang::statics ... ok
test lang::slices ... ok
warning: `lang-shapes-fixture` (lib) generated 1 warning (run `cargo fix --lib -p lang-shapes-fixture` to apply 1 suggestion)
    Finished `differential` profile [optimized + debuginfo] target(s) in 15.05s
   Compiling lang-wrappers-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/wrappers)
    Finished `differential` profile [optimized + debuginfo] target(s) in 15.84s
   Compiling lang-zeros-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/zeros)
test lang::wrappers ... ok
    Finished `differential` profile [optimized + debuginfo] target(s) in 15.99s
   Compiling lang-tuples-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/tuples)
test lang::shapes ... ok
test lang::zeros ... ok
    Finished `differential` profile [optimized + debuginfo] target(s) in 16.94s
   Compiling lang-sorts-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/sorts)
warning: field `pad` is never read
  --> fixtures/lang/sorts/src/sorts.rs:54:2
   |
51 | struct Item<P> {
   |        ---- field in this struct
...
54 |     pad: P,
   |     ^^^
   |
   = note: `Item` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis
   = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default

warning: field `0` is never read
  --> fixtures/lang/sorts/src/sorts.rs:84:13
   |
84 | struct Held([u8; 2]);
   |        ---- ^^^^^^^
   |        |
   |        field in this struct
   |
   = help: consider removing this field

test lang::tuples ... ok
test stock has been running for over 60 seconds
warning: `lang-sorts-fixture` (lib) generated 2 warnings
    Finished `differential` profile [optimized + debuginfo] target(s) in 19.49s
   Compiling lang-splits-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/lang/splits)
    Finished `differential` profile [optimized + debuginfo] target(s) in 18.78s
test lang::splits ... ok
test crates ... ok
   Compiling cfg-if v1.0.5
   Compiling getrandom v0.4.3
   Compiling rand_core v0.10.1
   Compiling allocator-api2 v0.2.21
   Compiling equivalent v1.0.3
   Compiling foldhash v0.2.0
   Compiling chacha20 v0.10.2
   Compiling hashbrown v0.17.1
   Compiling rand v0.10.3
    Blocking waiting for file lock on build directory
   Compiling hashlink v0.12.2
   Compiling stock-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/stock)
    Finished `differential` profile [optimized + debuginfo] target(s) in 4.37s
   Compiling share-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/share)
    Finished `differential` profile [optimized + debuginfo] target(s) in 3.65s
test lang::sorts ... ok
   Compiling share-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/share)
    Finished `differential` profile [optimized + debuginfo] target(s) in 0.26s
test stock ... ok
   Compiling share-fixture v0.0.0 (/var/cache/github-runner-work/rust2/rust2/ts/fixtures/share)
    Finished `differential` profile [optimized + debuginfo] target(s) in 0.26s
test share ... ok

failures:

---- lang::pointers stdout ----

thread 'lang::pointers' (301629) panicked at tests/fixtures.rs:110:9:
lang/pointers: pointers.ts differs from its snapshot; rerun with UPDATE_EXPECT=1
- 			const _85: rt.char = rt.u8Add(97, rt.u8Rem(n, 26)) as rt.char;
- 			const _83: rt.Place<string> = text;
- 			_83.v = _83.v + String.fromCodePoint(_85);
- 			const _91: rt.Place<string> = text;
- 			_91.v = _91.v + "!";
- 			rt.iterExtendBytes(out, rt.iterSlice(rt.byteSliceView(rt.stringAsBytes(text.v))), undefined, true);
- 		} else if (_23 === 4) {
- 			const _99: rt.Arc<undefined> = rt.arcClone(count);
+ 			const _83: rt.Place<string> = text;
+ 			_83.v = _83.v + String.fromCodePoint(rt.u8Add(97, rt.u8Rem(n, 26)) as rt.char);
+ 			const _91: rt.Place<string> = text;
+ 			_91.v = _91.v + "!";
+ 			rt.iterExtendBytes(out, rt.iterSlice(rt.byteSliceView(rt.stringAsBytes(text.v))), undefined, true);
+ 		} else if (_23 === 4) {
+ 			const _99: rt.Arc<undefined> = rt.arcClone(count);
+ 			const fresh: Held.Held = new Held.Held(n, _99);

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- lang::strs stdout ----

thread 'lang::strs' (305094) panicked at tests/fixtures.rs:110:9:
lang/strs: strs.ts differs from its snapshot; rerun with UPDATE_EXPECT=1
- 				const _47: rt.usize = offset(s, a);
- 				const _46: rt.Place<string> = rt.place(() => s, (value8) => s = value8);
- 				_46.v = rt.stringTruncate(_46.v, _47);
- 			} else if (_42 === 2) {
- 				const _51: rt.Place<string> = rt.place(() => s, (value7) => s = value7);
- 				const edit4 = rt.stringPop(_51.v);
- 				_51.v = edit4[0];
- 				putChar(out, edit4[1]);
+ 				const _46: rt.Place<string> = rt.place(() => s, (value8) => s = value8);
+ 				const _47: rt.usize = offset(s, a);
+ 				_46.v = rt.stringTruncate(_46.v, _47);
+ 			} else if (_42 === 2) {
+ 				const _51: rt.Place<string> = rt.place(() => s, (value7) => s = value7);
+ 				const edit4 = rt.stringPop(_51.v);
+ 				_51.v = edit4[0];
+ 				putChar(out, edit4[1]);



failures:
    lang::pointers
    lang::strs

test result: FAILED. 77 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 175.58s

error: test failed, to rerun pass `--test fixtures`
error: recipe `test` failed on line 31 with exit code 101
error: recipe `ci` failed on line 28 with exit code 101
##[error]Process completed with exit code 101.
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/bbf43082-5e30-41a4-9e9f-76034f18dfc0' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/rust2/rust2
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 0 objects and 0 action results (584.4 KiB logical); 50.0 GiB logical remain
dropped 2 stale checkout records
dropped 1 session event streams
Cleaning up orphan processes

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 3, 2026 05:47
@moq-bot

moq-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

codex / Codex land fixer note:

The PR changes temporary materialization, which interacts with the newer borrowed-place emission change on base. The merge left the pointers and strs generated snapshots stale. Merged base without conflicts and regenerated those two snapshots without changing compiler or runtime code. Full TypeScript tests passed, including all 79 fixture tests and native differential comparisons. Go tests, TypeScript Clippy, formatting, and the runtime type check also passed.

(written by codex / Codex)

@moq-bot

moq-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

quest land watch: armed auto-merge on this pull request at e4ed6c2, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix (codex / Codex): Merged base cleanly and regenerated two stale TypeScript snapshots; passed cargo test --locked in ts/ and go/, cargo clippy --locked --all-targets -- -D warnings and cargo fmt --all --check in ts/, and tsc -p runtime in ts/..

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 39eb6a0 Nov 3, 2026
5 checks passed
@moq-bot
moq-bot Bot deleted the quest/m3/ts-refcell-debug branch November 3, 2026 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant