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

Give a better error message for shared borrow treated as unique for purposes of lifetimes #76630

Open
bonsairobo opened this issue Sep 12, 2020 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bonsairobo
Copy link

bonsairobo commented Sep 12, 2020

I tried this code:

#[derive(Default)]
struct Foo { i: i32, j: i32 }

impl Foo {
    fn modify_and_borrow(&mut self) -> &i32 { self.j += 1; &self.i }
}

fn main() {
    let mut foo = Foo::default();
    let mut refs = Vec::new();
    for _ in 0..2 {
        refs.push(foo.modify_and_borrow());
    }
}

I expected to see this happen:

The error should say something like, "cannot borrow foo as mutable while it is already borrowed as immutable."

Instead, this happened:

error[E0499]: cannot borrow `foo` as mutable more than once at a time
  --> src/main.rs:12:19
   |
12 |         refs.push(foo.modify_and_borrow());
   |                   ^^^ mutable borrow starts here in previous iteration of loop

Meta

Playground link:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a6450c7200878a52ff8bf4466cb09f5f

@bonsairobo bonsairobo added the C-bug Category: This is a bug. label Sep 12, 2020
@jonas-schievink
Copy link
Contributor

modify_and_borrow does a mutable borrow though, so the message looks correct to me

@bonsairobo
Copy link
Author

@jonas-schievink But it doesn't mutably borrow "more than once at a time" right? The mutable borrow gets dropped before the end of each iteration.

@jonas-schievink
Copy link
Contributor

The returned &i32 mutably borrows self and is pushed onto refs

@bonsairobo
Copy link
Author

Hmm that doesn't make sense to me. How is &i32 a mutable borrow of self?

@jyn514
Copy link
Member

jyn514 commented Sep 14, 2020

@bonsairobo
Copy link
Author

@jyn514 Thank you! I didn't realize that the lifetime of the mutable borrow was being extended to the same lifetime as the immutable borrows.

I guess that makes sense, because locally inside of modify_and_borrow, the compiler just sees that you've been given a reference to self, so it must return a reference with the same lifetime. If it had the global knowledge that foo lives for the duration of the loop, then it wouldn't need to extend the lifetime of &mut self. But that is probably a hard problem for the compiler to solve ;)

I still think it would be nice if this was more clear from the compiler's error message. Maybe even if there were a tool that could desugar the code for me so I could see the explicit lifetimes.

@jyn514 jyn514 changed the title Wrong error message about multiple mutable borrows Give a better error message for shared borrow treated as unique for purposes of lifetimes Sep 15, 2020
@jyn514 jyn514 added A-lifetimes Area: Lifetimes / regions A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. labels Sep 15, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 23, 2020
…idtwco

Added better error message for shared borrow treated as unique for purposes of lifetimes

Part of Issue rust-lang#76630

r? `@jyn514`
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 24, 2020
…twco

Added better error message for shared borrow treated as unique for purposes of lifetimes

Part of Issue rust-lang#76630

r? `@jyn514`
@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
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 A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants