Skip to content
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

Add error help to "cannot borrow ... as mutable more than once at a time" error that recommends assigning to a variable first #77834

Closed
camelid opened this issue Oct 11, 2020 · 4 comments · Fixed by #83174
Assignees
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Oct 11, 2020

A newcomer to Rust may have some code that looks like this (playground):

struct Foo;

impl Foo {
  fn foo(&mut self, _: f32) -> i32 { todo!() }
  fn bar(&mut self) -> f32 { todo!() }
  fn baz(&mut self) {
    self.foo(self.bar());
  }
}

Which you would think would work, since the call to self.foo() happens after self.bar() returns, and so they have unique access to self. However, this desugars into something like this:

struct Foo;

impl Foo {
  fn foo(&mut self, _: f32) -> i32 { todo!() }
  fn bar(&mut self) -> f32 { todo!() }
  fn baz(&mut self) {
    Self::foo(&mut self, Self::bar(&mut self));
  }
}

Which, due to something else in the Rust aliasing rules that can be confusing, means that there are actually two unique borrows of self live at the same time, and produces this error:

   Compiling playground v0.0.1 (/playground)
error[E0499]: cannot borrow `*self` as mutable more than once at a time
 --> src/lib.rs:7:14
  |
7 |     self.foo(self.bar());
  |     ---- --- ^^^^ second mutable borrow occurs here
  |     |    |
  |     |    first borrow later used by call
  |     first mutable borrow occurs here

However, this error is confusing because the error is only visible when the code is desugared. Because of how the code is desugared, this code – which is functionally equivalent – compiles fine (playground):

struct Foo;

impl Foo {
  fn foo(&mut self, _: f32) -> i32 { todo!() }
  fn bar(&mut self) -> f32 { todo!() }
  fn baz(&mut self) {
    let temp = self.bar();
    self.foo(temp);
  }
}

Therefore I propose adding a help message to the error that suggests extracting the result into a temporary. Something like this:

error[E0499]: cannot borrow `*self` as mutable more than once at a time
 --> src/lib.rs:7:14
  |
7 |     self.foo(self.bar());
  |     ---- --- ^^^^ second mutable borrow occurs here
  |     |    |
  |     |    first borrow later used by call
  |     first mutable borrow occurs here
  |
  = help: try assigning `self.bar()` to a variable and then call `self.foo()` with that variable

Cc @jyn514

@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels Oct 11, 2020
@camelid
Copy link
Member Author

camelid commented Oct 11, 2020

I would like to attempt implementing this too :)

@camelid camelid self-assigned this Oct 11, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

This is basically the same as #77826 (but much more detailed, thanks :)

@camelid
Copy link
Member Author

camelid commented Oct 11, 2020

Wow, the timing is so funny :)

I thought of opening this last night, but wanted to write something more in-depth so waited until today.

@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 14, 2020
@camelid
Copy link
Member Author

camelid commented Dec 12, 2020

I'm hoping to get to this at some point soon!

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 10, 2021
Suggest using a temporary variable to fix borrowck errors

Fixes rust-lang#77834.

In Rust, nesting method calls with both require `&mut` access to `self`
produces a borrow-check error:

    error[E0499]: cannot borrow `*self` as mutable more than once at a time
     --> src/lib.rs:7:14
      |
    7 |     self.foo(self.bar());
      |     ---------^^^^^^^^^^-
      |     |    |   |
      |     |    |   second mutable borrow occurs here
      |     |    first borrow later used by call
      |     first mutable borrow occurs here

That's because Rust has a left-to-right evaluation order, and the method
receiver is passed first. Thus, the argument to the method cannot then
mutate `self`.

There's an easy solution to this error: just extract a local variable
for the inner argument:

    let tmp = self.bar();
    self.foo(tmp);

However, the error doesn't give any suggestion of how to solve the
problem. As a result, new users may assume that it's impossible to
express their code correctly and get stuck.

This commit adds a (non-structured) suggestion to extract a local
variable for the inner argument to solve the error. The suggestion uses
heuristics that eliminate most false positives, though there are a few
false negatives (cases where the suggestion should be emitted but is
not). Those other cases can be implemented in a future change.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 11, 2021
Suggest using a temporary variable to fix borrowck errors

Fixes rust-lang#77834.

In Rust, nesting method calls with both require `&mut` access to `self`
produces a borrow-check error:

    error[E0499]: cannot borrow `*self` as mutable more than once at a time
     --> src/lib.rs:7:14
      |
    7 |     self.foo(self.bar());
      |     ---------^^^^^^^^^^-
      |     |    |   |
      |     |    |   second mutable borrow occurs here
      |     |    first borrow later used by call
      |     first mutable borrow occurs here

That's because Rust has a left-to-right evaluation order, and the method
receiver is passed first. Thus, the argument to the method cannot then
mutate `self`.

There's an easy solution to this error: just extract a local variable
for the inner argument:

    let tmp = self.bar();
    self.foo(tmp);

However, the error doesn't give any suggestion of how to solve the
problem. As a result, new users may assume that it's impossible to
express their code correctly and get stuck.

This commit adds a (non-structured) suggestion to extract a local
variable for the inner argument to solve the error. The suggestion uses
heuristics that eliminate most false positives, though there are a few
false negatives (cases where the suggestion should be emitted but is
not). Those other cases can be implemented in a future change.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 11, 2021
Suggest using a temporary variable to fix borrowck errors

Fixes rust-lang#77834.

In Rust, nesting method calls with both require `&mut` access to `self`
produces a borrow-check error:

    error[E0499]: cannot borrow `*self` as mutable more than once at a time
     --> src/lib.rs:7:14
      |
    7 |     self.foo(self.bar());
      |     ---------^^^^^^^^^^-
      |     |    |   |
      |     |    |   second mutable borrow occurs here
      |     |    first borrow later used by call
      |     first mutable borrow occurs here

That's because Rust has a left-to-right evaluation order, and the method
receiver is passed first. Thus, the argument to the method cannot then
mutate `self`.

There's an easy solution to this error: just extract a local variable
for the inner argument:

    let tmp = self.bar();
    self.foo(tmp);

However, the error doesn't give any suggestion of how to solve the
problem. As a result, new users may assume that it's impossible to
express their code correctly and get stuck.

This commit adds a (non-structured) suggestion to extract a local
variable for the inner argument to solve the error. The suggestion uses
heuristics that eliminate most false positives, though there are a few
false negatives (cases where the suggestion should be emitted but is
not). Those other cases can be implemented in a future change.
@bors bors closed this as completed in 433a13b Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. 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.

3 participants