3
pub type Data = i32;

pub struct Foo {
    data: Data,
}

impl Foo {
    pub fn data_mut(&mut self) -> &mut Data {
        &mut self.data
    }
}

pub struct Context {
    data: Data,
    foos: Vec<Foo>,
}

impl Context {
    pub fn broken(&mut self) -> &mut Data {
        // What are the lifetimes here that make this version not work?
        &mut self.foos.first_mut().unwrap().data_mut()
    }

    pub fn working(&mut self) -> &mut Data {
        &mut self.foos.first_mut().unwrap().data
    }
}

fn main() {}

(Playground)

error[E0597]: borrowed value does not live long enough
  --> src/main.rs:21:14
   |
21 |         &mut self.foos.first_mut().unwrap().data_mut()
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough
22 |     }
   |     - temporary value only lives until here
   |
note: borrowed value must be valid for the anonymous lifetime #1 defined on the method body at 19:5...
  --> src/main.rs:19:5
   |
19 | /     pub fn broken(&mut self) -> &mut Data {
20 | |         // What are the lifetimes here that make this version not work?
21 | |         &mut self.foos.first_mut().unwrap().data_mut()
22 | |     }
   | |_____^

I didn't want to have the data field public, so I tried to use a getter. I know getters are not working well in Rust, and properly encapsulated collection shouldn't have a mutable get, but this is some code I'm porting from a different language, so I'm not performing any refactoring at the moment (just porting and covering with tests). What's the lifetime issue there?

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
iwat0qs
  • 146
  • 3
  • 10
  • 1
    I find the question interesting, but I think that the code can be minimized. – Boiethios Jul 24 '18 at 16:00
  • @Boiethios Minimal reproduction: https://play.rust-lang.org/?gist=3957938b4cd94f640624bfcc8adfc2ad&version=nightly&mode=debug&edition=2018 I also thought that NLL would fix this code. – Peter Hall Jul 24 '18 at 16:26
  • 1
    I can't tell if it applies to your original larger code, but the issue in your playground samples is that you don't need the `&mut`, since `data_mut` already returns a `&mut Data`. https://play.rust-lang.org/?gist=b885337f7afd99e59ecb1bf62aaf2b3c&version=nightly&mode=debug&edition=2018 – loganfsmyth Jul 24 '18 at 18:12
  • Oh my. *Facepalm*. Yes, that worked. I was getting used to prefix field references with `&mut `, and prefixed the getter returns as well. Waiting to mark your answer as **the** answer – iwat0qs Jul 24 '18 at 18:32
  • So, as I understand now, the getter was returning a proper reference up to the caller. Prefixing the result with `&mut` caused that result to become a temporary, and the code was attempting to return a reference to this temporary, which was technically correct, because of the coercion – iwat0qs Jul 24 '18 at 18:44

1 Answers1

2

With

pub fn broken(&mut self) -> &mut Data {
    &mut self.foos.first_mut().unwrap().data_mut()
}

the core issue is that the return type of data_mut() is already a &mut Data value, so you're essentially creating a &mut &mut Data, though that will collapse. The simplest fix in your case is to drop the &mut entirely

pub fn broken(&mut self) -> &mut Data {
    self.foos.first_mut().unwrap().data_mut()
}

It would seem that by adding the &mut you're causing the borrow checker to create a temporary location and then take a reference to that location.

loganfsmyth
  • 156,129
  • 30
  • 331
  • 251