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

Improve borrow checking error in cases where + use<> could be used #130545

Closed
tmandry opened this issue Sep 19, 2024 · 4 comments · Fixed by #131186
Closed

Improve borrow checking error in cases where + use<> could be used #130545

tmandry opened this issue Sep 19, 2024 · 4 comments · Fixed by #131186
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tmandry
Copy link
Member

tmandry commented Sep 19, 2024

I'm not sure if anyone has raised this, but maybe this error reporting could be improved to also say "consider adding + use<> rather than just complaining about an immutable borrow?

error[E0502]: cannot borrow `data` as mutable because it is also borrowed as immutable

 --> src/main.rs:6:5
  |
5 |     let mut i = indices(&data);
  |                         ----- immutable borrow occurs here
6 |     data.push(4);
  |     ^^^^^^^^^^^^ mutable borrow occurs here
7 |     i.next();
  |     - immutable borrow later used here

Instead,

error[E0502]: cannot borrow `data` as mutable because it is also borrowed as immutable

 --> src/main.rs:6:5
  |
5 |     let mut i = indices(&data);
  |                         ----- immutable borrow occurs here
6 |     data.push(4);
  |     ^^^^^^^^^^^^ mutable borrow occurs here
7 |     i.next();
  |     - immutable borrow later used here

10 | fn indices<T>(
11 |     slice: &[T],
12 | ) -> impl Iterator<Item = usize> + use<> {
  |                                     ----- `impl Trait` must mention all type parameters in scope in `use<...>`

Originally posted by @bsodmike in #125836

@tmandry tmandry added the A-diagnostics Area: Messages for errors, warnings, and lints label Sep 19, 2024
@bsodmike
Copy link

Thanks for picking this up :)

@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-borrow-checker Area: The borrow checker labels Sep 19, 2024
@compiler-errors
Copy link
Member

@bsodmike: The link you shared is broken. Could you share the sample of code you provided?

@compiler-errors
Copy link
Member

nvm, I manually extracted it out of gist:

#![allow(warnings)]

fn main() {
    let mut data = vec![1, 2, 3];
    let mut i = indices(&data);
    data.push(4);
    i.next();
}

fn indices<T>(
    slice: &[T],
) -> impl Iterator<Item = usize> {
    0 .. slice.len()
}

@bsodmike
Copy link

bsodmike commented Oct 3, 2024

nvm, I manually extracted it out of gist:

#![allow(warnings)]

fn main() {
    let mut data = vec![1, 2, 3];
    let mut i = indices(&data);
    data.push(4);
    i.next();
}

fn indices<T>(
    slice: &[T],
) -> impl Iterator<Item = usize> {
    0 .. slice.len()
}

Sorry. The Rust playground is having a day off. Excellent!

bors added a commit to rust-lang-ci/rust that referenced this issue Oct 4, 2024
…rrowck, r=<try>

Try to point out when edition 2024 lifetime capture rules cause borrowck issues

Lifetime capture rules in 2024 are modified to capture more lifetimes, which sometimes lead to some non-local borrowck errors. This PR attempts to link these back together with a useful note pointing out the capture rule changes.

This is not a blocking concern, but I'd appreciate feedback (though, again, I'd like to stress that I don't want to block this PR on this): I'm worried about this note drowning in the sea of other diagnostics that borrowck emits. I was tempted to change the level of the note to `.span_warn` just so it would show up in a different color. Thoughts?

Fixes rust-lang#130545

Opening as a draft first since it's stacked on rust-lang#131183.
r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 30, 2024
…rrowck, r=estebank

Try to point out when edition 2024 lifetime capture rules cause borrowck issues

Lifetime capture rules in 2024 are modified to capture more lifetimes, which sometimes lead to some non-local borrowck errors. This PR attempts to link these back together with a useful note pointing out the capture rule changes.

This is not a blocking concern, but I'd appreciate feedback (though, again, I'd like to stress that I don't want to block this PR on this): I'm worried about this note drowning in the sea of other diagnostics that borrowck emits. I was tempted to change the level of the note to `.span_warn` just so it would show up in a different color. Thoughts?

Fixes rust-lang#130545

Opening as a draft first since it's stacked on rust-lang#131183.
r? `@ghost`
@bors bors closed this as completed in c8b8378 Oct 31, 2024
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` 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.

4 participants