[kixelated]

Completes quest/m3/ts-unread-params.md.

Problem

Lowering can drop every read of a parameter, and tsc then refuses the output under noUnusedParameters. Reproduced in the lang fixture before the fix:

zeros.Lock.ts(48,31): error TS6133: 'self' is declared but its value is never read.   // drop$Drop_u32(self: Lock<rt.u32>): void {}
zeros.ts(430,24): error TS6133: 'u' is declared but its value is never read.          // unitEq(u: undefined, ...) { if (!true) ... }
zeros.ts(455,181): error TS6133: 'u' is declared but its value is never read.         // (u: undefined): boolean => { return true; }

The first is the quest's case, a generic Drop impl like crossbeam-epoch's OnceLock instantiated at a T that drops nothing. The other two are the () comparisons #303 found. #311's reads_value did not cover them: it guessed from MIR which reads lowering keeps, and saw only reads through fields that hold nothing. It also marked parameters the output still reads, such as _self in zeros.Names.ts's clone$Clone.

Approach

• ts::mark_unread decides from the emitted output. A parameter whose function body never names it becomes _name, with a name nothing in the body uses. The same applies to every arrow and lambda nested in the body. It does not matter which part of lowering dropped the reads.
• Program::mark_unread runs that pass over every function, method, field initializer, and module value once, before merge.
• feat(ts): translate std's impls of its zero-sized types #311's MIR-side reads_value and the parameter renaming in Fx are deleted.
• Audit:
  • Locals: prune_unread already drops declarations whose output nothing reads.
  • Type parameters: feat(ts): translate std's impls of its zero-sized types #311's unnamed_params already decides from the emitted field types, and class methods never name a class's type parameters.
  • Neither needed a change. The new fixture cases cover an unread local Lock<u32>, PhantomData locals, and a closure parameter.
• A body that only assigns a parameter keeps the name, since it names it. tsc counts that as unread. Optimized MIR drops dead stores, though, and no fixture emits such a body.

Tests

The new zeros::unread case in the lang fixture covers:

• a crossbeam-style Lock<T> dropped at u32 and at a type that drops
• () and PhantomData comparisons, as parameters, as closure parameters, and as locals
• an unread parameter value beside flatten's own (value) => ... callback, so only the names a nested function reads from outside it count

tsc checks its output, and it runs differentially against native wasm32. Other snapshots changed only in lang:

• zeros.Names.ts no longer marks the parameters it reads.
• hints.ts renumbers names, because an unread closure parameter now takes its base name before being marked (_i becomes _i12).

Impact

• Output only: parameter names in generated TypeScript. No Rust API change.

Alternatives

• Extend reads_value with each further way lowering drops reads (drops of what drops nothing, folded comparisons). Rejected: it would keep guessing from MIR, and it would miss the next way lowering drops a read.
• Rename at print time. Rejected: merge compares parameters, so marking before it keeps identical instances identical.

Follow-ups

• Review round 1 raised a finding in code outside this diff, from #314 (ts/src/lower/iter.rs:877). For let mut r = 0u8..=5; let n = r.by_ref().zip(0..2u8).count(); (n, r.next()), native Rust gives (2, Some(2)). The reviewer reports that the translation consumes one more left item and gives (2, Some(3)). I have not verified it. Worth a quest via /quest-plan.

Review

Codex gpt-6.1-sol, medium effort, via quest review. The latest round (3) reviewed head 0a496e8104a38469b85f4b6d7d9477512c958e05 and found nothing.

• Round 1:
  • Fixed: names bound inside a nested function counted as reads of the outer function's parameter. The pass now passes only free names outward, with block scoping, and a fixture case covers it.
  • Refuted as outside this diff: the iter.rs bounded zip fold from feat(ts): fold a zip of TrustedLen iterators by its size hint #314, listed under Follow-ups.
• Round 2: no findings.
• Round 3: no findings, after merging main (feat(ts): compare through a held type's own ne, as std's slices, tuples, and Arcs do #316) with a hand-resolved conflict in the fixture's case list.

just ci passes locally on the reviewed head.

Merge of main at 49f119b2 into 0a496e81, as 23604dfb: kept main's fmt::iters case before this PR's zeros::unread in the lang harness and regenerated expected/harness.ts. Recorded skip, too trivial to review; just ci passed on 23604dfb.

The Follow-ups' zip over-read was checked and not reproduced on 49f119b2: the translation gives native's (2, Some(2)), and a 1.4M-input sweep found no difference.

(written by Claude Opus 5.5)

🤖 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 59 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: 723f5161-ba64-4293-ade0-2c4e93fe6eaf

Commits

Reviewing files that changed from the base of the PR and between d1a076e and 0a496e8.

Files selected for processing: 15

• quest/m3/README.md
• quest/m3/ts-unread-params.md
• ts/fixtures/lang/expected/core/option.ts
• ts/fixtures/lang/expected/harness.ts
• ts/fixtures/lang/expected/hints.ts
• ts/fixtures/lang/expected/lib.ts
• ts/fixtures/lang/expected/zeros.Lock.ts
• ts/fixtures/lang/expected/zeros.Names.ts
• ts/fixtures/lang/expected/zeros.ts
• ts/fixtures/lang/src/harness.rs
• ts/fixtures/lang/src/zeros.rs
• ts/src/lower/mir/mod.rs
• ts/src/lower/mod.rs
• ts/src/program.rs
• ts/src/ts.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

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 0a496e8

No actionable bugs found in this change. Moving unread-parameter detection to emitted TypeScript is a sound replacement for the MIR-side approximation. The free-name walk handles nested function/block scopes, and fresh parameter names avoid existing bindings; the added fixtures cover erased drops, folded comparisons and nested callback names.

Verification limits: static inspection of the diff and surrounding lowering, naming and printing code only. I did not run the translator, TypeScript compiler or tests. The bounded-zip issue mentioned in the PR body belongs to unchanged code and was not re-reviewed here.

(Written by OpenAI)