CODEWITHSUNDEEP
rustborrow-checker
Al Mamun
Al Mamun

Reputation: 1056

Why rust requires manual drop of a variable in this case?

Following code gives error if cnode isn't manually dropped after each iteration. As it's going out of scope, it should be automatically dropped, and I think there is no chance of outlive the borrowed value. But it complains with error borrowed value does not live long enough.

#[cfg(feature = "localrun")]
#[derive(Debug, PartialEq, Eq)]
pub struct TreeNode {
  pub val: i32,
  pub left: Option<Rc<RefCell<TreeNode>>>,
  pub right: Option<Rc<RefCell<TreeNode>>>,
}
#[cfg(feature = "localrun")]
impl TreeNode {
  #[inline]
  pub fn new(val: i32) -> Self {
    TreeNode {
      val,
      left: None,
      right: None
    }
  }
}

// use std::borrow::Borrow;
use std::rc::Rc;
use std::cell::RefCell;
impl Solution {
    pub fn max_depth(root: Option<Rc<RefCell<TreeNode>>>) -> i32 {
      let mut depth = 0;

      let mut vec = vec![];

      
      if let Some(vnode) = &root {
        vec.push(vnode.clone());
      }

      while !vec.is_empty() {
        let mut size = vec.len();
        while size > 0 {
          size -= 1;
          let cnode = vec.pop().unwrap();
          if let Some(lnode) = cnode.borrow().left.as_ref() {
            vec.push(lnode.clone());
          }

          if let Some(rnode) = cnode.borrow().right.as_ref() {
            vec.push(rnode.clone());
          }
          // drop(cnode); // <---- Uncommenting This fixes the error though
        }
      }
      unimplemented!()
    }
}

#[cfg(feature = "localrun")]
struct Solution{}

Edit: Full error msg:

error[E0597]: `cnode` does not live long enough
   --> src/w35_w36_easy/n104.rs:105:30
    |
105 |         if let Some(rnode) = cnode.borrow().right.as_ref() {
    |                              ^^^^^^^^^^^^^^
    |                              |
    |                              borrowed value does not live long enough
    |                              a temporary with access to the borrow is created here ...
...
109 |       }
    |       -
    |       |
    |       `cnode` dropped here while still borrowed
    |       ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `Ref<'_, TreeNode>`
    |
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
    |
107 |         };
    |          +

I just noticed the helpful error message from compiler. It says to add a semicolon after if block. Actually it fixed the error! Though I'm not sure why semicolon made the difference.

Upvotes: 1

Views: 257

Answers (1)

eggyal
eggyal

Reputation: 125925

This is actually quite interesting, but requires delving a little more deeply than you might expect.

A while loop is, like most things in Rust, an expression—which is to say that the loop itself evaluates to some value (currently it's always of unit type (), although that could change in the future) and it can therefore be used in expression position, such as on the right hand side of an assignment:

let _: () = while false {};

The block of a while loop is also an expression (albeit one that must also always be of unit type); this value comes from the final expression in the block—which, in your case, is the final if let:

let _: () = if let Some(rnode) = cnode.borrow().right.as_ref() {
    vec.push(rnode.clone());
};

The borrow of cnode continues to the end of this expression. Since the expression is evaluated as that of the while loop's block, which is evaluated as that of the while loop expression itself, it actually lives until the while loop expression is evaluated (i.e. the while loop terminates). But your borrow of cnode must not live that long, because subsequent iterations of the loop may need to borrow it again! Hence the error, and the suggestion to add a semicolon at the end of the if let expression (thus converting it into a statement and terminating the borrow of cnode before the end of the while loop block):

if let Some(rnode) = cnode.borrow().right.as_ref() {
    vec.push(rnode.clone());
};

Perhaps Rust could/should be more intelligent here, and recognise the borrow cannot possibly be required for so long, but no doubt that would add considerable additional complexity and may make it harder to introduce future changes to the language in a backwards-compatible way.

Upvotes: 3

Related Questions