Accepting nested method calls with an `&mut self` receiver

language design
21

I think “pure deref” case is important: in particular because of interactions with borrowck it is impossible to do “perfect forwarding” in Rust now.

On the other hand, “pure deref” does seem orthogonal to “upgradeable borrow”. I support solving “upgradeable borrow” first. One step at a time.

Among concrete proposals, I like proposal 2 better. Even if I didn’t, as I understand proposal 2 is the more conservative one?

As for implementation, I support desugaring and hacking this into current borrowck instead of waiting on MIR borrowck, but then it is unlikely I will get to implement this, so there is not much reason you should listen to me here.

1 Like
22

My strong personal impression is that Rust has so many “I have to refactor this perfectly good code because the borrow checker is too dumb to realize it’s safe” moments, it leaves a bad taste to add “I have to refactor this because the borrow checker is opinionated about the design”, if at all avoidable. So I prefer option 1.

Besides, I think something like ‘borrowing for later’ is definitely the future, and I don’t think the no-mutation rule generalizes well to that. The rule would have to be that you can access a variable between when you take a mutable reference to it and when you use the reference, but only immutably; and that would block a lot of code that isn’t nearly as crazy as in the present case. (Consider the case where the reference is stored in some other structure.)

5 Likes
23

Of course. Tracking when you mutate through that reference becomes the hard part.

24

Sure enough.

And as we're talking about 2-phase lifetimes, let me propose something that should "fix" this (with a change to Index):

  1. Add the possibility of "not-yet-active" lifetimes (ATM every lifetime has been active at some point). Find some hopefully-unhacky way of integrating this with lifetime inference.
  2. Add a Ref2Φ<'immut, 'mutbl, T> where T: 'immut, 'immut: 'mutbl reference type. Note that
&'a T ~ Ref2Φ<'a, 'empty, T>
&'a mut T ~ Ref2Φ<'a, 'a, T>

Where the equivalence/coercion/whatever can't be made to an eqty because you can't dispatch on lifetimes, but they should be at least coercions. 3) Make all borrows biphase. A borrowck conflict happens when the immutable segment of one Ref2Φ conflicts with the mutable segment of another. Reborrowing works like usual. 4) PROFIT:

Vec::push(&mut v, Vec::len(&v)) // this works regardless of the desugaring
Vec::deref_mut(&mut v)[0] = Vec::len(&v); // still does not work
  1. BONUS: make Ref2Φ and NYA lifetimes user-accessible (bikeshed about the syntax until you get tired), and define:
trait Deref2Φ : Deref {
    fn deref2Φ<'immut, #[may_dangle] 'mutbl>(self: Ref2Φ<'immut, 'mutbl, Self>)
        -> Ref2Φ<'immut, 'mutbl, Self::Target>;
}

trait Index2Φ<Idx: ?Sized> : Index<Idx> {
    fn index2Φ<'immut, #[may_dangle] 'mutbl>(self: Ref2Φ<'immut, 'mutbl, Self>,
                                             index: Idx)
        -> Ref2Φ<'immut, 'mutbl, Self::Output>;
}
  1. BONUS: profit even more, because now you can have a single unified Index/IndexMut implementation (for backwards compat, forward both to Index2Φ) and "everything" works.
1 Like
25

Interesting. This seems like an extension on 'borrowing for the future' to me, one which exposes the two (overlapping) lifetimes in the type (Ref2, as you called it) and thus allows us to build abstractions that mirror those lifetimes further up.

26

Actually, I think it's more friendly for that, as I mentioned in the text. In particular, since it uses an already existing set of restrictions (sharing), it lets you think of "borrowing for the future" as starting out with a shared borrow and "upgrading" it to a mutable borrow when you first use it (or, alternatively, as having two overlapping lifetimes, one that starts later, which is how @arielb1 described it below (or, alternatively, having a notion of a "reservation" -- that is, when you do an &mut foo in lifetime 'x, you register that foo is borrowed during 'x -- if 'x hasn't started yet, it is "reserved" which means we treat it like a shared borrow)). Anyway TL;DR having fewer sets of rules seems easier.

27

First, that's Ref2Φ with a Φ - that makes it bad syntax :-).

Second, my first intent was to formalize the thing within the borrow checker first, and only expose it to the user afterwards.

28

I really like the Ref2Φ idea. It preserves the property that once borrowed, an object cannot be mutated, which I like. Moreover, it seems to capture exactly the sort of borrowing behavior I wanted from streaming iterators:

trait StreamingIterator {
    type Item<'a>;

    fn next<'immut, /* N.B.: no #[may_dangle] */ 'mubl>(
        self: Ref2Φ<'immut, 'mutbl, Self>
    ) -> Item<'immut>;
}

This would allow, e.g., doing a nice iteration through a file while still being able to access the file properties (path, size, etc.) in the middle of iteration.

However, instead of using “not-yet-active” lifetimes, it might be cleaner to just use arbitrary sets of program points (not necessarily contiguous) for the mutable section. With a lifetime, a reference would be able to be changed from immutable to mutable to immutable, but not anything more complex than that. This seems like an odd choice, and it lacks symmetry - labeling the immutable part of the borrow would allow a completely different set of patterns.

Essentially, your initial Ref2Φ proposal allows

let v_ref = &mut v;
Vec::push(v_ref, Vec::len(&v));

and

let v_ref = &mut v;
Vec::push(v_ref, 3);
Vec::push(v_ref, 5);

but disallows

let v_ref = &mut v;
Vec::push(v_ref, Vec::len(&v));
Vec::push(v_ref, 5);

which seems strange to me.

Another way to look at this is as a generalized notion of the mutability part of a reference. A reference currently has three parts - a lifetime, a mutability (mut vs. not), and a type. This would just change the mutability part from a binary choice to a binary choice at every program point. Therefore, a change like this could be initially exposed to the user by just being generic over mutability (which has already been RFCed).

29

These notions of “stable lvalues” and “borrowing for the future” seem very relevant to the recent coroutine proposals, since a coroutine’s state has to be borrowed on suspension until it gets resumed, and due to potential implicit self-borrowing that state has to be unmoveable until the coroutine finally returns.

2 Likes
30

Spin-off thread: [blog post] Nested method calls via two-phase borrowing

31

What are the problems with first desurgaring everything to UFCS, then at each level evaluating in an order: borrow then mutable borrow/move?

vec.push(vec.len()) desugars to Vec::push(&mut vec, vec.len()). vec.len() is a borrow so it is evaluated first (read: desugared like proposal 1), then it continues.

Optionally, you can make it depth-first with respect to nested calls which accomplishes the same thing by having the desugared Vec::len(vec) evaluated first.

Note: I am being imprecise because I’m on my phone. A literal desurgaring into UFCS first would make the semantics for things like x.mut_something().bar(y.only_borrow()) awkward since bar(foo(&mut x), baz(&y)) would force evaluation of baz first, which is not what we want. I think, if my understanding of stable lvalues was correct, we only would any of these rules when there is a stable lvalue on the left? EDIT: the perils of writing on your phone. Stable path, not stable lvalue. But basically we only want to initiate these desurgaring when the left hand side is a variable? I don’t have time to come up with a good definition right this second.

32

This sounds sort-of similar to the evaluation order that @arielb1 proposed. I left some thoughts on their proposal here.

33

I did draw inspiration from that, but mine has different drawbacks. Mine supports UFCS and I propose that it would not come into play for indices, etc. The exact mechanism by which we avoid initiating my rules are as of yet unspecified until it is determined the rest of the idea isnt horribly flawed.

34

I don’t understand the desire to prevent receiver from being modified by the arguments.

To me it’s normal than arguments are evaluated before the function call, and so it’s normal that arguments are evaluated before a method call too.

For me x.add(x.increment(1)) seems entirely reasonable, and I’d expect it to always be identical to

let tmp = x.increment(1); 
x.add(tmp);
2 Likes
35

I keep thinking ‘nested method calls with an &mut self receiver’ means something completely different:

trait Foo {
    fn foo(&mut self);
    fn bar(&mut self);
}
impl Foo for () {
    fn foo(&mut self) { println!("()::foo"); }
    fn bar(&mut self) { println!("()::bar"); self.foo(); }
    //                                       ^^^^^^^^^^ this here 
}

Since (I’m pretty sure) the above is already OK, can anyone think of a more descriptive term for the feature actually under discussion?

36

Edit: I’m coming to the realization this was proposal 1. So I’ll just say I’m in support of proposal 1.

Perhaps I’m misunderstanding here, but it seems to me the easiest way to fix this would be to modify the desugaring so that the end result looks like what we’re currently using as a workaround today.

All arguments need to be evaluated before the inner function call can begin, so why not always translate

vec.push(vec.len());

to

let a = vec.len() vec.push(a);

That’s more or less what the resulting assembly code would look like is it not? Why do we need to introduce new language features to accomplish this? We can keep our existing left to right evaluation. This would also work for more complicated setups too, such as

let a = vec.len() let b = vec.len() * 2; vec.push((a, b));

1 Like
37

Unfortunately, your proposal does break the existing left to right evaluation, because that includes the implicit “receiver” argument.

vec.push(vec.len());

evaluated strictly left to right, really desugars to something like

{
    let mut v1 = &vec;
    let a = {
        let v2 = &vec;
        Vec::len(v2)
    };
    Vec::push(v1, a);
}

Your change would move the let mut v1 = &vec below the let a = { ... }, and that’s the problem. As I understand it, there is quite a bit of existing code that depends on the receiver expression being evaluated before any of the arguments.

1 Like
38

I see. Could you give me a code example where the borrow was required to be taken prior to the arguments being evaluated? I don’t quite see how this could impact end behavior, as taking a borrow isn’t a mutating operation.

39

There’s one in the very first post in this thread. :slight_smile: Here it is:

let mut v: Vec<String> = vec![format!("Hello, ")];
v[0].push_str({ v.push(format!("foo")); "World!" });
//              ^^^^^^^^^^^^^^^^^^^^^^ sneaky attempt to mutate `v`

The point is that the receiver (the self argument) can be any expression, so it can be a mutating operation. Notice that the index access [0] is calling Index::index, which can run arbitrary code. It could have side-effects on global variables or do I/O or whatever. We can’t just re-order it to run after the arguments have been evaluated, that would change the behavior.

40

It seems like it would enable enough of the ergonomic improvement to desugar into something like:

let arg0 = &mut vec;
let arg1 = arg0.len();
Vec::push(arg0, arg1);

That is, keeping track of “simple” variable usage and allowing use of an alias in that case. I’d be fine if some of these more complicated examples continued to not compile.

1 Like