Skip to content

fix(clippy): allow macro-generated double_must_use on four async_trait traits (unblocks every PR) - #104

Closed
bkataru wants to merge 1 commit into
mainfrom
pawan-86-double-must-use
Closed

bkataru wants to merge 1 commit into
mainfrom
pawan-86-double-must-use

Conversation

@bkataru

@bkataru bkataru commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Unblocks Check & Lint — four macro-generated double_must_use errors on main

main is red under clippy 1.99 with 4 × clippy::double_must_use, which fails Check & Lint and blocks every PR, including the release-gate work in #97.

The four sites

All are #[async_trait] attributes. The macro generates a boxed-future method carrying #[must_use], layered on a return type that is already #[must_use]:

site item returns
agent/backend/mod.rs:18 LlmBackend Result<LLMResponse>
agent/pool.rs:44 PoolExecutor PoolResult
coordinator/mod.rs:444 TaskRunner crate::Result<Value>
tools/mod.rs:42 Tool tool method return types

Why a must_use grep finds nothing

This is what made #86 read as stale to earlier investigation. The attribute is not in the source — it is produced by #[async_trait] expansion. A source scan for must_use returns 0 on a tree where the lint fires at all four sites. That argument is not sound evidence of absence.

The other check, a duplicated #[must_use] on a synthetic function not firing, exercises a different construct (literal duplicate attributes, which rustc's unused_attributes catches) on clippy 0.1.95.

The fix

A targeted #[allow(clippy::double_must_use)] on each trait, matching this repo's existing convention of per-item allows — cf. agent/backend/ares_backend.rs:18, tools/task.rs:325, tools/edit_tests.rs:1304. There is no workspace [lints] section and no crate-root lint level, so this is the established pattern.

20 insertions, 0 deletions. No behaviour change.

Verification — CI's exact gate, both directions

# .github/workflows/ci.yml:11,29
RUSTFLAGS: -D warnings
run: cargo clippy --workspace -- -D warnings
allows REMOVED   ->  cargo +1.99.0 clippy --workspace -- -D warnings  ->  exit 101, 4 x double_must_use
allows RESTORED  ->  cargo +1.99.0 clippy --workspace -- -D warnings  ->  error lines: 0, exit 0

Full three-step gate on the patched tree:

cargo check  --workspace              exit 0
cargo clippy --workspace -D warnings   error lines: 0, exit 0
cargo fmt    --all -- --check          exit 0

Run on 1.99.0, not local stable (1.98.0) — see below.

Why it re-broke, and why that matters more than the lint

ci.yml:19 pins dtolnay/rust-toolchain@stable and the repo has no rust-toolchain.toml, so the compiler that decides CI floats. This was green when earlier lanes checked it and went red when stable reached 1.99. The recurring "is this still real?" cycle on #86 is the real cost.

Not done here: adding a rust-toolchain.toml or changing the -D warnings policy. Whether to pin the toolchain or absorb new lints is a maintainer decision, and #87 tracks the version bump. This change only makes the current failure go away.

Fixes #86

…t traits

`Check & Lint` fails on `main` under clippy 1.99 with four
`clippy::double_must_use` errors, which blocks every PR (incl. #97).

All four are `#[async_trait]` attributes whose generated boxed future
carries `#[must_use]`, on top of a return type that is already
`#[must_use]`:

  agent/backend/mod.rs:18   LlmBackend     -> Result<LLMResponse>
  agent/pool.rs:44          PoolExecutor  -> PoolResult
  coordinator/mod.rs:444    TaskRunner     -> crate::Result<Value>
  tools/mod.rs:42           Tool

The duplication is produced by macro expansion, so there is no
source-level `#[must_use]` to remove or annotate. A `must_use` grep
finds nothing at any of these sites -- that is why the issue read as
stale.

Uses the repo's existing convention of per-item allows (cf.
agent/backend/ares_backend.rs:18, tools/task.rs:325). No behaviour
change: 20 insertions, 0 deletions.

Fixes #86
@bkataru

bkataru commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Closing as superseded — the fix is upstream, and a bump already exists

This PR adds four #[allow(clippy::double_must_use)] attributes. That is the wrong fix. The attribute clippy is complaining about is emitted by async-trait, and upstream removed it. A dependency bump clears all four sites at the source with zero suppressions.

Measured in the local cargo registry:

async-trait-0.1.89/src/expand.rs:69    must_use occurrences in src/: 1
async-trait-0.1.92                       must_use occurrences in src/: 0

0.1.89 pushes #[must_use] onto every async trait method and rewrites the return to Pin<Box<dyn Future<…>>>. 0.1.92 emits neither. Since double_must_use fires precisely because the generated method carries a #[must_use] on top of an already-#[must_use] return, removing the emitted attribute removes the duplication by construction.

This repo pins async-trait 0.1.89 (Cargo.lock), which is why it is exposed.

The bump is already proposed — four times

PR state title
#87 open fix(deps): async-trait 0.1.92 for clippy::double_must_use
#92 open build: bump async-trait 0.1.89 -> 0.1.92 for clippy::double_must_use
#94 open fix(deps): async-trait 0.1.92 for clippy::double_must_use (re-derived, …)
#95 closed deps: async-trait 0.1.89 -> 0.1.92 to drop the macro-emitted #[must_use]

Opening a fifth would add to a cluster, not to a fix.

Two corrections this PR would otherwise have shipped

  1. A false claim in a source comment. It asserted PoolResult "is already #[must_use]". It is not — pool.rs:34-42 is #[derive(Debug, Clone)] pub struct PoolResult, no such attribute. The declared return type is not what the lint inspects at all: it becomes the future's Output.
  2. Four suppressions that become dead weight. Once 0.1.92 lands, these #[allow]s mask nothing and would need removing.

Closing rather than leaving it open as an alternative, because it is not a real alternative — it is strictly worse than the bump.

Fixes #86 only via the bump, so #86 stays open until one of #87/#92/#94 merges. It also means the four-site census is still useful: it is what any of those PRs should be validated against (cargo +1.99.0 clippy --workspace -- -D warnings must go from 4 errors to 0).

Note for whoever picks the survivor: four open PRs for one dependency bump is itself the defect to clean up — pick one, close the rest, preserve the strongest verification.

@bkataru bkataru closed this Oct 6, 2026
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.

main is red on clippy::double_must_use (new stable lint vs -D warnings) — blocks every PR

1 participant