Skip to content

feat(ts): map a name or an operator argument to its own Rust - #549

Merged
kixelated merged 4 commits into
mainfrom
quest/m3/ts-argument-spans
Nov 3, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m3/ts-argument-spans

Conversation

@kixelated

@kixelated kixelated commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

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), ...), a tsc error on m or on the receiver a was reported at scaled or wrapping_add, and one on (a ^ b) >>> 0 at wrapping_mul.

Approach

  • operands records each argument read from a place (a name, a member, or an operator folded in from a temporary) with its own MIR Spanned span. 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.
  • Once the call terminator, or a slotted call, has made its call, locate_arguments wraps the first argument of a call or new in the result that is structurally equal to each recorded value in Located at that argument's span.
  • std's mappings and the iterator protocol (stepped_make) see the arguments bare, so no pass that matches on expression shapes sees a new wrapper. Wrapping in operands itself, as the quest suggested, broke 32 fixtures, mostly an argument a mapping passes through (into_iter of a stepped iterator), and would need every mapping audited.
  • The spans fixture gains operators, with an operator written in the arguments (a ^ 7, at / 2). Its snapshot now names m, the receivers a and v, the closure's x, c for (a ^ b) >>> 0, and each operator, at its own Rust.

No .ts snapshot changed. On moq's codec translation (moq/ts.toml, pinned rev, release build) every .ts file is byte-identical to the base's:

base this PR
.ts.map bytes 164,193 167,615 (+2.1%)
wall time, median of 7 alternating runs 14.3 s 13.3 s

The wall time is within the shared host's noise (runs spread 11.5 to 16.2 s, one 40.8 s outlier).

Impact

  • The translator's error spans and source maps are finer for name and operator arguments. No public API change, so no MIGRATE.md entry.

Alternatives

  • Wrapping each argument in operands and making every shape-matching pass look through Located (the quest's plan). It changes what std's mappings see, and 32 fixtures failed or changed; this keeps the mappings untouched instead.
  • Locating constant arguments too. The quest scopes names and operators; a literal's type error is rare, and it would add map entries for little gain.
  • An operand of a binary operator (the b in c.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 ci passed locally on 4cc95e3 (after merging origin/main at d7e0512).

Review

  • Reviewer: Codex gpt-6.1-sol, medium effort, via quest review --base origin/main.
  • Round 1 (head f4ae80d): no findings.
  • Head 4cc95e3 adds only a conflict-free merge of origin/main (#544), which needs no new review. just ci passed on it.

(written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 4 commits November 3, 2026 11:58
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 name or an operator argument at its own Rust span feat(ts): map a name or an operator argument to its own Rust Nov 3, 2026
@kixelated
kixelated marked this pull request as ready for review November 3, 2026 20:33
@kixelated
kixelated enabled auto-merge November 3, 2026 20:33
@coderabbitai

coderabbitai Bot commented Nov 3, 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 18 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: 5ebeca23-5353-49a2-a10a-7c64a4376028
Commits

Reviewing files that changed from the base of the PR and between d7e0512 and 4cc95e3.

Files selected for processing: 8
  • quest/m3/README.md
  • quest/m3/ts-argument-spans.md
  • ts/fixtures/reject/spans.toml
  • ts/fixtures/reject/spans.txt
  • ts/fixtures/reject/src/spans.rs
  • ts/src/lower/mir/call.rs
  • ts/src/lower/mir/mod.rs
  • ts/src/lower/mir/scalar.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

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ 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_arguments wraps the first argument of a Call/New structurally equal to a recording at that recording's span. peeker note: the recording happens before lend::give and std mappings reshape values, so std mappings keep seeing args bare and no output text changes.
  • ts/src/lower/mir/scalar.rs — slotted_raw locates its direct call's args the same way.
  • ts/fixtures/reject/{spans.toml,spans.txt,src/spans.rs} — new operators root (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 spans passes, so the snapshot reproduces exactly from this code.
  • cargo fmt --check on 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 already Located-wrapped by their own pass, which shields them from stealing an outer recording; self.arguments drains at every consume and is cleared at every Fx::call entry, so no stale entries reach a later locate_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)

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using glm-5.3-flash | 𝕏

Comment thread ts/src/lower/mir/call.rs
&& 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));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Merged via the queue into main with commit 464e41e Nov 3, 2026
8 checks passed
@kixelated
kixelated deleted the quest/m3/ts-argument-spans branch November 3, 2026 21:00
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