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

Unable to use match with a borrowed value which will be mutated and returned within the match statement #115390

Open
ProbstDJakob opened this issue Aug 30, 2023 · 9 comments
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ProbstDJakob
Copy link

When trying to mutate a value within a match statement which is borrowed by the statement and returned as the result of the function, the compiler fails with the error E0506.

I tried this code:

struct LazyItem {
    item: Option<String>
}

impl LazyItem {
    fn get_item(&mut self) -> &str {
        match &self.item {
            None => {
                self.item = Some("Greetings".to_string());
                let Some(content) = &self.item else { unreachable!() };
                content
            },
            Some(content) => content,
        }
    }
}

fn main() {
    let mut instance = LazyItem { item: None };
    assert_eq!(instance.get_item(), "Greetings");
}

I expected to see this happen: That the code compiles and runs successfully.

Instead, this happened: The compilation failed with the following error:

error[E0506]: cannot assign to `self.item` because it is borrowed
  --> src/main.rs:9:17
   |
6  |       fn get_item(&mut self) -> &str {
   |                   - let's call the lifetime of this reference `'1`
7  |           match &self.item {
   |           -     ---------- `self.item` is borrowed here
   |  _________|
   | |
8  | |             None => {
9  | |                 self.item = Some("Greetings".to_string());
   | |                 ^^^^^^^^^ `self.item` is assigned to here but it was already borrowed
10 | |                 let Some(content) = &self.item else { unreachable!() };
...  |
13 | |             Some(content) => content,
14 | |         }
   | |_________- returning this value requires that `self.item` is borrowed for `'1`

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

Using if syntax the code compiles and runs successfully.

Code

struct LazyItem {
    item: Option<String>
}

impl LazyItem {
    fn get_item(&mut self) -> &str {
        if let None = &self.item {
            self.item = Some("Greetings".to_string());
            let Some(content) = &self.item else { unreachable!() };
            content
        } else if let Some(content) = &self.item {
            content
        } else {
            unreachable!()
        }
    }
}

fn main() {
    let mut instance = LazyItem { item: None };
    assert_eq!(instance.get_item(), "Greetings");
}

Meta

rustc --version --verbose:

rustc 1.72.0 (5680fa18f 2023-08-23)
binary: rustc
commit-hash: 5680fa18feaa87f3ff04063800aec256c3d4b4be
commit-date: 2023-08-23
host: x86_64-unknown-linux-gnu
release: 1.72.0
LLVM version: 16.0.5

Nigthly version with the same result:

rustc 1.74.0-nightly (4e78abb43 2023-08-28)
binary: rustc
commit-hash: 4e78abb437a0478d1f42115198ee45888e5330fd
commit-date: 2023-08-28
host: x86_64-unknown-linux-gnu
release: 1.74.0-nightly
LLVM version: 17.0.0
@ProbstDJakob ProbstDJakob added the C-bug Category: This is a bug. label Aug 30, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 30, 2023
@fmease
Copy link
Member

fmease commented Sep 6, 2023

This is a known limitation of the borrow checker. It's fixed by Polonius (-Zpolonius).

@fmease fmease added A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 7, 2023
@ProbstDJakob
Copy link
Author

Thanks. I am not sure how hard it would be to achieve, but wouldn't it be good to adapt the error message in order to refer to the Polonius project?

@Kixunil
Copy link
Contributor

Kixunil commented Nov 7, 2023

I had a similar issue, using ref rather than matching on reference fixes it.

What I'm really confused about is that matching on references isn't just a syntax sugar for ref. I'd expect the compiler to desugar the code first before doing any analysis.

@ProbstDJakob
Copy link
Author

Thank you. This is much more readable/intuitive than the if statement. None the less this issue should be resolved and apparently already is in the Polonius project (not implying that you do not want this to be solved).

@Kixunil
Copy link
Contributor

Kixunil commented Nov 7, 2023

Polonius is still unstable and forcing people to use nightly is not nice, so using ref is the best option.

@ProbstDJakob
Copy link
Author

Definitely. I did not want to say to use nightly, only that a fix exists but the underlying project is still in the works.

@fmease
Copy link
Member

fmease commented Nov 7, 2023

I had a similar issue, using ref rather than matching on reference fixes it.

As far as I know, each match arm may have a different binding mode (move, ref, ref mut).

If you borrow the scrutinee (&self.item), it's borrowed for the entire match expression and therefore you can't assign to it in the None arm.

If you turn the Some(ref content) => content into Some(content) => &content, you move self.item into the match expression since the body of the match doesn't influence the binding modes I think. And thus &content creates a temporary that gets dropped too early.

@fmease fmease added the fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. label Jan 25, 2024
@fmease
Copy link
Member

fmease commented Jan 26, 2024

Also compiles successfully with -Zpolonius after the following change:

- let Some(content) = &self.item else { unreachable!() };
- content
+ self.item.as_ref().unwrap()

I could close this as a duplicate of #54663 since unreachable!() and return … are both diverging expressions. However, they're sufficiently dissimilar to warrant keeping this open, maybe 🤔

@ProbstDJakob
Copy link
Author

I am not sure if those two are similar, since the issue #54663 has a problem with the borrow checker expanding behind the return and in this issue the borrow checker borrows the value even if there is nothing to borrow (None). But I am not that into the internals of Rust and do not now if both issues do have the same cause or if None is really not borrowable.

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 C-bug Category: This is a bug. fixed-by-polonius Compiling with `-Zpolonius` fixes this issue. 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