Skip to content

New lint: needless_owned_generic_args - #17800

Open
Tunglies wants to merge 1 commit into
rust-lang:masterfrom
Tunglies:needless_owned_generic_args
Open

Tunglies wants to merge 1 commit into
rust-lang:masterfrom
Tunglies:needless_owned_generic_args

Conversation

@Tunglies

@Tunglies Tunglies commented Sep 27, 2026 •

Copy link
Copy Markdown

Adds a new restriction lint needless_owned_generic_args that detects owned,
non-Copy values passed by value to a function-level generic type parameter
when a shared reference to the value would also satisfy the callee's complete
instantiated predicate set:

fn read(path: impl AsRef<Path>) {}

let path = PathBuf::from("/tmp");
read(path);

Use instead:

read(&path);

Explicit Clone::clone calls in such argument positions are detected too
(read(path.clone()) -> read(&path)).

How it works

The lint builds &A, substitutes it for exactly the one function-level generic
argument in the callee's GenericArgs, re-instantiates all caller bounds of
the callee, and proves each with predicate_must_hold_modulo_regions (same
idiom as useless_conversion / needless_borrows_for_generic_args). It is
deliberately conservative and skips:

  • formal parameters that are not directly a type parameter (e.g. Option<T>)
  • impl/trait-level or Self parameters
  • parameters occurring more than once in the signature (shared by another
    argument, or appearing in the return type)
  • calls with explicit turbofish
  • Copy values, references, temporaries, and non-place expressions
  • types marked #[clippy::has_significant_drop] (e.g. MutexGuard)
  • .clone() calls that do not resolve to Clone::clone

It is the ownership-policy counterpart of needless_borrows_for_generic_args
(&T -> T, see #12454 / #12706): that lint only removes borrows of Copy
values, temporaries, and uniquely-used mutable references, so the two lints
cannot produce a fix-loop (covered by a .fixed UI test that recompiles
warning-free with both lint groups active). The duplicated path_has_args
helper was replaced by clippy_utils::last_path_segment.

Since impl Trait for T and impl Trait for &T may behave differently and
moving changes drop location, the lint is in the restriction group and the
suggestion is MaybeIncorrect.

Performance: candidate arguments are filtered with cheap HIR checks before any
signature/predicate queries run. Measured overhead of the lint pass is below
wall-clock noise on clippy_lints itself; the trait-solver cost is roughly
25us per emitted lint (2000 hits in ~47ms).

changelog: new lint: [needless_owned_generic_args]

LLM usage disclosure

This PR (implementation, tests, and this description) was drafted with LLM
assistance. The changes were built and verified locally before submission:
cargo test (including the new UI tests and dogfood), cargo dev fmt, and
cargo dev update_lints --check all pass.

  • Followed lint naming conventions
  • Added passing UI tests (including committed .stderr file)
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

Detects owned, non-`Copy` values passed by value to a function-level
generic type parameter when a shared reference to the value would also
satisfy the complete instantiated predicate set of the callee, e.g.

    fn read(path: impl AsRef<Path>) {}
    let path = PathBuf::from("/tmp");
    read(path); // suggest `read(&path)`

Also detects explicit `Clone::clone` calls in such argument positions,
suggesting to borrow the original value instead.

The analysis is type-aware: the lint builds `&A`, substitutes it for
exactly the one function-level generic argument in the callee's
`GenericArgs`, re-instantiates *all* caller bounds of the callee and
proves each one with `predicate_must_hold_modulo_regions`. It
conservatively skips:

* formal parameters that are not directly a type parameter (nested
  occurrences like `Option<T>`),
* impl/trait-level or `Self` parameters,
* parameters occurring more than once in the signature (shared by
  another argument or the return type),
* explicit turbofish on the callee,
* `Copy` values, references, temporaries and non-place expressions,
* types marked `#[clippy::has_significant_drop]` (e.g. `MutexGuard`),
* `.clone()` calls not resolving to `Clone::clone`.

Because trait implementations for `T` and `&T` can differ and moving
changes drop location, the lint is in the `restriction` group and the
suggestion is `MaybeIncorrect`.

It is complementary to `needless_borrows_for_generic_args` (`&T -> T`):
that lint only suggests removing a borrow for `Copy` values,
temporaries and uniquely-used mutable references, so the two cannot
produce a fix-loop. The duplicated `path_has_args` helper is replaced
by `clippy_utils::last_path_segment`.

Candidate arguments are filtered with cheap HIR checks before any
signature/predicate queries run. Measured overhead is below wall-clock
noise: ~25us per emitted lint (2000 hits in ~47ms), and no measurable
difference on `clippy_lints` itself.
@rustbot rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Sep 27, 2026
@rustbot

rustbot commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome!

You should hear from one of our reviewers after this PR gets at least 2 reviews from the community.

Please see the contribution instructions for more information.

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 27, 2026
@github-actions

Copy link
Copy Markdown

Lintcheck changes for b5217fc

Lint Added Removed Changed
clippy::needless_owned_generic_args 688 0 0

This comment will be updated if you push new changes

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants