Skip to content

Match and report an unused_pub item by its position, not by its key - #29

Merged
alii merged 1 commit into
scarletindustries:masterfrom
robobun:robobun/5c0dd972/judge-by-position
Sep 22, 2026
Merged

alii merged 1 commit into
scarletindustries:masterfrom
robobun:robobun/5c0dd972/judge-by-position

Conversation

@robobun

@robobun robobun commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Found through oven-sh/bun, where the mordant job runs over three targets and its baseline check passed or failed at random.

Problem

  • A run over several targets records one item once per target, and not always under one key. The definition path numbers the impl blocks of a module, so a cfg(windows) block above an item gives it {impl#10} on Windows and {impl#9} on Linux and macOS.
  • A use recorded under the Windows key does not count for the Linux record of the same item. An item that only Windows code calls is reported as unused.
  • judge removes duplicates with dedup_by on the key, which removes only neighbours. With the records of three targets in the order A, B, A nothing is removed, with A, A, B one is. The order is the order in which cargo finishes the targets, so the number of findings changes from run to run over the same code. bun counted 65 or 68 findings in one file, against a baseline of 65.

Fix

  • judge takes an item as used when a use of any of its keys was recorded. It groups the records by Def::position_key() (the crate plus the file and offset of the name), which is the same in every unit.
  • It keeps one finding per position: the one with the smallest key, so the choice does not depend on the order of the units.
  • The "item of an unused type goes with it" step still uses keys, because parent is the key that the item's own unit gave the type.
  • Test: unused_pub_knows_one_item_under_the_keys_of_two_targets, a library with a cfg(windows) impl block above another impl block, and a binary that calls into it under cfg(windows). Without the change it reports by_windows (a false positive) and reports by_nothing twice.

Checked

  • cargo fmt --check, cargo test, cargo clippy --all-targets -- -D warnings, and mordant over itself with -D warnings.
  • Over oven-sh/bun, three targets, no baseline: unused_pub reports go from 188 to 173. 18 items are no longer reported, and I checked a sample: each has a real use under one target (Tag::epoll_ctl on Linux, Tag::uv_spawn on Windows, FilePollRef::set_flag, ExtraPipe::fd). The three items that were reported two or three times are reported once. Nothing new is reported. The pairs that still share a message are separate items: bun_sys::O::NOATIME is defined at three places under different cfgs.

A run over several targets records one item once per target, and not
always under one key. The definition path numbers the `impl` blocks of a
module, so a `cfg(windows)` block above an item gives it `{impl#10}` on
Windows and `{impl#9}` on Linux and macOS.

Two things went wrong in `judge`. A use recorded under the Windows key
did not count for the Linux record of the same item, so an item that
only Windows code calls was reported. And `dedup_by` on the key removes
only neighbours: with the records of three targets in the order A, B, A
nothing was removed, with A, A, B one was. The order is the order in
which cargo finishes the targets, so the number of findings changed from
run to run over the same code: oven-sh/bun counted 65 or 68 in one file,
and its baseline job failed on the second.

An item's position, the crate plus where its name is, is the same in
every unit. `judge` now takes an item as used when a use of any of its
keys was recorded, and keeps one finding per position, the one with the
smallest key so the choice does not depend on the order of the units.
@alii
alii merged commit 00778d3 into scarletindustries:master Sep 22, 2026
2 checks passed
alii pushed a commit to oven-sh/bun that referenced this pull request Sep 22, 2026
…43810)

### Problem

- The `mordant` job passes or fails at random at the pinned `d0dca00`.
`cargo mordant` reports three associated constants of `bun_sys::Tag` two
or three times each, so the `unused_pub` count for `src/sys/lib.rs` is
65 or 68 over the same code. The baseline records 65, and the first run
on #43805 counted 68.
- The cause is in mordant. It identified an item by a key that contains
the index of its impl block. A `cfg(windows)` impl block shifts that
index on Windows only, and mordant dropped a duplicate finding only when
it sat next to its twin. The order of the three targets' records is the
order in which cargo finishes them.

### Fix

- `MORDANT_REV` moves to `00778d35391dc1a75381008754fd40726c1682f0`
(scarletindustries/mordant#29). It matches and reports an item by its
position: crate, file, offset of the name. `MORDANT_TOOLCHAIN` stays
`nightly-2026-09-01`.
- `mordant-baseline.toml` is regenerated and only goes down:
`src/sys/lib.rs` 65 to 56, and the entries for `src/io/lib.rs` (1) and
`src/spawn_sys/spawn_process.rs` (5) are gone.
- Verified: `bun scripts/rust-mordant.ts` is clean from an empty
`target/mordant`, and clean again on a warm run. The `mordant` check on
this PR is the CI proof.

### Background

- The job checks three targets in one run, so each `pub` item is
recorded up to three times. `cargo mordant` decides `unused_pub` after
the build, from all of those records.
- The baseline holds a count per lint and file. A run fails when a count
goes over.

<details><summary>Notes</summary>

What changes in the findings, measured over this tree with no baseline:

- 18 items are no longer reported. Each has a use under one target, and
the other targets' records of the same item had a different key, so the
use did not count for them. Examples: `Tag::epoll_ctl` (Linux),
`Tag::uv_spawn` (Windows), `FilePollRef::set_flag`, `ExtraPipe::fd`.
- The three `Tag` constants that were reported two or three times are
reported once.
- Six more reports appear, all in `src/sys/lib.rs`, for items that share
a name under different `cfg`s, such as `O::NOATIME`, which is defined at
three places. Each is a separate unused item. The old key was the same
for all of them, so they were merged into one finding.
- In total 188 reports become 173.

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · no src or test change; test-proof not
applicable

<!-- robobun:evidence:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants