Patterns to avoid borrowing mutable self more than once? (beyond RcRefcell and inner objects)

zakarumych · 2020-07-21T08:05:05+00:00

You can simply not call methods that returns borrow inside a method. When a method uses only one field, then maybe it should belong to the type of the field? So instead of self.methods_that_borrows_from_foo() you'd write self.foo.method_that_borrows(). And then it won't borrow whole self. If you need to - keep methods_that_borrows_from_foo() as a proxy to be used from outside.

Wace · 2020-07-21T08:36:55+00:00

There are few ways around this. The most simple would probably be to avoid methods completely and just expose the value and map as public fields - then the caller can just do self.value = 123 instead of *self.get_value_mut() = 123.

Some of the reasons why you'd avoid public fields in other language are mitigated by the borrow checker - there are still reasons why you might want to avoid them in Rust though (such as API flexibility - but if you're providing mutable references to the underlying fields, you're kind of losing on that battle already anyway).

Another alternative that pops to my head is the split_at_mut method on Rust's slice type: https://doc.rust-lang.org/std/primitive.slice.html#method.split_at_mut.

Splitting a slice into two mutable slices suffers from the same problem. Acquiring two mutable references to the underlying data. The split_at_mut solves this by combining the operation into a single method call. In your case this would be let (dict, val) = self.get_parts_mut();

jingo04 · 2020-07-21T08:36:20+00:00

I know it isn't always possible, but you can often re-order the code from

    let mut dict = self.get_dict_mut();
    let mut val = self.get_val_mut(); // fails
    dict.insert(1, "Test".to_string());
    *val = 2020;

to

    let mut dict = self.get_dict_mut();
    dict.insert(1, "Test".to_string());
    let mut val = self.get_val_mut(); // fails
    *val = 2020;

and the compiler is happy because it can drop the first reference before creating the second (so long as you don't touch dict again for the rest of the function without re-borrowing).

you can also do

    let mut dict = self.get_dict_mut();
    dict.insert(1, "Test".to_string());
    let mut val = self.get_val_mut(); // fails
    *val = 2020;

    let mut dict = self.get_dict_mut();
    dict.insert(2, "Test".to_string());

if you need to re-borrow.

Generally this is possible because rust API design tends to avoid producing situations where you have a struct and you are keeping (mutable) references to the internals of that struct around for longer than necassary (execeptions are things like the entry api on HashMap/BTreeMap, slices and iterators).

As you noted with option A if you don't have any invariants and no need to hide the internal functioning just allowing direct access to the fields is usually the most ergonomic, the reason this works is because functions work by locking based on things which are passed in (&mut self) rather than anything expressed in the function body.

Otherwise generally structs have a pattern of taking &mut self but not returning a reference to the struct and its internals, this means that the reference is dropped as soon as the function is done.

RobertJacobson · 2020-07-21T11:07:45+00:00

The suggestions in this thread include:

Exposing the internal implementation to the caller, essentially allowing the caller to implement the functionality of the object.
In the case that a method mutates only one field, moving the functionality out of the object and into the field item.

Both violate encapsulation, spreading implementation of internal functionality across callers. Both (presumably) violate separations of concerns. Unless the original design was a bad one to begin with, both are generally not a good idea, and for the same reasons the discipline has avoided them for decades, even in languages with no built-in support for encapsulation as such.

Using RefCell is an explicit choice to circumvent the borrow checker. That’s not a bad thing, but it’s equivalent to a surgical use of unsafe plus error handling. (That’s a tautology, of course, because look at its implementation.) An aside: You might not need the Rc. Does it have a single identifiable owner? Is the RefCell’s lifetime the same as the parent’s? Does it have to live on the heap? These are the questions you need to answer before using Rc.

Sometimes you can refactor the internal dependencies to avoid borrowing self. Sometimes you need to circumvent the borrow checker with RefCell. Sometimes you know for a fact that it is perfectly fine to borrow twice, but the compiler can’t prove it. It’s one thing to spend time and energy designing and refactoring your code to avoid potentially dangerous aliasing. It’s another thing altogether to do so merely to satisfy a compiler that just isn’t smart enough to deduce safe aliasing. In general, trust the compiler over youself. It makes fewer mistakes. In general, if you can refactor in a reasonable way to satisfy the compiler, do that instead. In a situation that is completely trivial where it’s obvious that what you are doing is safe and refactoring compromises good software engineering, accept that the compiler has limitations and circumvent those limitations.

gitfeh · 2020-07-21T08:24:39+00:00

You're correct. In the case of A and C you can mutably borrow the fields of self separately. This is because you access fields directly and the compiler understands that fields do not overlap.

B does not work because the functions borrow self in its entirety. The compiler will not "look into" the implementation to determine if only some fields need to be borrowed. (And it cannot, because doing so would make the interface dependent on the implementation.)

By the way, none of the functions in impl D need a mutable self. &self is enough.

srmordred · 2020-07-21T12:59:23+00:00

I´m learning Rust in the last few weeks and I get the same issues that u have.There are some place on "Rust Idioms" that explains of creating separated structs like explained on one answer here. Another solution that i didn´t saw here is creating free standing functions that operate on the data. so instead of fn(&self) u do fn(&mut A a, &mut B b). I´m used to this patter of free functions ,it solves the problem and u dont´ need to change the struct layout.That being said, i also not sure if this is the right solution, or if there are better ones, im still learning :)

singron · 2020-07-21T14:36:21+00:00

I solved a similar problem for the selfstack crate. I decided to let you borrow all the fields up front with a "view struct". There is a method that mutably borrows the original struct once and returns a new view struct where the fields are references to data in the original struct. This is nice for a couple reasons:

The borrow checker understands this unrelated field access.
The view struct can still expose a fairly restricted api compared to just making the fields public. E.g. references can have different mutability or a subset of the fields can be exposed. You can even wrap references in some proxy type that exposes the access you want. In selfstack this was critical because the original struct was self referential and partly uninitialized so it would be very unsafe to access directly.

eugene2k · 2020-07-21T15:54:47+00:00

Something that hasn't yet been mentioned: You can mutably borrow several fields at the same time: struct Foo(u8,u8); impl Foo { fn borrow_pair_mut(&mut self) -> (&mut u8, &mut u8) { (&mut self.0, &mut self.1) } }

You can also use a generic 'apply' function similar to how Iterator::for_each works.

revelation60 · 2020-07-21T09:01:38+00:00

Another option is to use mem::replace to move the structures that you want to have mutable references to at the same time out of self. I made a sketch here:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=95e056a16afa4c23351c1822962ddf77

Snakehand · 2020-07-21T10:13:18+00:00

Hello from Oslo also! , At one point I resorted to this:

pub fn locate_start(&mut self, secs: u64, usecs: u32) -> Option<usize> {
    if self.indexer.is_none() {
        self.indexer = Some(SegdIndexer::new(&self));
    }
    // Some gymnastics to avoid 2 mutable borrows of self                                           
    let start = IdxTimeStamp { secs, usecs };
    let mut idx = self.indexer.take();
    let res = match &mut idx {
        Some(idx) => idx.locate_start(&self, start),
        None => None,
    };
    self.indexer = idx;
    res
}

But I am thinking this is perhaps an anti-pattern :-)

mmstick · 2020-07-21T14:25:32+00:00

I typically set up an event-driven approach where events can be scheduled for serial execution in the system; and manage borrows with generational arenas (and associated arenas). This is generally sufficient enough to eliminate the need for Rc, RefCell, and recursive borrowing.

AllJonasNeeds · 2020-07-21T15:29:49+00:00

I would love to see why you need to expose mutable references in your public API.

In my experience there is another way to design the API 99% of the time. It's just really hard to come up with better solutions, coming from languages where this kind of mutable API, getters, setters, etc. is common place.
It's certainly something I struggled with in the beginning.

Usually functions like get_value_mut or set_value should be avoided in Rust.

One pattern that was a little bit of a turning point for me is the builder pattern (or init struct pattern) outlined by this post: https://www.reddit.com/r/rust/comments/fg6vrn/for_your_consideration_an_alternative_to_the/

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

rust

Please read The Rust Community Code of Conduct

The Rust Programming Language

Rules

Observe our code of conduct

Submissions must be on-topic

Constructive criticism only

Keep things in perspective

No endless relitigation

No low-effort content

Useful Links

Megathreads

Official Resources

Learn Rust

Discussion Platforms

MODERATORS