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 value does not live long enough in if let #22449

Closed
ptal opened this issue Feb 17, 2015 · 10 comments
Closed

RefCell value does not live long enough in if let #22449

ptal opened this issue Feb 17, 2015 · 10 comments
Assignees
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ptal
Copy link

ptal commented Feb 17, 2015

#![feature(std_misc)]

use std::cell::RefCell;

fn main() {
  let b = RefCell::new(Some(5));
  if let Some(x) = b.borrow().clone() {
  }
}

And the error is:

error: `b` does not live long enough
@ptal ptal changed the title Value lives too long in if let Value lives does not live long enough in if let Feb 17, 2015
@ptal ptal changed the title Value lives does not live long enough in if let RefCellvValue lives does not live long enough in if let Feb 17, 2015
@ptal ptal changed the title RefCellvValue lives does not live long enough in if let RefCell value does not live long enough in if let Feb 17, 2015
@steveklabnik
Copy link
Member

This is not specific to if let, this happens with match as well. I don't think it's actually a bug, just a side effect of how long temporaries last.

@pnkfelix
Copy link
Member

Yes, but in this case, its a bit awkward because the problem is only arising due to our special rules for how long temporaries last for the last expression in a block.

If you turn that last expression in the block (i.e. function body) above into a statement, the code will compile fine.

Like so:

use std::cell::RefCell;

fn main() {
  let b = RefCell::new(Some(5));
  if let Some(x) = b.borrow().clone() {
    println!("x: {}", x);
  }; // <--- note semicolon here, turning the expression into a statement.
}

@ptal
Copy link
Author

ptal commented Feb 17, 2015

Also consider the fact that b is still borrowed in the if body and that it prevents any mutation... (it was my initial problem).

use std::cell::{RefCell, BorrowState};

fn main() {
  let b = RefCell::new(Some(5));
  if let Some(x) = b.borrow().clone() {
    assert_eq!(b.borrow_state(), BorrowState::Reading);
  };
}

@steveklabnik steveklabnik added the A-borrow-checker Area: The borrow checker label Feb 17, 2015
@steveklabnik
Copy link
Member

Triage: no change.

@knutaf
Copy link

knutaf commented Jan 23, 2018

I think I've hit a similar case to this. I don't know if it's the same bug becausue I'm new to Rust, but it seems like it, so I wanted to drop it in the bug notes.

struct Thing<'t> {
    data : &'t u32,
}

fn main() {
    let x = 1;
    let ans;
    {
        let s = std::cell::RefCell::new(Thing { data : &x });
        ans = s.borrow().data // adding a semicolon here fixes it
    }
}

I get:

    error[E0597]: `x` does not live long enough
  --> src\main.rs:9:57
   |
9  |         let s = std::cell::RefCell::new(Thing { data : &x });
   |                                                         ^ borrowed value does not live long enough
...
12 | }
   | - `x` dropped here while still borrowed
   |
   = note: values in a scope are dropped in the opposite order they are created

error[E0597]: `s` does not live long enough
  --> src\main.rs:10:15
   |
10 |         ans = s.borrow().data // adding a semicolon here fixes it
   |               ^ borrowed value does not live long enough
11 |     }
   |     - `s` dropped here while still borrowed
12 | }
   | - borrowed value needs to live until here

@pnkfelix
Copy link
Member

At this point I'm pretty sure the right thing is to categorize this as a duplicate of #21114

(I am currently investigating ways to address #21114 which is why I'm trying to go through some other bugs and make sure they link to it when relevant.)

@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 21, 2018
@pnkfelix pnkfelix self-assigned this Sep 21, 2018
@pnkfelix
Copy link
Member

(also, I'm not sure exactly what decision @Mark-Simulacrum tagged this for. Is it just about whether to close this as a duplicate? and/or confirm that it is a duplicate?)

@Mark-Simulacrum
Copy link
Member

I think the decision tag was "is this a bug" - that is, intentional limitation/feature or incorrect checking.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 4, 2018

After PR #54782 lands, the error diagnostic here will be:

error[E0597]: `b` does not live long enough
 --> ../../issue-22449-ref-cell-if-let.rs:7:23
  |
7 |     if let Some(_x) = b.borrow().clone() {
  |                       ^---------
  |                       |
  |                       borrowed value does not live long enough
  |                       a temporary with access to the borrow is created here ...
8 |     }
9 | }
  | -
  | |
  | `b` dropped here while still borrowed
  | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::cell::Ref<'_, std::option::Option<i32>>`
  |
  = 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 reason the compiler cannot insert the semi-colon automatically: The language has a set of rules determining when temporaries are dropped. Adding the semi-colon changes the relative order of destruction, and thus as a policy that (semantically significant) change should be reflected in the source code.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 4, 2018

My main point is that this appears to be a duplicate of #21114 or perhaps #46413 (which should probably be themselves merged as duplicates once NLL is the default after the 2018 edition ships).

The problem here is a consequence of our rules for when r-value temporaries are dropped. My current inclination is to try to improve our diagnostics and documentation so that developers understand the rules and their implications, but not actually change the rules themselves.

Thus, closing as duplicate.

@pnkfelix pnkfelix closed this as completed Oct 4, 2018
@pnkfelix pnkfelix removed the I-needs-decision Issue: In need of a decision. label Oct 4, 2018
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 C-bug Category: This is a bug. 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