[kixelated]

Problem

A local String that some other path assigns whole is borrowed through an rt.Place temporary. s.pop(), s.remove(0), or s.split_off(1) on it emitted a write through _N.v to a temporary that was never declared, so tsc refused the output. MIR reads the &mut once, so the translator held its rt.Place back to fold into that one read. The edit then reads it twice: once in const edit = rt.stringPop(_N.v), and again in the store _N.v = edit[0]. The first read folded the temporary away. Any other edit only got away with this when an operand between the borrow and the edit (such as offset(&s, a)) flushed the temporary first.

Approach

borrowed_place now emits the held temporaries that its path reads, since every caller reads the place and then writes it. This replaces the per-call materialize that Cow::to_mut and Option's inserts did by hand for the same reason.

The strs fixture drops #479's re-declared let s = s + ... workaround, so the local is now assigned whole. It also adds remove(0) and split_off(1) on the local: constant offsets that read nothing between the borrow and the edit. Without the fix, pop, remove(0), and split_off(1) fail tsc. With it, they match native Rust in the differential test.

Impact

• The translator's output: a borrowed place's pending temporaries are declared before the edit. In pointers, one char argument is now its own const instead of being folded into the +. That is cosmetic only.

Alternatives

• Materialize only in pop/remove/split_off. This was rejected because it is the same per-call pattern that let this bug in.
• Make &mut s an alias of s even when s is assigned twice (borrow checking keeps it put while borrowed). That would give s = ... output instead of an rt.Place. It is a wider change to alias/stable and was not needed for correctness.

Follow-ups

None.

Review

Codex gpt-6.1-sol, medium, round 1 on 93d61273 against origin/main e81934e2: 0 findings. Head 37e23672 adds only a conflict-free merge of origin/main.

🤖 Generated with Claude Code

[coderabbitai]

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 50 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: 059c8556-e9c4-4372-b0ae-85544acd1096

Commits

Reviewing files that changed from the base of the PR and between e81934e and 37e2367.

Files selected for processing: 6

• quest/m3/README.md
• quest/m3/ts-reassigned-pop.md
• ts/fixtures/lang/pointers/expected/pointers.ts
• ts/fixtures/lang/strs/expected/strs.ts
• ts/fixtures/lang/strs/src/strs.rs
• ts/src/lower/mir/call.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

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

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