Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial borrowing (for fun and profit) #1215

Open
kylewlacy opened this Issue Jul 18, 2015 · 16 comments

Comments

Projects
None yet
@kylewlacy
Copy link

kylewlacy commented Jul 18, 2015

Consider the following struct and impl for 2D coordinates:

struct Point {
    x: f64,
    y: f64
}

impl Point {
    pub fn x_mut(&mut self) -> &mut f64 {
        &mut self.x
    }

    pub fn y_mut(&mut self) -> &mut f64 {
        &mut self.y
    }
}

fn main() {
    let mut point = Point { x: 1.0, y: 2.0 };
    // ...
}

Even in this simple example, using point.x and point.y gives us more flexibility than using point.mut_x() and point.mut_y(). Compare following examples that double the components of a point:

{
    // Legal:
    point.x *= 2.0;
    point.y *= 2.0;
}
{
    // Legal:
    let x_mut = &mut point.x;
    let y_mut = &mut point.y;
    *x_mut *= 2.0;
    *y_mut *= 2.0;
}
{
    // Legal:
    let ref mut x_ref = point.x;
    let ref mut y_ref = point.y;
    *x_ref *= 2.0;
    *y_ref *= 2.0;
}
{
    // Legal:
    *point.x_mut() *= 2.0;
    *point.y_mut() *= 2.0;
}
{
    // Illegal:
    let x_mut = point.x_mut();
    let y_mut = point.y_mut();
    //          ^~~~~
    // error: cannot borrow `point` as mutable more than once at a time
    *x_mut *= 2.0;
    *y_mut *= 2.0;
}

The lifetime elision rules make it pretty clear why this happens: x_mut() returns a mutable borrow that has to live at least as long as the mutable borrow of self (written as fn x_mut<'a>(&'a self) -> &'a f64).

In order to 'fix' the above example, there needs to be a way to say that a borrow only has to live as long as the mutable borrow of self.x. Here's a modification to the above impl block for a possible syntax to express partial borrows using a where clause:

impl Point {
    pub fn x_mut<'a>(&mut self)
        -> &'a f64
        where 'a: &mut self.x
    {
        &mut self.x
    }

    pub fn y_mut<'a>(&mut self)
        -> &'a f64
        where 'a: &mut self.y
    {
        &mut self.y
    }
}

While the above examples aren't particularly encouraging, allowing for this type of change could be very powerful for abstracting away implementation details. Here's a slightly better use case that builds off of the same idea of representing a Point:

struct Vec2(f64, f64) // Represents a vector with 2 coordinates
impl Vec2 {
    // General vector operations
    // ...
}

// Represents a coordinate on a 2D plane.
// Note that this change to `Point` would require no change to the above
// example that uses `Point::x_mut` and `Point::y_mut` (the details of
// the structure are effectively hidden from the client; construction could
// be moved into `Point::new` as well)
struct Point(Vec2);
impl Point {
    pub fn x_mut<'a>(&mut self)
        -> &'a mut f64
        where 'a: &mut self.0.0
    {
        &mut self.0.0
    }

    pub fn y_mut<'a>(&mut self)
        -> &'a mut f64
        where 'a: &mut self.0.1
    {
        &mut self.0.1
    }

}

For a more involved example of an API that requires mutable borrows, see this gist that describes a possible zero-cost OpenGL wrapper.

There has been some discussion about partial borrows in the past, but it hasn't really evolved into anything yet, and it seems like the idea hasn't been rejected either.

@P1start

This comment has been minimized.

Copy link
Contributor

P1start commented Jul 18, 2015

An alternate syntax (adds a new keyword (that could easily just be contextual), but in my opinion makes more sense and is clearer):

impl Point {
    pub fn x_mut(&mut self borrowing x) -> &mut f64 {
        &mut self.x
    }
}

// With a non-self parameter:
pub fn point_get_x(x: &Point borrowing x) -> &f64 {
    &x.x
}

Edit: another syntax that doesn’t require any syntactic additions to the language:

impl Point {
    pub fn x_mut(&mut Point { ref mut x, .. }: &mut Self) -> &mut f64 {
        x
    }
}

pub fn point_get_x(&Point { ref x, .. }: &Point) -> &f64 {
    x
}

Although in addition to requiring extra borrowck logic, it would require considering any ‘static’ method that has a Self, &Self, &mut Self, or Box<Self> parameter to be a method that can be called with the x.y() notation (a feature that has been proposed in the past as part of UFCS).

@kylewlacy

This comment has been minimized.

Copy link
Author

kylewlacy commented Jul 20, 2015

@P1start I think either of those syntaxes would work fine as well! But, there would need to be a way to annotate explicit lifetimes for either syntax as well; something like this:

pub fn x_mut<'a>(&mut self borrowing 'a x) -> &'a mut f64 { ... }

or this:

pub fn x_mut<'a>(&mut Point { 'a ref mut x, .. }: &mut Self) -> &mut f64 { ... }

...which would be necessary where lifetime elision would be ambiguous (such as a bare function that takes multiple &mut and returns a &mut).

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Jul 24, 2015

I want a green roof on the bikeshed:

impl Point {
    pub fn x_mut<'a>(self: &'a mut Self::x) -> &'a mut f64 {
        self.x
    }
}

for borrowing multiple fields you'd use Self::{x, y}.

Also this needs a lint that denies specifying all fields.

@glaebhoerl

This comment has been minimized.

Copy link
Contributor

glaebhoerl commented Jul 24, 2015

I don't like the idea of a type signature requiring knowledge of private fields to understand, and if the field is public then of course there's not much point. The use case of borrowing multiple fields at the same time can be addressed at a slight cost to ergonomics by just having a single method return both, for example fn x_y_mut(&mut self) -> (&mut f64, &mut f64).

@pczarn

This comment has been minimized.

Copy link

pczarn commented Aug 21, 2015

Yes, exposing names of private fields doesn't sit well. There's another way. Composite lifetimes allow separate borrows of individual parts of a type. Two mutable borrows with composite lifetimes '(a, _) and '(_, b) of a struct are possible if no field has both 'a and 'b assigned in the struct declaration. Here, _ is a free lifetime variable.
Example:
(I think the syntax Self::x is incompatible with lowercase associated types.)

struct Point<ref '(a, b)> where ref x: 'a, ref y: 'b {
    x: f64,
    y: f64
}

impl Point {
    pub fn x_mut<'a>(&'(a,) mut self) -> &'a mut f64 {
        &mut self.x
    }

    pub fn y_mut<'b>(&'(_, b) mut self) -> &'b mut f64 {
        &mut self.y
    }
}

// Typical lifetime parameters work.
struct PointRef<'a> {
    borrowed: &'a Point
}

// type signature.
fn draw<'a>(point: PointRef<'a>) {}
// equivalent type signature, for some 'c, 'd.
fn draw<'c, 'd>(point: PointRef<'(c, d)>) {}
@ticki

This comment has been minimized.

Copy link
Contributor

ticki commented Oct 18, 2015

I really, really like this proposal. I had some cases where I could choose to either rewrite the same piece of code a lot of times (because it's borrowing self) or clone the values before mutation (which is costy, especially if LLVM can't optimize it away).

However, the syntaxes proposed here seems noisy and non-obvious. The syntax should be something that's easy to spot and write.

I like the idea of @P1start's second proposal for a syntax because it seems consistent with the rust syntax. However, I find it hard to include lifetimes in that proposal, as ref does not enable annotation of lifetimes. Introducing lifetimes for ref breaks the consistency.

@glaebhoerl

This comment has been minimized.

Copy link
Contributor

glaebhoerl commented Oct 18, 2015

@ticki What does your use case look like? Does the fn x_y_mut(&mut self) -> (&mut f64, &mut f64) pattern not work?

pnkfelix added a commit to pnkfelix/pgy that referenced this issue Oct 18, 2015

Switch from `&mut self` to `&self` for the `Backend` trait.
This was to enable me to have the backend own the grammar, which was
the easiest option for me to force the grammar of the backend to match
the grammar we use during codegen. I would like to exploe alternative
options here; perhaps this RFC issue would be an avenue to explore:

  rust-lang/rfcs#1215
@aidanhs

This comment has been minimized.

Copy link
Member

aidanhs commented Nov 4, 2015

Just a vote for wanting this in some form.
It'd be nice to be able to get immutable references to the keys in a hashmap while also being able to get mutable references to the values later.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Mar 16, 2016

pub fn point_get_x(&Point { ref x, .. }: &Point) -> &f64 {

I've mentioned, e.g. pub fn point_get_x(p: &Point { x }) -> &f64 { &p.x } before, on reddit.

What makes this attractive to me is that it's refinement to include only specific fields, which would interact not only with borrowing, but many other language features, such as partial moves, ..default syntax and variant types.

@crumblingstatue

This comment has been minimized.

Copy link

crumblingstatue commented May 8, 2017

Here is some motivation fuel from a contributor to the sdl2 crate: https://cobrand.github.io/rust/sdl2/2017/05/07/the-balance-between-soundness-cost-useability.html

@burdges

This comment has been minimized.

Copy link

burdges commented May 9, 2017

I think this interacts well with #1546.

Also, there is a cute color for shed door if you permit multiple self arguments, like :

trait Foo {
    position: usize,
    slice: &[u8],
    fn tweak(&mut self.position, &'a self.slice) -> &'a [u8];
}

It does not play so nicely with non-self arguments, but maybe it could be short hand for the green roof suggested by @oli-obk. It works with some sort of destructing function call syntax too :

fn foo((ref x,ref 'a mut y) : (usize,[u8])) -> &'a [u8] { .. }
@oberien

This comment has been minimized.

Copy link

oberien commented May 13, 2017

I really would love to see this as well. But for partial self borrowing to happen I guess someone™ needs to write up an RFC, summarizing everything in this issue.

@crumblingstatue

This comment has been minimized.

Copy link

crumblingstatue commented Sep 30, 2017

Since we probably don't want to expose the names of private fields in a public API, there should be a way to declare abstract "borrow regions" for your type as part of the public API. You can then privately declare which fields belong to which region. Multiple fields can belong to one region.

Here is an (arbitrary) example:

// Note that this is just pseudo-syntax, I'm not proposing any particular syntax.
// This example uses a new keyword called `region`.

struct Vec<T> {
    /// Data belonging to the `Vec`.
    pub region data;
    /// Length of the `Vec`.
    pub region length;
    #[region(data)] ptr: *mut T,
    #[region(length)] len: i32,
}

impl<T> Vec<T> {
    // Needs mutable access to `data`, but doesn't mutate `length`.
    fn as_mut_slice(&region(mut data, length) self) -> &mut [T] {
        do_something_with(self.ptr, self.len)
    }
    // Only needs to access the `length` region.
    fn len(&region(length) self) -> usize {
        self.len
    }
    // This one borrows everything, so no need for special region annotation.
    // This means it borrows all regions.
    fn retain<F>(&mut self, f: F) where F: FnMut(&T) -> bool {
        // ...
    }
}

This would allow disjoint borrows, while still not exposing private fields in the public API or error messages.

fn main() {
    let mut v = vec![1, 2, 3];
    // This is fine because the borrow regions for `as_mut_slice` and `len` don't conflict.
    for num in v.as_mut_slice() {
        println!("{} out of {}", num, v.len());
    }
    // Not valid:
    v.retain(|num| num == v.len());
    // Error: Cannot borrow region `length` of `v` as immutable, because it is also borrowed as mutable.
    // v.retain(|&num| num == v.len());
    // -        ^^^^^^        -      - mutable borrow of `length` ends here
    // |        |             |
    // |        |             borrow occurs due to use of `v` in closure
    // |        immutable borrow of `length` occurs here
    // mutable borrow of `length` occurs here
}

Oh, and this would also work nicely for traits. The trait could declare the regions it has, and the implementing type then can map that to its own private fields.

@burdges

This comment has been minimized.

Copy link

burdges commented Oct 1, 2017

That sounds like the disjointness rules in #1546

@Rantanen

This comment has been minimized.

Copy link

Rantanen commented Dec 29, 2017

The fields in traits will resolve some of these issues. However they can't be used in case the struct/trait involves multi-field invariants, which are maintained by controlling their modification. An example of this would be data/len of Vec<T>

While the region syntax might be a bit of an overkill for this scenario, I personally like it a lot. Here is another example with it that includes traits as well.

I encountered the need in a scenario where the user impling their own type and they were holding immutable borrow for one field while wanting to call a method on self, which would have modified other disjoint fields.

struct Sphere {
    // Field belonging to two regions would allow callers to acquire two mutable
    // references to the field by using two different mutable regions.
    // Since we don't want this, we don't need to support defining fields in multiple
    // regions. This allows us to use scopes instead of attributes here.

    pub region position { x: f64, y: f64, z: f64 }
    pub region size { radius: f64 }
    pub region physics { weight : f64 }
}

impl Foo {
    // Borrows everything.
    fn copy_from(&mut self, other: &Sphere) { ... }

    // Mutably borrows position.
    fn move(&mut self borrows { &mut position }, x: f64, y: f64, z: f64) { ... }

    // Immutable borrow of size.
    fn get_size(&self borrows { &size }) -> f64 { ... }

   // Bikeshedding: If we are using 'regions', any syntax that relies on field
   // destructuring probably won't work.
}

When it comes to traits, these could be implemented using specific regions. There's no reason borrowing for Display would need to borrowck fields that are not used for display.

// Subtracting a point only alters the sphere position.
pub trait SubAssign<Point3D> for Sphere using { mut position } {

    // 'self' is valid only on fields under 'position' here.
    fn sub_assign(&mut self, rhs: Point3D) {
        self.x -= rhs.x;
        self.y -= rhs.y;
        self.z -= rhs.z;
    }
}

Also traits could possibly describe their own regions.

trait Render {
    region geometry;
    region texture;
    ...
}

// Render is implemented on Sphere using position and size.
// Borrowing 'physics' region is still valid while the sphere is being rendered.
impl Render for Sphere using { position, size } {

    // Rendered geometry inludes both position and size.
    region geometry = { position, size }

    // Sphere doesn't contain texture data. It'll use static values
    // for any method that would need textures.
    region texture = {}

    ...
}

// Acquiring reference to the trait will automatically borrow the used regions.
let r : &Render = &sphere;

// OK:
// increase_weight mut borrows only 'physics' region, which isn't used by 'Render' impl:
increase_weight( &mut sphere );

// FAIL:
// grow mut borrows physics and size. This fails, as &Render is holding borrow to size region.
grow( &mut sphere );
@Zauberklavier

This comment has been minimized.

Copy link

Zauberklavier commented Oct 4, 2018

I think regions are not a good idea.

They do not convey anything about the reasons for being laid out in a particular way and they are at best tangent to describing which parts of the struct need to be borrowable separately as opposed to which parts lay close in whatever mental model.

Overall I think the information about which part of a struct to borrow should be specified or inferred at the point at which it is borrowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.