Make "set_", "init_", and "clear" methods take a "mut self"

[erickt]

This changes the mutation methods to take a &mut self. I think I got everything, but one edge case I couldn't figure out was how to deal with the list .get() method. It feels like it should be returning a &T or a &mut T, but right now it's by value that contains a 'a type.

[dwrensha]

I'm not convinced that this is the way to go. The thing is, a Builder<'a> already behaves like a &'a mut T; it's just a fancy pointer to some other memory. The fields of the Builder<'a> itself should never mutate.

The place where you do need to borrow mutably is at the root of the message, where you call MessageBuilder::init_root() to get a Builder<'a>. Unfortunately, we are currently forced to use a 'static lifetime here due to some bugs in rustc (rust-lang/rust#13853, rust-lang/rust#13645), but in theory that root borrow is what dictates the lifetimes of all of the sub-builders of that message.

[erickt]

Yikes, that bug is scary. Fortunately it was pretty easy to work around by moving the lifetime, so I filed #18. Even with that fix though I'm concerned that there are some bugs in how capnproto-rs is working with lifetimes. For example, this compiles:

test.capnp:

@0xb05abd72309922f1;

struct Test {
  test @0 :Int64;

  method @1 :Method;
  enum Method {
    foo @0;
    bar @1;
  }
}

test.rs:

extern crate capnp;

use std::task::spawn;
use capnp::message::{MessageBuilder, MallocMessageBuilder};

pub mod test_capnp;

fn create_test() -> test_capnp::test::Builder<'static> {
    let mut builder: MallocMessageBuilder = MallocMessageBuilder::new_default();
    builder.init_root::<test_capnp::test::Builder>()
}

fn create_test2<'a>(builder: &'a mut MallocMessageBuilder) -> test_capnp::test::Builder<'a> {
    builder.init_root::<test_capnp::test::Builder>()
}

fn main() {
    let test = create_test();
    let test2 = test;
    test.set_test(1);

    spawn(proc() {
        test2.set_test(2);
        println!("hi");
    });

    let mut builder = MallocMessageBuilder::new_default();
    let test = create_test2(&mut builder);
    let test2 = test;
    test.set_test(1);

    // Properly errors with: error: `builder` does not live long enough
    /*
    spawn(proc() {
        test2.set_test(2);
        println!("hi");
    });
    */
}

[dwrensha]

I think the problem here is just that init_root needs to have a signature like this:

    fn init_root<'a, T : FromStructBuilder<'a> + HasStructSize>(&'a mut self) -> T;

But until rust-lang/rust#13853 gets fixed, I don't think we can express the constraints we need to.

[erickt]

Sorry for jumping between tickets :)

I'm not sure if we need rust-lang/rust#13853 fixed. I'm pretty sure that this is equivalent to what you're doing, but it appears to be working appropriately:

trait Reader<'a> {
    fn get_as_struct<T: FromStructReader<'a>>(&'a self) -> T;
}

struct MyReader {
    x: int,
}

impl<'a> Reader<'a> for MyReader {
    fn get_as_struct<T: FromStructReader<'a>>(&'a self) -> T {
        FromStructReader::new(&self.x)
    }
}

trait FromStructReader<'a> {
    fn new(x: &'a int) -> Self;
}

struct MyStructReader<'a> {
    x: &'a int,
}

impl<'a> FromStructReader<'a> for MyStructReader<'a> {
    fn new(x: &'a int) -> MyStructReader<'a> {
        MyStructReader { x: x }
    }
}

// errors with: error: `reader` does not live long enough
/*
fn foo1() -> MyStructReader<'static> {
    let reader = MyReader { x: 5 };
    reader.get_as_struct::<MyStructReader>()
}
*/

fn foo2<'a>(reader: &'a MyReader) -> MyStructReader<'a> {
    reader.get_as_struct::<MyStructReader>()
}

fn main() {
    let reader = MyReader { x: 5 };
    let struct_reader = foo2(&reader);
}

[dwrensha]

Oh, nice. Looks like things behave as we want if I apply #18 and add a 'a to the &mut self arguments of the MessageBuilder methods.

fn init_root<T : FromStructBuilder<'a> + HasStructSize>(&'a mut self) -> T

[dwrensha]

Now every Builder<'a> is non-Copy and can be reborrowed. I think they are as close to built-in mutable references as is currently possible. See this post for details.