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

NLL: if let not detecting proper lifetime #51826

Open
Ekleog opened this issue Jun 26, 2018 · 5 comments
Open

NLL: if let not detecting proper lifetime #51826

Ekleog opened this issue Jun 26, 2018 · 5 comments
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-polonius Issues related for using Polonius in the borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Ekleog
Copy link

Ekleog commented Jun 26, 2018

Hello,

I have recently been pointed to this example:

#![feature(nll)]

fn find_best(vec: &mut Vec<i32>) -> Option<&mut i32> {
    vec.get_mut(0)
}

fn foo(vec: &mut Vec<i32>) -> &mut i32 {
    if let Some(best) = find_best(vec) {
        best
    } else {
        &mut vec[1]
    }
}

fn main() {
    let mut x = vec![1, 2, 3];
    *foo(&mut x) = 42;
}

To me, the code looks sane, and looks like it should compile with NLL enabled (as the borrow to vec ends at the end of the if block, and shouldn't propagate to the else block), yet it looks like it doesn't.

Hope that helps, and as always thank you for your work on rustc!

@matthewjasper matthewjasper added A-NLL Area: Non-lexical lifetimes (NLL) WG-compiler-nll labels Jun 27, 2018
@jonhoo
Copy link
Contributor

jonhoo commented Oct 14, 2018

Another similar example (playground):

#![feature(nll)]

struct Packet<'a> {
    b: &'a [u8],
}

impl<'a> Packet<'a> {
    fn new(b: &'a [u8]) -> Option<Self> {
        Some(Packet { b: b })
    }
}

struct PacketReader {
    bytes: Vec<u8>,
}

impl PacketReader {
    fn poll<'a>(&'a mut self) -> Packet<'a> {
        loop {
            if let Some(p) = Packet::new(&self.bytes[..]) {
                return p;
            }

            // we need to read some more
            self.bytes.truncate(0);
        }
    }
}

This produces:

error[E0502]: cannot borrow `self.bytes` as mutable because it is also borrowed as immutable
  --> src/lib.rs:25:13
   |
20 |             if let Some(p) = Packet::new(&self.bytes[..]) {
   |                                           ---------- immutable borrow occurs here
...
25 |             self.bytes.truncate(0);
   |             ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
   |
note: immutable borrowed value must be valid for the lifetime 'a as defined on the method body at 18:13...
  --> src/lib.rs:18:13
   |
18 |     fn poll<'a>(&'a mut self) -> Packet<'a> {
   |             ^^

error: aborting due to previous error

@jonhoo
Copy link
Contributor

jonhoo commented Oct 14, 2018

Actually, maybe the above example is really #51526?

@matthewjasper matthewjasper added NLL-polonius Issues related for using Polonius in the borrow checker and removed NLL-deferred labels Dec 1, 2018
@ia0
Copy link
Contributor

ia0 commented Oct 7, 2019

I think I have a similar issue, but inside the if branch, not in the else or after.

#[derive(Debug)]
struct NotCopy(u8);
struct Foo(Vec<NotCopy>);
impl Foo {
    fn iter(&self) -> impl Iterator<Item = (NotCopy, &NotCopy)> {
        self.0.iter().map(|x| (NotCopy(x.0), x))
    }
    fn iter2(&self) -> impl Iterator<Item = NotCopy> + '_ {
        self.iter().map(|(x, _)| x)
    }
    fn work(&mut self) {
        // let x = self.iter2().find(|_| true); if let Some(x) = x {
        if let Some(x) = self.iter2().find(|_| true) {
            self.0.push(x);
        }
    }
}

fn main() {
    let mut v = Foo(vec![NotCopy(0)]);
    v.work();
    println!("{:?}", v.0);
}
error[E0502]: cannot borrow `self.0` as mutable because it is also borrowed as immutable
  --> src/main.rs:15:13
   |
14 |         if let Some(x) = self.iter2().find(|_| true) {
   |                          ------------
   |                          |
   |                          immutable borrow occurs here
   |                          a temporary with access to the immutable borrow is created here ...
15 |             self.0.push(x);
   |             ^^^^^^ mutable borrow occurs here
16 |         }
17 |     }
   |     - ... and the immutable borrow might be used here, when that temporary is dropped and runs the destructor for type `impl std::iter::Iterator`
   |
   = note: The temporary is part of an expression at the end of a block. Consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped.

The workaround is to uncomment the commented line and comment the one after.

@crlf0710 crlf0710 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2020
@est31
Copy link
Member

est31 commented May 30, 2023

Not 100% sure but this might be the same as #103108 ... with #107251 as a proposed PR to address it.

@est31
Copy link
Member

est31 commented Jun 19, 2023

Just checked with a local build: #107251 does not fix this, it's therefore a different issue than #103108.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-polonius Issues related for using Polonius in the borrow checker 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

7 participants