Overcoming 'cannot borrow `x` as mutable more than once' – what is the best option, in this case?

[d4h0]

Hello everybody,

I have a struct, that keeps data that needs to be valid as long as the struct exists, and offers methods to add data to it. Because of the lifetime and mutability requirements, I get a cannot borrow x as mutable more than once error.

I understand why this happens, and I'm wondering what my best options are to overcome it. Currently, I believe, interior mutability via RefCell or Mutex is the way to go, but I'm curious what others think.

The not working code looks as follows (Playground):

use std::borrow::Cow;
use std::collections::HashMap;

/// HTML Input element
struct Input<'input> {
    name: &'input str,
}

/// HTML form element
struct Form<'form> {
    data: HashMap<&'form str,  Cow<'form, str>>,
    elements: Vec<Input<'form>>,
}

impl<'form> Form<'form> {
    fn fill<F: FnOnce(&'form Input) -> Cow<'form, str>>(&'form mut self, with: F) {
        let input = &self.elements[0];
        let value = with(input);
        self.data.insert(input.name, value.into());
    }
}

fn main() {
    let mut form = Form {data: HashMap::new(), elements: vec![Input {name: "foo"}]};
    form.fill(|el| Cow::Borrowed(el.name));
    form.fill(|el| Cow::Borrowed(el.name));
}

Error: cannot borrow form as mutable more than once at a time

Using RefCell fixes the problem (see this Playground).

Form is part of an HTTP client I'm working on, and will be used in a multithreaded, async context. If I'm not mistaken, using RefCell means, there could be no .await until the form is submitted (which consumes the Form).

Normally, there would not be a need to .await something while "filling out" a form, but I can't be sure that this always will be the case.

Another option, therefore, would be, to use a Mutex, instead of the RefCell.

As far as I know, this would have more overhead than RefCell, so I'd like to avoid it.

On the other hand, the overhead of Mutex should be minuscule compared to the other stuff that is going on (HTTP requests, and parsing HTML). Besides that, if I'm not mistaken, there normally shouldn't be any contention for the Mutex (currently, I give users mutable access to the Form data, so in theory contention would be possible).

With that information, what do you think, would be the best approach here?

[LegionMammal978]

It seems like you only need to change the method signature (Rust Playground):

 impl<'form> Form<'form> {
-    fn fill<F: FnOnce(&'form Input) -> Cow<'form, str>>(&'form mut self, with: F) {
+    fn fill<F: FnOnce(&Input<'form>) -> Cow<'form, str>>(&mut self, with: F) {
         let input = &self.elements[0];
         let value = with(input);
         self.data.insert(input.name, value.into());
     }
 }

This makes with take an &'_ Input<'form> instead of a &'form Input<'_>, so that &self.elements[0] can be fed into it. The current signature requires the Input itself to live for all of 'form, which generally isn't the case.

[d4h0]

Thank you, @LegionMammal978!

I didn't think about that!

Unfortunately, my actual code is a bit different, and the closure doesn't even get a reference to the HTML element (it gets an owned value that implements Copy, which acts similar to a reference).

Below, I have extracted the relevant code, in case you (or anybody else) would like to have a look.

The code depends on the scraper crate, which isn't available on play.rust-lang.org. However, I have also set up a playground on replit.com, which allows using any crate (unfortunately, this seems to require signing up for a free account. But Replit looks pretty amazing, so I can only recommend giving it a try, anyway).

Playground on play.rust-lang.org | Playground on replit.com

use scraper::{element_ref::ElementRef, Html, Selector};
use std::{borrow::Cow, collections::HashMap, convert::Infallible};

pub struct Form<'form> {
    form_element: ElementRef<'form>,
    data: HashMap<&'form str, Cow<'form, str>>,
}

impl<'form> Form<'form> {
    pub fn fill<R, E, T, V>(&'form mut self, selector: &Selector, value: V) -> ()
    where
        R: Into<Cow<'form, str>>,
        E: std::error::Error + std::fmt::Debug,
        V: IntoFormInputValue<'form, R, E, T>,
    {
        let input_element = self.form_element.select(selector).next().unwrap();
        let name = input_element.value().attr("name").unwrap();
        let value = value.into_form_input_value(input_element).unwrap();
        self.data.insert(name, value.into());
    }
}

pub trait IntoFormInputValue<'form, R, E, T>
where
    R: Into<Cow<'form, str>>,
    E: std::error::Error + std::fmt::Debug,
{
    fn into_form_input_value(self, element: ElementRef<'form>) -> Result<R, E>;
}

/// Implementation for `FnOnce` that returns a [Result].
impl<'form, R, E, F> IntoFormInputValue<'form, R, E, (R, E, F)> for F
where
    R: Into<Cow<'form, str>>,
    E: std::error::Error + std::fmt::Debug,
    F: FnOnce(ElementRef<'form>) -> Result<R, E>,
{
    fn into_form_input_value(self, element: ElementRef<'form>) -> Result<R, E> {
        self(element)
    }
}

/// Implementation for `FnOnce` that returns a `Into<Cow<'form, str>>`.
impl<'form, R, F> IntoFormInputValue<'form, R, Infallible, (R, F)> for F
where
    R: Into<Cow<'form, str>>,
    F: FnOnce(ElementRef<'form>) -> R,
{
    fn into_form_input_value(self, element: ElementRef<'form>) -> Result<R, Infallible> {
        Ok(self(element))
    }
}

/// Implementation for `Into<Cow<'form, str>>` itself.
impl<'form, R> IntoFormInputValue<'form, R, Infallible, (R,)> for R
where
    R: Into<Cow<'form, str>>,
{
    fn into_form_input_value(self, _element: ElementRef<'form>) -> Result<R, Infallible> {
        Ok(self)
    }
}

fn main() {
    let html = r#"<form><input type="text" name="name"></form>"#;
    let html = Html::parse_fragment(html);
    let form_selector = Selector::parse("form").unwrap();
    let form_element = html.select(&form_selector).next().unwrap();
    let mut form = Form {
        form_element,
        data: HashMap::new(),
    };
    let input_selector = Selector::parse("form").unwrap();

    form.fill(&input_selector, "test");
    form.fill(&input_selector, |_input_element| "test");
}

[LegionMammal978]

Why do you need the &'form mut self in Form::fill()? The code compiles and runs as-is without it:

@@ -7,7 +7,7 @@
 }
 
 impl<'form> Form<'form> {
-    pub fn fill<R, E, T, V>(&'form mut self, selector: &Selector, value: V) -> ()
+    pub fn fill<R, E, T, V>(&mut self, selector: &Selector, value: V) -> ()
     where
         R: Into<Cow<'form, str>>,
         E: std::error::Error + std::fmt::Debug,
@@ -70,7 +70,7 @@
         form_element,
         data: HashMap::new(),
     };
-    let input_selector = Selector::parse("form").unwrap();
+    let input_selector = Selector::parse("input").unwrap();
 
     form.fill(&input_selector, "test");
     form.fill(&input_selector, |_input_element| "test");

(Note that when you have a struct MyStruct<'a>, it's usually an antipattern to use &'a mut self as a method receiver, since such a method effectively consumes self when called.)

[d4h0]

Thanks a million, @LegionMammal978!

Why do you need the &'form mut self in Form::fill() ? The code compiles and runs as-is without it:

I thought this was required, but after thinking about it, it clearly/obviously isn't. This is one of the most difficult code I wrote so far, and I tried many things to get a nice interface.

(Note that when you have a struct MyStruct<'a> , it's usually an antipattern to use &'a mut self as a method receiver, since such a method effectively consumes self when called.)

That is very good to know. Normally, I try to avoid lifetimes, if possible at all. But my HTTP client has to have a ton of them, because of underling crates I use (and because I wanted to expand my Rust knowledge a bit).

Thanks, again!

[quinedot]

For future reference, this

impl<'form> Form<'form> {
    pub fn fill<R, E, T, V>(&'form mut self, selector: &Selector, value: V) -> ()
    where /* ... */

Means the method takes a

&'form mut Form<'form>

And this "thing with lifetime 'x behind a &'x mut" pattern is a red flag; it requires borrowing the thing for the entirety of its lifetime, and thus you cannot use it at all after the call (modulo indirectly through some return value).

[d4h0]

@quinedot

And this "thing with lifetime 'x behind a &'x mut " pattern is a red flag; it requires borrowing the thing for the entirety of its lifetime, and thus you cannot use it at all after the call (modulo indirectly through some return value).

Ah, thank you, @quinedot! Now I understand better what was going on. Somehow I though the compiler would select the minimum lifetime that works. But in reality, what I say is a hard rule for the compiler.

As I wrote, I don't use named lifetimes often (especially not in types, if I can avoid it anyhow... ;)), and usually only local ones (declared on functions), and mostly follow what the compiler recommends on errors (which wasn't anything helpful this time).

[quinedot]

@d4h0

Somehow I though the compiler would select the minimum lifetime that works. But in reality, what I say is a hard rule for the compiler.

Yeah, I could have been more thorough in explanation -- part of the problem is that lifetimes behind a &mut are invariant (cannot be "shrunk"), as otherwise you could for example store a &'short str into a &mut Vec<&'long str> (and then hold onto the reference after its backing data has been dropped).

Shared references do not have this constraint, so &'x Thing<'x> is much less likely to cause a problem, as a &'short Thing<'long> will coerce to a &'short Thing<'short> so long as Thing itself is covariant (which is common). That being said, it's still less general, and just not naming the reference lifetime if not actually needed is a good default.

[simonbuchan]

In general, this is another example of "lifetimes in structs are hard-mode". You have no loss of generality by just using String, for example, instead of &'form str, only performance, and a few more clone()s / into()s. Not to discourage trying hard mode if you want, of course, but don't let it ruin your day!

[d4h0]

Shared references do not have this constraint, so &'x Thing<'x> is much less likely to cause a problem, as a &'short Thing<'long> will coerce to a &'short Thing<'short> so long as Thing itself is covariant (which is common).

Ah, then that is probably the reason why I thought mutable references would act this way.

otherwise you could for example store a &'short str into a &mut Vec<&'long str> (and then hold onto the reference after its backing data has been dropped).

But that makes of course sense.

[d4h0]

In general, this is another example of "lifetimes in structs are hard-mode". You have no loss of generality by just using String , for example, instead of &'form str , only performance, and a few more clone() s / into() s. Not to discourage trying hard mode if you want, of course, but don't let it ruin your day!

Yes, using Strings would have been much simpler and easier.

My main goal was to offer a nice API, where all the following is possible:

form.fill("#input", "value");
form.fill("#input", format!("{} value", "dynamic");
form.fill("#input", |_| "derived value");
form.fill("#input", |element| format!("derived value {element:?}");
// Plus `Cow` variants

It wouldn't be a big deal to add .to_string() if needed, but I thought implementing what I want wouldn't be too difficult because Form is (normally) only used in the scope in which it is created.

In the end, I learned several new things, which is also great

[simonbuchan]

You will want to look into using impl Into<String> parameters, and then all the other conversion and interpretation traits like AsRef, Borrowed, and the like, or even defining your own: this let's you as the library eat all the usage complexity, and the user gets the nice API experience.

Nothing is free, of course, but the cost here is that the function becomes a generic, so it could be compiled multiple times, and can't be used in a trait itself (without other tricks), and the like, so it's generally a good trade-off, and you'll see the standard library do this all the time.

[d4h0]

You will want to look into using impl Into<String> parameters, and then all the other conversion and interpretation traits like AsRef , Borrowed , and the like, or even defining your own: this let's you as the library eat all the usage complexity, and the user gets the nice API experience.

Thank you, @simonbuchan. Can you describe how that would make the API easier? To me, it looks similar to what I ended up with.

In your comment you are using the impl Trait form. Does that change anything? From what I read, it only means, that users of a function can't explicitly specify the generic argument.

One mistake I did (I just realized), was to use Into<Cow<'form, str>> instead of Into<Cow<'static, str>>. In the majority of cases it shouldn't be necessary to borrow anything from the input element itself, so using 'form was a mistake.

[simonbuchan]

Sorry, missed the actual code! You're absolutely right on all counts.