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

Polonius fails to infer lifetimes of borrows #134554

Open
zeenix opened this issue Dec 20, 2024 · 5 comments
Open

Polonius fails to infer lifetimes of borrows #134554

zeenix opened this issue Dec 20, 2024 · 5 comments
Labels
C-bug Category: This is a bug. NLL-polonius Issues related for using Polonius in the borrow checker T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@zeenix
Copy link

zeenix commented Dec 20, 2024

I tried this code:

struct Foo([u8; 10]);

impl Foo {
    fn read_non_empty<'c, P>(&'c mut self) -> P
    where
        P: serde::Deserialize<'c> + std::string::ToString,
    {
        loop {
            let b: P = self.read();
            if !b.to_string().is_empty() {
                return b;
            }
        }
    }

    fn read<'c, P>(&'c mut self) -> P
    where
        P: serde::Deserialize<'c>,
    {
        unimplemented!();
    }
}

I expected to see this happen: Compilation to succeed.

Instead, this happened: Got an error:

RUSTFLAGS='-Zpolonius' cargo +nightly c --tests
    Checking tmp-scdnga v0.1.0 (/home/zeenix/.cache/cargo-temp/tmp-sCDnga)

error[E0499]: cannot borrow `*self` as mutable more than once at a time
 --> src/main.rs:9:24
  |
4 |     fn read_non_empty<'c, P>(&'c mut self) -> P
  |                       -- lifetime `'c` defined here
...
9 |             let b: P = self.read();
  |                        ^^^^-------
  |                        |
  |                        `*self` was mutably borrowed here in the previous iteration of the loop
  |                        argument requires that `*self` is borrowed for `'c`

For more information about this error, try `rustc --explain E0499`.
error: could not compile `tmp-scdnga` (bin "tmp-scdnga" test) due to 1 previous error

If I modify the code to not use a loop, I still get the same:

    fn read_non_empty<'c, P>(&'c mut self) -> P
    where
        P: serde::Deserialize<'c> + std::string::ToString,
    {
        {
            let b: P = self.read();
            if !b.to_string().is_empty() {
                return b;
            }
        }

        {
            let b: P = self.read();
            if !b.to_string().is_empty() {
                return b;
            }
        }

        unimplemented!();
    }

Meta

rustc --version --verbose:

rustc 1.85.0-nightly (9e136a30a 2024-12-19)
binary: rustc
commit-hash: 9e136a30a965bf4e63f03095c57df7257bf96fd6
commit-date: 2024-12-19
host: x86_64-unknown-linux-gnu
release: 1.85.0-nightly
LLVM version: 19.1.6
@zeenix zeenix added the C-bug Category: This is a bug. label Dec 20, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 20, 2024
@bjorn3
Copy link
Member

bjorn3 commented Dec 20, 2024

-Zpolonius enables the legacy implementation of polonius. -Zpolonius=next enables the new from scratch implementation. Polonius next is not yet complete, but legacy polonius has several known issues that polonius next is supposed to fix AFAIK.

@bjorn3 bjorn3 added the NLL-polonius Issues related for using Polonius in the borrow checker label Dec 20, 2024
@zeenix
Copy link
Author

zeenix commented Dec 20, 2024

Polonius next is not yet complete, but legacy polonius has several known issues that polonius next is supposed to fix AFAIK.

Gotcha. Just for the record, I get an error with RUSTFLAGS='-Zpolonius=next' as well, which I guess is expected since polonius-next is incomplete.

@lqd
Copy link
Member

lqd commented Dec 20, 2024

I don't have that much time to look into it, so I may also be missing something, but at first glance the error looks correct to me.

argument requires that *self is borrowed for 'c

Your read call is an exclusive borrow for 'c, that lasts for the rest of the function since you return the value, so you can't have two self.read() calls, with or without polonius.

If this reading is correct, it kind of looks like the following, which is clearer to see why it errors

fn read_non_empty(foo: &mut u32) {
    let a = read(foo);
    let _ = read(foo);
    drop(a);
}

fn read(_: &mut u32) -> &u32 {
    unimplemented!();
}

Now if b was overwritten at each iteration of the loop, there would be no error with polonius: you'd be in #47680, which already works, for example https://rust.godbolt.org/z/hcqTGeq8e.

@zeenix
Copy link
Author

zeenix commented Dec 20, 2024

Your read call is an exclusive borrow for 'c, that lasts for the rest of the function since you return the value

But why should me returning the value, make the borrow last for the entire scope of the function? 🤔 If I return the value, nothing else in the function can access it anymore. If I do not return the value, the scope of the borrow should ensure that it's dropped (explicitly dropping doesn't help here either).

fn read_non_empty(foo: &mut u32) {
let a = read(foo);
let _ = read(foo);
drop(a);
}

but that's not my use case. I need to return the value if it satisfied a certain condition, otherwise keep reading it.

@zeenix
Copy link
Author

zeenix commented Dec 20, 2024

fn read_non_empty(foo: &mut u32) {
let a = read(foo);
let _ = read(foo);
drop(a);
}

but that's not my use case. I need to return the value if it satisfied a certain condition, otherwise keep reading it.

Ah, you meant to reduce the reproducer. Sorry, I misunderstood.

@saethlin saethlin added T-types Relevant to the types 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 Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. NLL-polonius Issues related for using Polonius in the borrow checker T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants