rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.1k stars 1.49k forks source link

needless_option_as_deref false positive in struct literal #13077

Closed SimonSapin closed 1 week ago

SimonSapin commented 2 weeks ago

Summary

Clippy emits a warning but the suggested fix does not compile. This looks like another case of https://github.com/rust-lang/rust-clippy/pull/8646.

Lint Name

clippy::needless_option_as_deref

Reproducer

I tried this (somewhat minimized) code:

pub fn something(mut maybe_side_effect: Option<&mut String>) {
    for _ in 0..10 {
        let _ = S { field: other(maybe_side_effect.as_deref_mut()) };
    }
}

fn other(_maybe_side_effect: Option<&mut String>) {
    unimplemented!()
}

pub struct S { pub field: () }

Clippy emitted a warning:

    Checking playground v0.0.1 (/playground)
warning: derefed type is same as origin
 --> src/lib.rs:3:34
  |
3 |         let _ = S { field: other(maybe_side_effect.as_deref_mut()) };
  |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `maybe_side_effect`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref
  = note: `#[warn(clippy::needless_option_as_deref)]` on by default

warning: `playground` (lib) generated 1 warning (run `cargo clippy --fix --lib -p playground` to apply 1 suggestion)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.50s

The suggested fix does not compile:

error[E0382]: use of moved value: `maybe_side_effect`
 --> src/lib.rs:3:34
  |
1 | pub fn something(mut maybe_side_effect: Option<&mut String>) {
  |                  --------------------- move occurs because `maybe_side_effect` has type `Option<&mut String>`, which does not implement the `Copy` trait
2 |     for _ in 0..10 {
  |     -------------- inside of this loop
3 |         let _ = S { field: other(maybe_side_effect) };
  |                                  ^^^^^^^^^^^^^^^^^ value moved here, in previous iteration of loop
  |
note: consider changing this parameter type in function `other` to borrow instead if owning the value isn't necessary
 --> src/lib.rs:7:30
  |
7 | fn other(_maybe_side_effect: Option<&mut String>) {
  |    ----- in this function    ^^^^^^^^^^^^^^^^^^^ this parameter takes ownership of the value
help: consider moving the expression out of the loop so it is only moved once
  |
2 ~     let mut value = other(maybe_side_effect);
3 ~     for _ in 0..10 {
4 ~         let _ = S { field: value };
  |

I believe the warning should not have been emitted in the first place.

Version

rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: aarch64-apple-darwin
release: 1.79.0
LLVM version: 18.1.7

rustc 1.81.0-nightly (cc8da78a0 2024-07-04)
binary: rustc
commit-hash: cc8da78a036dc3c15c35a97651b02af9a6d30c1e
commit-date: 2024-07-04
host: aarch64-apple-darwin
release: 1.81.0-nightly
LLVM version: 18.1.7

Additional Labels

@rustbot label + I-suggestion-causes-error

rustbot commented 2 weeks ago

Error: Parsing relabel command in comment failed: ...' label +' | error: empty label at >| ' I-suggest'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

SimonSapin commented 2 weeks ago

@rustbot label +I-suggestion-causes-error

(It’s was a good suggestion from the issue template, but I couldn’t find a way to copy-paste it!)