Skip to content

" change this to return FnMut instead of Fn" where the return type is a std::result::Result #125325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ambaxter opened this issue May 20, 2024 · 3 comments · Fixed by #126226
Closed
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ambaxter
Copy link

ambaxter commented May 20, 2024

I tried this code:
ambaxter/bbolt-rs@69eb00a#diff-4007187965c1fea4fd252a76f5f1007b2ea59f6d86e2f454edee547c5036be24R20

I expected to see this happen: Code compilation, or at least a sensible error.

Instead, this happened:

error[E0596]: cannot borrow `*f` as mutable, as it is a captured variable in a `Fn` closure
  --> src/util.rs:20:17
   |
14 | fn walk<F>(src: &Bolt, f: &mut F) -> crate::Result<()>
   |    ----                              ----------------- change this to return `FnMut` instead of `Fn`
...
18 |   src.view(|tx| {
   |            ---- in this closure
19 |     let tx_root = Breadcrumb::Root(b"tx".as_slice());
20 |     tx.for_each(|name, b| walk_bucket(&b, &tx_root, name, None, b.sequence(), f))
   |                 ^^^^^^^^^ cannot borrow as mutable                            - mutable borrow occurs due to use of `*f` in closure

For more information about this error, try `rustc --explain E0596`.

I tried to make a reproducer using the following code that expresses the same type and fn signatures on Rust Playground, but I'm unable to trigger it.

#![allow(dead_code)]
#![allow(unused_variables)]

use std::marker::PhantomData;

type Result<T, E = String> = std::result::Result<T, E>;

struct Bolt {}

trait BoltApi {
    fn view<'tx, F: FnMut(TxRef<'tx>) -> crate::Result<()>>(&'tx self, f: F) -> crate::Result<()>;
}

impl BoltApi for Bolt {
    fn view<'tx, F: FnMut(TxRef<'tx>) -> crate::Result<()>>(
        &'tx self,
        mut f: F,
    ) -> crate::Result<()> {
        Ok(())
    }
}

struct BucketImpl<'tx> {
    _tx: PhantomData<&'tx u64>,
}

trait BucketApi<'tx> {
    fn for_each<F: FnMut(&'tx [u8], Option<&'tx [u8]>) -> crate::Result<()>>(
        &self,
        f: F,
    ) -> crate::Result<()>;
}

impl<'tx> BucketApi<'tx> for BucketImpl<'tx> {
    fn for_each<F: FnMut(&'tx [u8], Option<&'tx [u8]>) -> crate::Result<()>>(
        &self,
        f: F,
    ) -> crate::Result<()> {
        Ok(())
    }
}

struct TxRef<'tx> {
    _tx: PhantomData<&'tx u64>,
}

trait TxApi<'tx> {
    fn for_each<F: FnMut(&[u8], BucketImpl<'tx>) -> crate::Result<()>>(
        &self,
        f: F,
    ) -> crate::Result<()>;
}

impl<'tx> TxApi<'tx> for TxRef<'tx> {
    fn for_each<F: FnMut(&[u8], BucketImpl<'tx>) -> crate::Result<()>>(
        &self,
        f: F,
    ) -> crate::Result<()>
where {
        Ok(())
    }
}

struct Tx {}

fn walk<F: FnMut(&[u8], Option<&[u8]>, u64) -> crate::Result<()>>(
    src: &Bolt,
    f: &mut F,
) -> crate::Result<()> {
    src.view(|tx| tx.for_each(|name, b| tx.for_each(|k, v| walk_bucket(&b, k, None, 0, f))))
}

fn walk_bucket<F: FnMut(&[u8], Option<&[u8]>, u64) -> crate::Result<()>>(
    b: &BucketImpl,
    key: &[u8],
    value: Option<&[u8]>,
    seq: u64,
    f: &mut F,
) -> crate::Result<()> {
    f(key, value, seq)?;
    b.for_each(|k, v| walk_bucket(b, k, v, 0, f))
}

fn main() {}

Meta

rustc --version --verbose:

% rustc --version --verbose
rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-apple-darwin
release: 1.78.0
LLVM version: 18.1.2

tested on Nightly as well.

Backtrace

 % RUST_BACKTRACE=1 cargo build
   Compiling bbolt-rs v1.3.9 (/Users/abaxter/RustroverProjects/bucket_test)
warning: unused import: `crate::common::cell::RefCell`
 --> src/common/pool.rs:1:5
  |
1 | use crate::common::cell::RefCell;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `std::rc::Rc`
 --> src/common/pool.rs:6:5
  |
6 | use std::rc::Rc;
  |     ^^^^^^^^^^^

warning: unused import: `size::Size`
 --> src/util.rs:3:5
  |
3 | use size::Size;
  |     ^^^^^^^^^^

warning: unused import: `DerefMut`
  --> src/bucket.rs:24:34
   |
24 | use std::ops::{AddAssign, Deref, DerefMut};
   |                                  ^^^^^^^^

warning: unused import: `std::io::Write`
  --> src/tx.rs:28:5
   |
28 | use std::io::Write;
   |     ^^^^^^^^^^^^^^

warning: unused variable: `size`
 --> src/util.rs:9:11
  |
9 |   let mut size = 0u64;
  |           ^^^^ help: if this is intentional, prefix it with an underscore: `_size`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `dst_tx`
  --> src/util.rs:10:11
   |
10 |   let mut dst_tx = dst.begin_rw_tx()?;
   |           ^^^^^^ help: if this is intentional, prefix it with an underscore: `_dst_tx`

warning: unused variable: `max_size`
 --> src/util.rs:7:44
  |
7 | pub fn compact(src: &Bolt, dst: &mut Bolt, max_size: u64) -> crate::Result<()> {
  |                                            ^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_max_size`

warning: variable does not need to be mutable
 --> src/util.rs:9:7
  |
9 |   let mut size = 0u64;
  |       ----^^^^
  |       |
  |       help: remove this `mut`
  |
  = note: `#[warn(unused_mut)]` on by default

warning: variable does not need to be mutable
  --> src/util.rs:10:7
   |
10 |   let mut dst_tx = dst.begin_rw_tx()?;
   |       ----^^^^^^
   |       |
   |       help: remove this `mut`

error[E0596]: cannot borrow `*f` as mutable, as it is a captured variable in a `Fn` closure
  --> src/util.rs:20:17
   |
14 | fn walk<F>(src: &Bolt, f: &mut F) -> crate::Result<()>
   |    ----                              ----------------- change this to return `FnMut` instead of `Fn`
...
18 |   src.view(|tx| {
   |            ---- in this closure
19 |     let tx_root = Breadcrumb::Root(b"tx".as_slice());
20 |     tx.for_each(|name, b| walk_bucket(&b, &tx_root, name, None, b.sequence(), f))
   |                 ^^^^^^^^^ cannot borrow as mutable                            - mutable borrow occurs due to use of `*f` in closure

For more information about this error, try `rustc --explain E0596`.
warning: `bbolt-rs` (lib) generated 10 warnings
error: could not compile `bbolt-rs` (lib) due to 1 previous error; 10 warnings emitted

@ambaxter ambaxter added the C-bug Category: This is a bug. label May 20, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 20, 2024
@pacak
Copy link
Contributor

pacak commented May 20, 2024

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=35e078a9eb6f26e1e7ea82875b355e2d - and even smaller example

@ambaxter
Copy link
Author

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=ef5f474042707ec6372bbad0595b074c - a working reproduction with no external dependencies (updated to make smaller)

Thank you!!

@jieyouxu jieyouxu added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 22, 2024
@gurry
Copy link
Contributor

gurry commented Jun 10, 2024

@rustbot claim

@bors bors closed this as completed in 9f5e2e3 Jun 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 17, 2024
Rollup merge of rust-lang#126226 - gurry:125325-improve-closure-arg-sugg, r=oli-obk

Make suggestion to change `Fn` to `FnMut` work with methods as well

Fixes rust-lang#125325

The issue occurred because the code that emitted the suggestion to change `Fn` to `FnMut` worked only for function calls  and not method calls. This PR makes it work with methods as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants