rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

clippy::let_and_return incorrectly applies when a temporary is needed to resolve borrows

Open nuttycom opened this issue 2 years ago • 4 comments

I tried this code:

    let result = stmt_select_note
        .query_and_then(
            named_params![
               ":txid": txid.as_ref(),
               ":output_index": index,
            ],
            to_spendable_note,
        )?
        .next()
        .transpose();

    result

And ran:

cargo clippy --all-features --tests --fix

I expected to see this happen:

The Clippy let_and_return lint should detect that the result temporary is necessary in order for the enclosing method to compile.

Instead, this happened:

Checking zcash_client_sqlite v0.7.1 (/home/nuttycom/work/librustzcash/work/zcash_client_sqlite)
warning: failed to automatically apply fixes suggested by rustc to crate `zcash_client_sqlite`

after fixes were automatically applied the compiler reported errors within these files:

  * zcash_client_sqlite/src/wallet/sapling.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0597]: `stmt_select_note` does not live long enough
   --> zcash_client_sqlite/src/wallet/sapling.rs:149:5
    |
149 |        stmt_select_note
    |   _____^
    |  |_____|
    | ||
150 | ||         .query_and_then(
151 | ||             named_params![
152 | ||                ":txid": txid.as_ref(),
...   ||
155 | ||             to_spendable_note,
156 | ||         )?
    | ||_________^- a temporary with access to the borrow is created here ...
    |  |_________|
    |            borrowed value does not live long enough
...
159 |    }
    |    -
    |    |
    |    `stmt_select_note` dropped here while still borrowed
    |    ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::ops::ControlFlow<std::result::Result<std::convert::Infallible, rusqlite::Error>, rusqlite::AndThenRows<'_, for<'r, 's> fn(&'r rusqlite::Row<'s>) -> std::result::Result<zcash_client_backend::wallet::ReceivedSaplingNote<ReceivedNoteId>, error::SqliteClientError> {wallet::sapling::to_spendable_note}>>`
    |
    = note: the temporary is part of an expression at the end of a block;
            consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
    |
149 ~     let x = stmt_select_note
150 |         .query_and_then(
  ...
157 |         .next()
158 ~         .transpose(); x
    |

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
Original diagnostics will follow.

warning: returning the result of a `let` binding from a block
   --> zcash_client_sqlite/src/wallet/sapling.rs:158:5
    |
147 | /     let result = stmt_select_note
148 | |         .query_and_then(
149 | |             named_params![
150 | |                ":txid": txid.as_ref(),
...   |
155 | |         .next()
156 | |         .transpose();
    | |_____________________- unnecessary `let` binding
157 |
158 |       result
    |       ^^^^^^
    |
    = note: `#[warn(clippy::let_and_return)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
    |
147 ~
148 |
149 ~     stmt_select_note
150 +         .query_and_then(
151 +             named_params![
152 +                ":txid": txid.as_ref(),
153 +                ":output_index": index,
154 +             ],
155 +             to_spendable_note,
156 +         )?
157 +         .next()
158 +         .transpose()
    |

warning: `zcash_client_sqlite` (lib test) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 16.79s

Meta

rustc --version --verbose:

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-unknown-linux-gnu
release: 1.65.0
LLVM version: 15.0.0

I also attempted to run this as cargo +nightly clippy --all-features --tests --fix and encountered the same error.

nuttycom avatar Aug 08 '23 17:08 nuttycom

For the record, you are using a version of Rust that is like half a year out of date.

This issue should be opened against the clippy repo, and a more up to date Rust release would've pointed you to that repo instead 😅

compiler-errors avatar Aug 08 '23 17:08 compiler-errors

@matthiaskrgr can you move it?

Noratrieb avatar Aug 08 '23 19:08 Noratrieb

I like to move it move it!

matthiaskrgr avatar Aug 08 '23 19:08 matthiaskrgr

I'm seeing the same issue (different code, but same error on the clippy-applied "fix") on current stable Rust (1.81) and on the latest nightly (e7c0d2750 2024-10-15).

akonradi-signal avatar Oct 16 '24 10:10 akonradi-signal