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

"left.extend_from_slice(right.strip_prefix(&mut left[n..]).unwrap())" doesn't compile #110362

Open
safinaskar opened this issue Apr 15, 2023 · 7 comments
Labels
A-borrow-checker Area: The borrow checker A-lifetimes Area: Lifetimes / regions A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. 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

@safinaskar
Copy link
Contributor

Here is simplified version of my code from actual production code base:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=99a751acd113949b2a77fa251e33f955 (rustc 1.68.2)

The code doesn't compile, but the code is correct, and it should compile. Yes, I can do the same task easier, but the code is still correct.

-Zpolonius doesn't change anything, i. e. this bug is not fixed by Polonius.

@rustbot label +A-lifetimes +T-compiler +A-borrow-checker +NLL-polonius +A-NLL

@safinaskar safinaskar added the C-bug Category: This is a bug. label Apr 15, 2023
@rustbot rustbot added A-borrow-checker Area: The borrow checker A-lifetimes Area: Lifetimes / regions 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. labels Apr 15, 2023
@TadaHrd
Copy link

TadaHrd commented Apr 15, 2023

That doesn't seem to be correct

@safinaskar
Copy link
Contributor Author

@TadaHrd , what you mean? Algorithm for vector merging is wrong? Or my code violates some Rust invariants?

@quaternic
Copy link

I don't think this is a bug.

Your code: (with comments removed)

let mut left = vec![0, 1, 2];
let right = vec![1, 2, 3];
let duplicate_part = &mut left[n..];
left.extend_from_slice(right.strip_prefix(duplicate_part).unwrap());

Due to argument evaluation order, I believe the last line expands to something like:

    {
        let extend_receiver = &mut left; // (*)
        let arg = right.strip_prefix(duplicate_part).unwrap();
        extend_receiver.extend_from_slice(arg);
    }

Now the lifetime error should be obvious, as duplicate_part and extend_receiver have overlapping lifetimes, and one is exclusive. Your compiling version effectively switches those two lines, solving the issue.

(*) More precisely, the method receiver is taken as a two-phase borrow, which is why the code would compile if duplicate_part was a shared borrow instead.

@lukas-code
Copy link
Member

lukas-code commented Apr 15, 2023

@quaternic Is there a reason why two-phase doesn't allow exclusive borrows in the arguments?

For example in this code, if the lifetime of the borrow in the argument outlives the method call it would be wrong for shared borrows as well as exclusive borrows, but only the exclusive borrow is a compiler error.

struct Foo;
impl Foo {
    fn bar(&mut self, _: ()) {}
}

fn main() {
    let mut foo = Foo;
    foo.bar(drop(&foo)); // OK
    foo.bar(drop(&mut foo)); // ERROR
}

@quaternic
Copy link

The two-phase functions like a shared borrow until its activation point (when the call is actually made), and exclusive from then on. See the link for details (bottom of the page).

URLO would be a much better medium for asking questions about the language. The issue tracker really isn't the place.

@TadaHrd
Copy link

TadaHrd commented Apr 16, 2023

@safinaskar, I think you need to implement it yourself if you need special functionality.
Rust is a Turing complete programming language to my knowledge so you should be able to.

@safinaskar
Copy link
Contributor Author

@quaternic

Due to argument evaluation order

The line let extend_receiver = &mut left doesn't have any side effects, so I think the compiler should move it down despite evaluation order guarantees.

In other words, I think the compiler should try all possible evaluation order strategies, which are equivalent to order specified in the reference (from side-effects point of view), until the compiler finds evaluation order, which satisfies borrow checker.

@TadaHrd

I think you need to implement it yourself if you need special functionality

You mean writing patch for rustc? I simply want this bug to stay opened for a long time. Until someone will eventually (possibly after several years) fix it. I. e. similarly to how bug #47680 (comment) (and similar bugs) eventually led to creation of Polonius.

I don't even ask to fix this bug. Just keep this report opened

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-lifetimes Area: Lifetimes / regions A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. 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

5 participants