EXPERIMENT: builder APIs via bon

[ErichDonGubler]

This PR is an experiment that uses bon's derive APIs

Steps to take to advance this experiment:

• Get user feedback.
• Draw the rest of the owl with the APIs that don't have builders yet.
  • Render pipelines
  • Compute pipelines
  • What else?
• Maybe try bon's method builders (as separate methods from the existing ones)?

Warning

Heavily WIP, and, notably, not a priority for my work at Mozilla.

[Veetaha]

Hi @ErichDonGubler, I'm the maintainer of bon. I just discovered this heroic effort through the github search 👀. Hopefully that was a fun experience 😸. I wonder if you have any feedback and most importantly discovered bugs / annoyances / suggestions to improve bon as a result of this?

[Veetaha]

Given that Label is just a type alias for Option<&str>, I'd suggest to inline that type alias so that bon recognizes it as an Option under the hood (it just uses a heuristic text match by the type's path to detect an Option).

For example with the current

#[builder(default, into)] label: Label<'a>

the following setters will be generated:

fn label(self, value: impl Into<Option<&str>>) -> NextBuilderState
fn maybe_label(self, value: Option<impl Into<Option<&str>>) -> NextBuilderState

which adds unnecessary Option nesting. If you inline the type alias, you can omit any #[builder] configs as well:

label: Option<&'a str>

and this will produce the following setters:

fn label(self, value: &str) -> NextBuilderState
fn maybe_label(self, value: Option<&str>) -> NextBuilderState

which removes the Option nesting, and greatly simplifies the code

[Veetaha]

Thank you for the background. I'm not familiar with wgpu (my background is in the distributed systems in the cloud), but I'd be glad to help with any effort of using bon to improve the API surface of the wpgu:

• Assist with code reviews, design recommendations and resolving any questions
• Implement new features / fix existing bugs and annoyances in bon that may hinder this use case
• I may also help with code changes to extend wgpu with the builder syntax APIs unless you'd like to cover all of that on your side.

So from the context you've given, I think that using derive API with structs is the ultimate way to go here. This allows you to keep the API backward compatible, and keeps it much more in line with the WebGPU spec. This will also make it easier for anyone else to maintain this API (no separate method-based builder syntax to maintain).

I've also heard opinions from people that "flat" method call API like this:

.create_texure()
.param(...)
.param(...)
.build()

doesn't look very nice when this method chain is extended with other similar builder API calls:

.create_texure()
.param(...)
.param(...)
.build()
.do_smth_else()
.other_method_param(...)
.other_method_param(...)
.call()
.other_method()
.param(...) 
// ...

Instead if the API is supposed to be chained a lot, then the "Descriptor" parameter structs syntax makes it easier to read with additional small nesting:

.create_texure(
    TextureDescriptor::builder()
        .param(...)
        .param(...)
        .build()
)
.do_smth_else(
    OtherDescriptior::builder()
        .other_method_param(...)
        .other_method_param(...)
        .build()
)
.other_method(
    OtherDescriptior2::builder()
        .other_method_param(...)
        .other_method_param(...)
        .build()
)

The style of "flat builder" API with method-based builders was inspired by AWS SDK API. In that case it makes sense since API calls in AWS SDK are not chained with each other, so such readability problem doesn't appear.

Also, the descriptor-struct-based API makes it possible to build the descriptor separately and and store that struct somewhere else, which I suppose may be potentially a frequent use case in wgpu.

This also gives me an idea that there should be a way to generate a separate "parameters" struct from the function with the bon's attributes syntax.

---

A cargo-feature gated API definitely makes sense in this case. It allows you to keep the builder API coupled with the existing descriptor structs declarations (which it should be indeed) and this way you avoid the need for publishing a separate crate (and the CI burden it may require).

Conditional compilation is supported just fine with cfg/cfg_attr() in bon.

[Veetaha]

Hi! Small update, I've released a 3.0 version of bon that stabilizes the builder's typestate, makes it much cleaner (it's especially visible in the generated docs) and adds many other new features. This is technically a major release, but breakages are very unlikely (most users will see errors due to rejections of some weird syntax in attributes). I've checked that after bumping the version in this PR everything compiles as before. Here is the release blog post with more details.

[ErichDonGubler]

@Veetaha: Did the bump, and it still compiles! Any recommendations for new features to take advantage of? 😀

[Veetaha]

Any recommendations for new features to take advantage of?

I think for the use cases here, there aren't obvious game-changer features from the ones added in 3.0.

The only thing I may add is to this comment: #96 (comment).

If you'd like to keep the type alias Label<'a> without inlining it to an Option<&'a str>, then you may use this pattern instead:

#[builder(default, with = |value: &'a str| Some(value))]
pub label: Label<'a>,

The with attribute will override the signature of setters to accept &str and Option<&str>. While with the #[builder(default, into)] in the current diff in this PR the setters would take impl Into<Option<&str>> and Option<impl Into<Option<&str>>.

Other than that there were just some background changes that you got automatically with this update like generated documentation improvements. For example, values specified with #[builder(default)] values are now rendered in docs automatically:

[Veetaha]

Unfortunately, it's not obvious, but optional fields, that use a generic type are discouraged. This is because it leads to weakened type inference.

In short, if you minimize the problem it boils down to this analogous pattern:

fn example<T>(_value: Option<T>) {}

fn main() {
    example(None);
}

This code doesn't compile with the error:

  |     example(None);
  |     ^^^^^^^ ---- type must be known at this point
  |     |
  |     cannot infer type of the type parameter `T` declared on the function `example`

This similar problem would occur if you use TextureDescriptor::builder() and omit to call the label setter or pass None to maybe_label() setter. The compiler will not be able to infer the generic type L in such case.

I described this problem in detail here, feel free to check that explanation on your leisure.