Cannot borrow as mutable more than once at a time


#1

I’m struggling to understand the borrow checker error here. I’d like to be able to create vertices outside the mesh, move them into the mesh (which will own them), and then share a reference back, so I can start using it to create faces.

It seems to be the act of returning a reference in add_vertex that causes the borrow check error. But I don’t know how to tell the compiler I’m done mutating the &self mesh. I could return an index, but I’d like to understand why I can’t ‘wire up’ a new vertex to a mesh this way.

extern crate nalgebra as na;

use self::na::Point3;

#[derive(Debug)]
pub struct Mesh {
    vertices: Vec<Vertex>
}

#[derive(PartialEq, Debug)]
pub struct Vertex {
    pub position: Point3<f64>
}

impl Mesh {

    fn new() -> Mesh {
        Mesh { vertices: Vec::new() }
    }

    fn add_vertex(&mut self, vertex: Vertex) -> &Vertex {
        let idx = self.vertices.len();
        self.vertices.push(vertex); // I'm done mutating the mesh here
        &self.vertices[idx] // I'd like to be able to tell the caller, here's a ref to your vertex, which I as the Mesh own, and you can borrow
    }

}

#[test]
fn test_create_mesh() {

    let mut mesh = Mesh::new();

    let v0 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
    let v1 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
    let v2 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });

   // Use the vertex handles to define faces.

}

Yields:

error[E0499]: cannot borrow `mesh` as mutable more than once at a time
  --> src/halfedge.rs:45:14
   |
44 |     let v0 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
   |              ---- first mutable borrow occurs here
45 |     let v1 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
   |              ^^^^ second mutable borrow occurs here
...
48 | }
   | - first borrow ends here

error[E0499]: cannot borrow `mesh` as mutable more than once at a time
  --> src/halfedge.rs:46:14
   |
44 |     let v0 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
   |              ---- first mutable borrow occurs here
45 |     let v1 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
46 |     let v2 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
   |              ^^^^ second mutable borrow occurs here
47 | 
48 | }
   | - first borrow ends here

error: aborting due to 2 previous errors

I have a feeling this is down to the lifetime of the vertex reference, causing the mesh self reference to continue being borrowed despite returning from the add_vertex method.


#2

Rust has actually saved you from a memory safety issue here.

add_vertex adds an element to a vector, then returns a reference into said vector. If you call add_vector again then you’re adding another element into the vector. But consider what happens if the vector is too small to contain the new element - it may have to be reallocated, potentially moving the vector to somewhere else in memory. Now the original reference you returned is pointing to unallocated memory!


#3

(facepalm) Doh, yes I should have realised that.

Even knowing that, I find the error message somewhat confusing - it seems Rust is aware of the lifetime of v0 and somehow ties it to indexing the mesh’s vertex vector.

This works, for instance:

    {
        let v0 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
    }
    {
        let v1 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
    }

}

Presumably because each time we’re letting that reference die.


#4

I just tried this instead, adding a reference back to the mesh, so I could use the index to provide the actual vertex via the face.

extern crate nalgebra as na;

use self::na::Point3;
use std::rc::Rc;

#[derive(Debug)]
pub struct Mesh {
    vertices: Vec<Vertex>,
    faces: Vec<Face>
}

#[derive(PartialEq, Debug)]
pub struct Vertex {
    position: Point3<f64>
}

#[derive(Debug)]
pub struct Face {
    parent_mesh: &Mesh,
    vertices: Vec<usize>
}

impl Mesh {

    fn new() -> Mesh {
        Mesh { vertices: Vec::new(), faces: Vec::new() }
    }

    fn add_vertex(&mut self, vertex: Vertex) -> usize {
        let idx = self.vertices.len();
        self.vertices.push(vertex);
        idx
    }


    fn add_face(&mut self, v0: usize, v1: usize, v2: usize) {
        let mut f = Face { vertices: Vec::new(), parent_mesh: self };

        f.vertices.push(v0);
        f.vertices.push(v1);
        f.vertices.push(v2);

        self.faces.push(f);
    }

}

#[test]
fn test_create_mesh() {

    let mut mesh = Mesh::new();
    let v0 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
    let v1 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });
    let v2 = mesh.add_vertex(Vertex { position: Point3::new(0.0, 0.0, 0.0) });

    mesh.add_face(v0, v1, v2);

}

As expected, Rust complains that I need to specify the lifetime of parent_mesh: &Mesh. However, if I add a lifetime to it, I end up coming full circle, having to define a lifetime for mesh, at which point Rust says there is no such lifetime.


#5

Even knowing that, I find the error message somewhat confusing - it seems Rust is aware of the lifetime of v0 and somehow ties it to indexing the mesh’s vertex vector.

Outside add_vertex, all the information rust needs is contained in the signature. The signature

fn add_vertex(&mut self, vertex: Vertex) -> &Vertex

is shorthand for

fn add_vertex<'a>(&'a mut self, vertex: Vertex) -> &'a Vertex

meaning the mutable borrow and returned borrow are constrained to have the same lifetime. (note: The rules describing lifetime elision are actually pretty simple; they’re described here in the old rust book and a bit more verbosely somewhere here in the new book)

Of course, within add_vertex, rust does check that the implementation is valid for the given signature. Almost certainly, any way in which you could change the signature to remove that “Cannot borrow as mutable…” error in test_create_mesh would simply cause another error inside the implementation of add_vertex. (else rustc has a serious bug!)


#6

The signature of add_vertex without lifetime elision is fn add_vertex<'a>(&'a mut self, vertex: Vertex) -> &'a Vertex (see https://doc.rust-lang.org/book/lifetimes.html#lifetime-elision).

This means that the lifetime of the return value is the same as the length of the borrow of mesh (which makes sense, given what we now know about ensuring memory safety). So we’re actually reasoning about how long the borrow of mesh lasts. Within a scope that does not return anything (i.e. no let x = { a; b; c }), a borrow lasts until the end of the scope (see https://github.com/rust-lang/rfcs/issues/811). You’ve introduced scopes to restrict the lifetimes so it works fine.

Putting aside the syntax of your most recent example to correctly describe the lifetimes, you’ve got a problem here in that Rust is not great with references to parent structs. For example, what happens if you try to move the Mesh into a box, or return it to another function? All of the references in the faces will be invalid. You might want to take a look at http://stackoverflow.com/questions/20698384/lifetime-of-rust-structs-that-reference-each-other, or you may want to consider redesigning - if faces are only accessible through a method on Mesh, you don’t need to reference to the parent because you’ll already have a Mesh to hand.

It really depends what kind of interface you’re hoping to present to the user.


#7

@aidanhs one example of how I want to consume this would be fn intersections(f1: Face, f2 Face) -> Result where f1 and f2 are from separate meshes.


#8

As often, if you really want to make this work, one option is using Rc for the Mesh. This will impose a small performance cost, but…

Well, if you care about performance, you need to refactor Face entirely. If this is a typical 3D mesh, with a huge number of faces but only 3-4 vertices per face (for most faces), it’s wasteful to give each face its own Vec and corresponding allocation. Instead, consider storing an array of (start index, count) pairs corresponding to the mesh’s vertex array. Then, to make the API more ergonomic, you could add a wrapper type for temporary references to a face that includes that info plus a pointer to the Mesh. Something like:

struct Mesh {
    vertices: Vec<Vertex>,
    face_info: Vec<FaceInfo>,
}
struct FaceInfo {
    vertex_start: usize,
    vertex_count: usize,
    color: Color,
    // whatever else
}
struct FaceRef<'a> {
    mesh: &'a Mesh,
    info: &'a FaceInfo,
}
impl Mesh {
    fn get_face(&self, n: usize) -> FaceRef {
        FaceRef { mesh: self, info: &self.face_info[n] }
    }
}
impl<'a> FaceRef<'a> {
    fn vertices(&self) -> &[Vertex] {
        &self.mesh.vertices[self.info.vertex_start..self.info.vertex_start+self.info.vertex_count]
    }
    // ...
}

#9

Thanks @comex - I’d agree it’s not performant to have a vector per face. I first started with what I was familiar with - a pure graph of vertices, faces, and (half)edges. But that gave me problems with circular references - which Googling shows is a common headache in Rust, even for people that are past the beginner stage.

I then just went hacking away trying to get it to even compile :slight_smile: The mental leap I wasn’t making it seems is what you’ve shown - the temporary reference to a face in the form of FaceRef. Using that resolves the lifetime issues I was facing trying to put the mesh reference directly in what you’ve named the FaceInfo struct. It seems irritating to have to write faceRef.faceInfo, rather than just face, although perhaps I could override the reference operator ..

What I can’t decide though, still, is whether all this is a compromise to Rust, or a better way of doing things regardless of the language used. I’d not need a FaceInfo and a FaceRef in C++.

EDIT - Although I’ve just noticed I don’t think your FaceRef can work either - it has a reference to a FaceInfo that’s in a vector. That reference can disappear as well.