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

spurious "borrow might be used .. when [variable] is dropped and runs the destructor" #70919

Closed
comex opened this issue Apr 8, 2020 · 3 comments · Fixed by #108780
Closed
Labels
A-borrow-checker Area: The borrow checker A-destructors Area: destructors (Drop, ..) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@comex
Copy link
Contributor

comex commented Apr 8, 2020

Playground

use std::rc::Rc;
use std::cell::{RefCell, RefMut};
use std::mem::drop;

fn huh(x: Rc<RefCell<i32>>) {
    let mut rc: Rc<RefCell<i32>> = x.clone();
    let mut inner: RefMut<'_, i32> = rc.borrow_mut();
    loop {
        drop(inner);
        drop(rc);
        rc = x.clone();
        inner = rc.borrow_mut();
    }
}

produces:

error[E0505]: cannot move out of `rc` because it is borrowed
  --> src/lib.rs:9:14
   |
6  |     let mut inner: RefMut<'_, i32> = rc.borrow_mut();
   |                                      -- borrow of `rc` occurs here
...
9  |         drop(rc);
   |              ^^ move out of `rc` occurs here
...
12 |         inner = rc.borrow_mut();
   |         ----- borrow might be used here, when `inner` is dropped and runs the destructor for type `std::cell::RefMut<'_, i32>`

error[E0506]: cannot assign to `rc` because it is borrowed
  --> src/lib.rs:11:9
   |
6  |     let mut inner: RefMut<'_, i32> = rc.borrow_mut();
   |                                      -- borrow of `rc` occurs here
...
11 |         rc = x.clone();
   |         ^^ assignment to borrowed `rc` occurs here
12 |         inner = rc.borrow_mut();
   |         ----- borrow might be used here, when `inner` is dropped and runs the destructor for type `std::cell::RefMut<'_, i32>`

(Same error with -Zpolonius.)

The error messages are wrong: the assignment to inner can never run a destructor, since the previous value of inner was dropped earlier on.

And AFAICT this code is sound and ought to be accepted.

@jonas-schievink jonas-schievink added A-borrow-checker Area: The borrow checker A-destructors Area: destructors (Drop, ..) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Apr 8, 2020
@MightyPork
Copy link

Similar issue in current rustc, my code involves std RwLock and match

        // self.conn is Option<Arc<RwLock<_>>>
        let the_ref = self.conn.as_ref().unwrap();
        let mut wg = match the_ref.write() {
            Ok(wg) => wg,
            Err(e) => {
                drop(the_ref);
                drop(e);

                self.conn = None;
                return Err(anyhow::anyhow!("conn lock is poisoned!"));
            }
        };
error[E0506]: cannot assign to `self.conn` because it is borrowed
   --> mod_opcua/src/lib.rs:303:17
    |
296 |         let the_ref = self.conn.as_ref().unwrap();
    |                       ------------------ borrow of `self.conn` occurs here
297 |         let mut wg = match the_ref.write() {
    |                            --------------- a temporary with access to the borrow is created here ...
...
303 |                 self.conn = None;
    |                 ^^^^^^^^^ assignment to borrowed `self.conn` occurs here
...
306 |         };
    |          - ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `Result<RwLockWriteGuard<'_, opcua_client::prelude::Session>, PoisonError<RwLockWriteGuard<'_, opcua_client::prelude::Session>>>`

the error makes no sense, i drop it manually right above. I can't find a way to subjugate the compiler.

@cole-miller
Copy link

cole-miller commented Jul 27, 2022

Probably the same underlying issue affects this example (playground):

enum Either<L, R> {
    Left(L),
    Right(R),
}

struct One<'a>(&'a mut [u8]);

impl<'a> One<'a> {
    fn step(self) -> Either<Two<'a>, OneSave> { unimplemented!() }
}

impl<'a> Drop for One<'a> {
    fn drop(&mut self) {}
}

struct OneSave;

impl OneSave {
    fn resume(self, _: &mut [u8]) -> One<'_> { unimplemented!() }
}

struct Two<'a>(&'a mut [u8]);

impl<'a> Two<'a> {
    fn step(self) -> One<'a> { unimplemented!() }
}

fn grow(_: &mut Vec<u8>) -> &mut [u8] { unimplemented!() }

pub fn go(stuff: &mut Vec<u8>) {
    let mut one = One(stuff);
    loop {
        let two = loop {
            one = match one.step() {
                Either::Left(x) => break x,
                Either::Right(save) => save.resume(grow(stuff)),
            };
        };
        one = two.step();
    }
}
error[E0499]: cannot borrow `*stuff` as mutable more than once at a time
  --> src/lib.rs:44:57
   |
39 |     let mut one = One(stuff);
   |                       ----- first mutable borrow occurs here
...
42 |             one = match one.step() {
   |             --- first borrow might be used here, when `one` is dropped and runs the `Drop` code for type `One`
43 |                 Either::Left(x) => break x,
44 |                 Either::Right(save) => save.resume(grow(stuff)),
   |                                                         ^^^^^ second mutable borrow occurs here

This compiles if you remove the Drop impl. I believe the error is spurious because the destructor for one never runs -- that value is consumed by the one.step() call.

@Aaron1011
Copy link
Member

Minimized:

struct WrapperWithDrop<'a>(&'a mut bool);
impl<'a> Drop for WrapperWithDrop<'a> {
    fn drop(&mut self) {}
}

fn main() {
    let mut base = true;
    let mut wrapper = WrapperWithDrop(&mut base);
    loop {
        drop(wrapper);
        
        base = false;
        wrapper = WrapperWithDrop(&mut base);
    }
}

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 6, 2023
Add regression tests for issue 70919

Desugaring DropAndReplace at MIR build (rust-lang#107844) fixed rust-lang#70919.
Add regressions tests, borrowed from rust-lang#102078, to ensure we check for this in the future.

cc `@Aaron1011`
@bors bors closed this as completed in 1866ea1 Mar 7, 2023
SimonRask added a commit to SimonRask/typst that referenced this issue Sep 2, 2023
This usage can be removed since the issue was fixed:
rust-lang/rust#70919
laurmaedje pushed a commit to typst/typst that referenced this issue Sep 4, 2023
This usage can be removed since the issue was fixed: rust-lang/rust#70919
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-destructors Area: destructors (Drop, ..) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
5 participants