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

RefCell::borrow in if let expression is not dropped until after else block #113792

Closed
froody opened this issue Jul 17, 2023 · 1 comment
Closed
Labels
C-bug Category: This is a bug.

Comments

@froody
Copy link

froody commented Jul 17, 2023

I tried this code:

use std::cell::RefCell;
use std::ops::Deref;

fn test(foo: RefCell<Option<u32>>) {
    if let Some(ref x) = foo.borrow().deref() {
        println!("got some {:?}",x);
    } else {
        println!("got none");
       *(foo.borrow_mut()) = Some(8);
    }
    println!("after {:?}", foo);
    *(foo.borrow_mut()) = Some(9);
    println!("finally {:?}", foo);
}

fn main() {
    test(RefCell::new(Some(7)));
    test(RefCell::new(None));
}

I expected to see this happen:

got some 7
after RefCell { value: Some(7) }
finally RefCell { value: Some(9) }
got none
after RefCell { value: Some(8) }
finally RefCell { value: Some(9) }

Instead, this happened:

got some 7
after RefCell { value: Some(7) }
finally RefCell { value: Some(9) }
got none
thread 'main' panicked at 'already borrowed: BorrowMutError', wat.rs:9:14

IMHO, the borrowed value should go out of scope before the body of the else branch. Trying to bind the temporary to a variable fails when using it in the else branch:

fn test(foo: RefCell<Option<u32>>) {
    if let y @ Some(ref x) = foo.borrow().deref() {
        println!("got some {:?} {:?}",x, y);
    } else {
        println!("got {}", y);
       *(foo.borrow_mut()) = Some(8);
    }
    println!("after {:?}", foo);
    *(foo.borrow_mut()) = Some(9);
    println!("finally {:?}", foo);
}

Note that the borrow checker is perfectly ok with the equivalent code not using a RefCell:

fn test(foo: &mut Option<u32>) {
    if let Some(ref x) = foo {
        println!("got some {:?}",x);
    } else {
        println!("got none");
       *foo = Some(8);
    }
    println!("after {:?}", foo);
    *foo = Some(9);
    println!("finally {:?}", foo);
}

fn main() {
    let mut tmp = Some(7);
    test(&mut tmp);
    tmp = None;
    test(&mut tmp);
}

Meta

Also repro's on 1.73.0-nightly (2023-07-16 0e8e857b11f60a785aea)

rustc --version --verbose:

rustc 1.70.0 (90c541806 2023-05-31)
binary: rustc
commit-hash: 90c541806f23a127002de5b4038be731ba1458ca
commit-date: 2023-05-31
host: aarch64-apple-darwin
release: 1.70.0
LLVM version: 16.0.2
Backtrace

got some 7
after RefCell { value: Some(7) }
finally RefCell { value: Some(9) }
got none
thread 'main' panicked at 'already borrowed: BorrowMutError', wat.rs:9:14
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:67:14
   2: core::result::unwrap_failed
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/result.rs:1687:5
   3: core::result::Result<T,E>::expect
   4: core::cell::RefCell<T>::borrow_mut
   5: wat::test
   6: wat::main
   7: core::ops::function::FnOnce::call_once

@froody froody added the C-bug Category: This is a bug. label Jul 17, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 17, 2023
@krtab
Copy link
Contributor

krtab commented Jul 17, 2023

Hi!
I think this is a duplicate of: #103108

@froody froody closed this as completed Jul 20, 2023
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 29, 2023
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.
Projects
None yet
Development

No branches or pull requests

4 participants