Owned pattern does not compile with custom default function.

[Wasabi375]

struct V(u32);

#[derive(Builder)]
#[builder(pattern = "owned")]
struct Foo {
    a: V,
    #[builder(default = "self.calc_b()")]
    b: V,
}

impl FooBuilder {
    fn calc_b(&self) -> V {
        V(self.a.as_ref().unwrap().0 + 5)
    }
}

This fails to compile with

borrow of partially moved value: `self`
 --> src/lib.rs:9:25
  |
3 | #[derive(Builder)]
  |          ------- value partially moved here
...
7 |     #[builder(default = "self.calc_b()")]
  |                         ^^^^^^^^^^^^^^^ value borrowed here after partial move

Looking at cargo expand it's easy to see why:

fn build(
        self,
    ) -> ::derive_builder::export::core::result::Result<Foo, FooBuilderError> {
        Ok(Foo {
            a: match self.a {
                // partial move here
                Some(value) => value,
                None => {
                    return ::derive_builder::export::core::result::Result::Err(
                        ::derive_builder::export::core::convert::Into::into(
                            ::derive_builder::UninitializedFieldError::from("a"),
                        ),
                    );
                }
            },
            b: match self.b {
                Some(value) => value,
                // borrow after partial move here
                None => self.calc_b(),
            },
        })
    }

A possible fix would be to calculate custom default values before the match expression.
A small issue with that is, that the default value will be calculated even if not needed, but I don't think that is a big problem here and can be ignored, except for maybe mentioning it in the documentation.

I'd be willing to create a PR for this if this solution is acceptable.

[TedDriggs]

I’m not comfortable with a change that would cause us to invoke defaults unnecessarily, as it violates the idea of being a zero-cost abstraction.

I’d be open to a docs update for this under the owned builders pattern, but I don’t think we should change the generated code to support this.

[Wasabi375]

Let me think about this some more. There should be a way to implement this without calculating all default values.

Would you be willing to accept a PR if I can implement this as a zero-cost abstraction?

[TedDriggs]

In principle, yes. However, I’m skeptical that it’s possible to do this without either cloning self.a or computing the default when you don’t technically need to.

[Wasabi375]

I think I found a way to do this without cloning or computing the default value when not necessary.
The generated code should look something like this at the end:

fn build(self) -> ::derive_builder::export::core::result::Result<Foo, FooBuilderError> {
    let default_a: Option<V> = match self.a.as_ref() {
        Some(_) => None,
        None => {
            return ::derive_builder::export::core::result::Result::Err(
                ::derive_builder::export::core::convert::Into::into(
                    ::derive_builder::UninitializedFieldError::from("a"),
                ),
            );
        }
    };
    let default_b: Option<V> = match self.b.as_ref() {
        Some(_) => None,
        None => Some(self.calc_b()),
    };

    Ok(Foo {
        a: self.a.or(default_a).unwrap(),
        b: self.b.or(default_b).unwrap(),
    })
}

Note that default_b is only calculated if b == None. Also the unwrap in the initializer will never panic, because either we have a default or we have a value. If there is no value and no default we return earlier when "creating" the default value.
default_a just moves the is_some check out of the initializers.

I will probably find time to implement this this week. Derive macros are a bit to complicated to finish this today.

[TedDriggs]

You wouldn’t need that unwrap if you use unwrap_or, right?

I’m still a bit skeptical of this - I worry that it will either greatly complicate the code for this one fairly narrow edge case, or that it is going to create new issues if the returned value uses a lifetime in some way I can’t immediately think of. I’m willing to review a PR for it, but would like to see a number of test cases to show behavior across different builder patterns, intersections with other features, etc