4
votes

I have a problem with self borrowing in a match expression:

fn add_once(&'t mut self, p_name: &'t str) -> Box<Element> {
    match self.get(p_name) {
        Some(x) => Box::new(*x),
        None => self.add(p_name),
    }
}

The signature of the get() and add() functions are:

fn get(&self, p_name: &str) -> Option<&Element>
fn add(&'t mut self, p_name: &'t str) -> Box<Element>

The compiler refuses this code:

error[E0502]: cannot borrow `*self` as mutable because it is also borrowed as immutable
  --> src/main.rs:38:21
   |
36 |         match self.get(p_name) {
   |               ---- immutable borrow occurs here
37 |             Some(x) => Box::new(*x),
38 |             None => self.add(p_name),
   |                     ^^^^ mutable borrow occurs here
39 |         }
40 |     }
   |     - immutable borrow ends here

I get that, but I don't see how I can rewrite the match expression.

In a related question, it was solved by having the match return a value and then calling the function. However, that doesn't work here because the meaning of the conditional is not to choose a value but selectively execute an action.

The full code sample is below:

struct Element<'e> {
    name: &'e str, 
}

impl<'e> Element<'e> {
    fn new(p_name: &str) -> Element {
        Element { name: p_name }
    }
}

struct Top<'t> {
    list: Vec<Element<'t>>,
}

impl<'t> Top<'t> {
    fn new() -> Top<'t> {
        Top { list: vec![] }
    }

    fn get(&self, p_name: &str) -> Option<&Element> {
        for element in self.list.iter() {
            if element.name == p_name {
                return Some(element);
            }
        }
        None
    }

    fn add(&'t mut self, p_name: &'t str) -> Box<Element> {
        let new_element = Box::new(Element::new(p_name));
        self.list.push(*new_element);
        return new_element;
    }

    fn add_once(&'t mut self, p_name: &'t str) -> Box<Element> {
        match self.get(p_name) {
            Some(x) => Box::new(*x),
            None => self.add(p_name),
        }
    }
}

fn main() {
    let mut t = Top::new();
    let plop1 = t.add_once("plop1");
    let plop2 = t.add_once("plop1");
}
1
Even without the mutable error, you would still have a type-unification issue. Both branches of match should return the same type. Similarly, I am surprised that add compiles as you first push into the vector and then return; however it seems to me that pushing into the vector should invalidate (move) the content of the box... Shouldn't add return &Element and the result of add_once be &Element ?Matthieu M.
@MatthieuM., it works because Element is Copy, so *new_element returns a copy of the element.Vladimir Matveev
@VladimirMatveev: ah yes, I had forgotten Copy was not yet opt-in. Still, it seems to me this indicate a design flaw: Top seems like it should be generic, and work on non-Copy types.Matthieu M.
I'm very new to Rust, so there are probably design errors. Top should indeed work on non-Copy types.Tifauv'
@MatthieuM. Initially, I wanted add and add_once to return &Element, but couldn't get it working.Tifauv'

1 Answers

3
votes

Let's fix the design issues first. The main issue is lifetime conflation:

struct Top<'t> {
    list: Vec<Element<'t>>,
}

impl<'t> Top<'t> {
    fn add(&'t mut self, p_name: &'t str) -> Box<Element>;
    fn add_once(&'t mut self, p_name: &'t str) -> Box<Element>;
}

You assert that self should live at least as long as 't, which is itself the lifetime bound of the references it will contain. It is not actually what you need, self should live less than 't to guarantee that any &'t is still alive and kicking until self dies.

If we change this, we can painlessly return references to Element:

impl<'t> Top<'t> {
    fn add<'a>(&'a mut self, p_name: &'t str) -> &'a Element<'t>;
    fn add_once<'a>(&'a mut self, p_name: &'t str) -> &'a Element<'t>;
}

Note that the lifetime 'a of the reference to Element is different (and actually will be shorter) than the lifetime 't of its contained reference.

With that out of the way, this should fix the functions:

fn position(&self, p_name: &str) -> Option<usize> {
    self.list.iter().position(|e| e.name == p_name)
}

fn add<'a>(&'a mut self, p_name: &'t str) -> &'a Element<'t> {
    self.list.push(Element::new(p_name));
    &self.list[self.list.len() - 1]
}

fn add_once<'a>(&'a mut self, p_name: &'t str) -> &'a Element<'t> {
    if let Some(p) = self.position(p_name) {
        return &self.list[p];
    }
    self.add(p_name)
}

And position can be reused for get:

fn get<'a>(&'a self, p_name: &str) -> Option<&'a Element<'t>> {
    self.position(p_name).map(|pos| &self.list[pos])
}

I would expect the following to work in a perfect world:

fn add_once<'a>(&'a mut self, p_name: &'t str) -> &'a Element<'t> {
    match self.get(p_name) {
        Some(x) => x,
        None => self.add(p_name)
    }
}

However, I recall a discussion in which it was determined that the borrow-checker was not lax enough: the scope of the borrow induced by self.get is computed to be the entire match expression even though in the None branch the temporary cannot be accessed.

This should be fixed once "Non-Lexical Lifetimes" are incorporated in Rust, which is a work in progress.