Repository navigation
feat(ts): map a name or an operator argument to its own Rust - #549
Conversation
Quest-Claim: 111b25c3ca2e94b8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 18 minutes. View limit details
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. CodeRabbit Verify
|
There was a problem hiding this comment.
ℹ️ No critical issues: one informational note inline.
Reviewed changes
Reviewed the PR as of 4cc95e38, reading the full diff and the affected lowering code end to end, with two read-only specialists over the argument-location mechanism:
ts/src/lower/mir/call.rs—operands()records each argument read from a place with its MIR span; after a call's value is made,locate_argumentswraps the first argument of aCall/Newstructurally equal to a recording at that recording's span.peekernote: the recording happens beforelend::giveand std mappings reshape values, so std mappings keep seeing args bare and no output text changes.ts/src/lower/mir/scalar.rs—slotted_rawlocates its direct call's args the same way.ts/fixtures/reject/{spans.toml,spans.txt,src/spans.rs}— newoperatorsroot (m.scaled(a ^ 7, at / 2)); the snapshot now reports receivers and named/operator args at their own Rust spans.quest/m3/README.md,quest/m3/ts-argument-spans.md— quest finished and removed from the index.
Verified locally on this branch:
cargo test --locked --test fixtures spanspasses, so the snapshot reproduces exactly from this code.cargo fmt --checkon the touched files is clean.- Reviewed correct at the mechanism level: no mapping emits a recorded value operand twice in a
Call/New, so first-match equality can't attach a location to the wrong occurrence; inner args of calls folded in are alreadyLocated-wrapped by their own pass, which shields them from stealing an outer recording;self.argumentsdrains at every consume and is cleared at everyFx::callentry, so no stale entries reach a laterlocate_arguments. The PR body's stated limit (same value twice locating at the first equal occurrence, map only) stands.
The moq byte-identity claim in the body was not re-measured here (needs just ts moq); the merge gate covers it, and CI had not started when this review was written.
(Written by GLM-5.3-Flash)
glm-5.3-flash | 𝕏
| && let Some(value) = values.last().filter(|v| !matches!(v, Expr::Located(..))) | ||
| && let Some(loc) = self.b.t.loc(arg.span) | ||
| { | ||
| self.arguments.push((value.clone(), loc)); |
There was a problem hiding this comment.
Recordings made here can also be dropped silently on a few flows, worth knowing even though each falls back to the whole call's span exactly as before this PR: intercept's DerefMut::deref_mut alias-slot arm records then returns Some(None) early, so that nested call's args never reach a locate_arguments; lend::give rewraps values[i] after recording so equality fails; and slotted_raw applies Expr::typed after recording, which strips an As(Str|Array|Object) wrapper. No wrong location is possible on any of these, map rows only, and all predate this PR's behavior for such args, so nothing needs to change.

Completes quest/m3/ts-argument-spans.md.
Problem
#520 maps an argument made by a call to its own Rust. An argument that is a plain name or an operator still mapped to the call it is passed to: in
Meter.scaled(m, rt.u32WrappingAdd(a, 1), ...), atscerror onmor on the receiverawas reported atscaledorwrapping_add, and one on(a ^ b) >>> 0atwrapping_mul.Approach
operandsrecords each argument read from a place (a name, a member, or an operator folded in from a temporary) with its own MIRSpannedspan. A call folded in keeps the loc feat(ts): map a tail, an argument, and what follows a call to the Rust that made them #520 gave it.locate_argumentswraps the first argument of a call ornewin the result that is structurally equal to each recorded value inLocatedat that argument's span.stepped_make) see the arguments bare, so no pass that matches on expression shapes sees a new wrapper. Wrapping inoperandsitself, as the quest suggested, broke 32 fixtures, mostly an argument a mapping passes through (into_iterof a stepped iterator), and would need every mapping audited.spansfixture gainsoperators, with an operator written in the arguments (a ^ 7,at / 2). Its snapshot now namesm, the receiversaandv, the closure'sx,cfor(a ^ b) >>> 0, and each operator, at its own Rust.No
.tssnapshot changed. On moq's codec translation (moq/ts.toml, pinned rev, release build) every.tsfile is byte-identical to the base's:.ts.mapbytesThe wall time is within the shared host's noise (runs spread 11.5 to 16.2 s, one 40.8 s outlier).
Impact
MIGRATE.mdentry.Alternatives
operandsand making every shape-matching pass look throughLocated(the quest's plan). It changes what std's mappings see, and 32 fixtures failed or changed; this keeps the mappings untouched instead.binc.wrapping_mul(a) + b) has no span of its own in MIR, so it still maps to its statement.Limit: if a mapping passes the same expression twice, the first equal argument is located first, so the two may swap locations. Only the map is affected.
Follow-ups
None.
Checks
just cipassed locally on 4cc95e3 (after merging origin/main at d7e0512).Review
gpt-6.1-sol, medium effort, viaquest review --base origin/main.just cipassed on it.(written by Claude Opus 5.5)
🤖 Generated with Claude Code