SE-0289 (review #2): Result Builders

Evolution Proposal Reviews
1

The second review of SE-0289: Result Builders begins now and runs through October 1, 2020.

This is a follow up to the previous review discussion.

There were a few concerns that were brought up, with one that clearly stood out: naming. Given this feedback, the core team decided to re-run the review to rename the attribute based on the suggestions that garnered traction during the review. Additional concerns that were brought up were around the documentation of the feature as well as concerns around tooling with things such as diagnostics. There has been work to address this feedback as well with improvements to the documentation in the proposal and additional diagnostics in the compiler.

There was some feedback over additional features such as enabling stateful builders and handling for the Never and Void types. These are interesting avenues to explore for future extension; however, the core team believes that they can be reasonably considered by later proposals.

This review is limited to:

  • the name of the feature and its attribute and
  • arguments that one or more of the suggested extensions will be problematic to explore in the future

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via email or direct message in the Swift forums).

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Saleem Abdulrasool
Review Manager

11 Likes
2

I think I misunderstood the Void/Never discussion from last time. Can a section be added about dropping Void/Never inputs as opposed to Void/Never outputs? (Or, alternately, is that something that's still open to discussion?)

3

Perhaps I misunderstood the discussion. The new section is about dropping Void/Never outputs from buildExpression, which was the original topic in that thread as I understood it. If you can do that, you can drop Void/Never inputs by adding, e.g.,

func buildExpression(_: Void) { }
func buildExpression(_: Never) { }

The workaround is also the same?

Doug

2 Likes
4

My original intention was the Void post-transformation (as output), so:

func buildExpression(_: Expression) -> Void // Gets ignored

I'm pretty sure Void pre-transformation (as input) would be even harder since it breaks the type-checking steps of 1) type-check buildExpression 2) type-check the entire block.

And I believe that, yes, the work around is much the same (by making the Components optional, etc.).

5

Pre-transformation is way more interesting to me because that's what lets people put assert in their use of someone else's function result builder. It's true that post-transformation would be sufficient to implement that (using extensions), but even if we don't have post-transformation to ignore arbitrary inputs I think pre-transformation is still relevant. I do see Lantua's point about it being hard to implement, and it could still be argued to be confusing for Void (there are three statements here but only two produce values), but I don't think the arguments against post-transformation obviously mean ruling out pre-transformation.

6

Hmm. I think my argument against is the same, and it's only a difference of whether the buildExpression call is effectively short-circuited or not. It still comes down to the buildBlock call excluding any partial results that ended up being Void or Never, which can only be determined after type checking the partial results.

Doug

1 Like
7

Regardless, as the core team laid out in our response to the first review, we see supporting Void/Never specially as a possible future enhancement, and it only needs to be considered in depth right now if there's a compelling argument that fixing it later will cause significant problems.

7 Likes
8

I think returnValueBuilder and resultBuilder are too generic.

For example:

Every function “builds” a “result/value”, but not every function is considered a "resultBuilder".
Every computed property “builds” a “result/value”, but not every computed property is considered a "resultBuilder".

2 Likes
9

+1.

I'm not a huge fan of the unannotated blocks of builders resulting in two different interpretations of an extension

However, the benefit cannot be denied of moving result builders into the standard language as they are already a heavily used Apple extension.

I personally would really like stateful builders, to be able to support a pattern closer to the more common 'accumulator' builder pattern in GoF literature, but I believe that would involve a few more tweaks beyond what is listed in the future directions section.

Is the problem being addressed significant enough to warrant a change to Swift

The style of result builders is directly influenced by the somewhat unique performance goal of translating a block into a strong type. If not for this goal, the design of this feature would be substantially reduced, or even perhaps be reduced to recommendations around the builder pattern.

In the context of that goal, this infrastructure is needed to maintain the readability of Swift.

Does this proposal fit well with the feel and direction of Swift?

I feel it is consistent with other choices in the Swift language, which rely heavily on the compiler to distill the language into a compact executable form.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I am unfamiliar with any languages that have this style of builder.

1 Like
10

Super big +1 on @resultBuilder, it's looks simple, concise, logical and intuitive for both DSL developers and general users.

Result Builder : building final result through a sequence of partial results returned from statements, and combine them to form the Result what we want to build - the result View/Scene/Commands... .

6 Likes
11

+1 for implementation, great work, though I think some more documentation and example on how exactly code is being transformed to build methods calls will be very helpful. Ideally this should be documented in a source code, but I don't see it possible while it's an annotation rather than a protocol like it was suggested in a future directions.

-1 for the name change though. The argumentation for a change claimed that "function" builder is misleading. Well, I don't see how "result" builder is less misleading considering there is a Result type and it's nowhere near this feature. And it was not the only option proposed but for some reason this was one was chosen (with no explanation in the proposal except referencing the forum) over for example a body builder that was also propose. It takes the body of a function or a property and builds something else (another body) out of it. It's not just a pun.
In Kotlin the similar feature is called Composable, great name imo.
Just builder would work nicely as well and there is no misleading whatsoever in that name. It feels like the only reason to prepped it with something else is to maintain some sort of convention for naming attributes where just @builder will not be symmetric to @propertyWrapper or @discardableResult etc. It would be ridiculous reasoning if that was true imo. "Result" word in this case does not add any more meaning or value and hence contradicts Swift API design guidelines IMO.

Not to mention that renaming widely know feature (even if it was private all this time so nothing is guarantied) at this point is in my opinion harmful as it will outdate accumulated knowledge around the feature just for the sake of arguably better name (similar to renaming SwiftUI property wrappers that are still all overt google with their original names)

8 Likes
12

I disagree, as you point out this is a private feature right now. Once made public, the body of knowledge concerning it will continue to grow without having to retain a misnomer for years to come.

One difference is that "Result" is used in common speech to describe an outcome. So both Result and @resultBuilder describe an end product and not the steps to get there.

It is possible that Result and @resultBuilder could be initially confused, but considering Result will never be as widely used as func the chance is a lot smaller IMO.

6 Likes
13

I dislike these languages features that add hidden complexity. They look great on slides, but IME they make debugging and understanding the code harder. Instead of adding new features to address current limitations, I'd rather those limitations be met directly.

I also don't find the motivation section of the proposal compelling. It's mostly demonstrating the difficulty of working with poorly abstracted code. This code is painful because it's writing a novel directly in HTML, skipping a meaningful semantic layer:

let chapter = spellOutChapter ? "Chapter " : ""

let d1header = useChapterTitles ? [header1(chapter + "1. Loomings.")] : []
let d1p1 = paragraph(["Call me Ishmael. Some years ago"])
let d1p2 = paragraph(["There is now your insular city"])
let d1 = division(d1header + [d1p1, d1p2])

let d2header = useChapterTitles ? [header1(chapter + "2. The Carpet-Bag.")] : []
let d2p1 = paragraph(["I stuffed a shirt or two"])
let d2 = division(d2header + [d2p1])

return body([d1, d2])

A DSL for writing books shouldn't use HTML directly. It should create an appropriate semantic layer for books:

struct Book {
    struct Chapter {
        var title: String
        var paragraphs: [String]
    }
    var chapters: [Chapter]
}

Once you do that, representing the book becomes easy:

let mobyDick = Book(chapters: [
    Book.Chapter(
        title: "Loomings.",
        paragraphs: [
            "Call me Ishmael. Some years ago",
            "There is now your insular city",
        ]
    ),
    Book.Chapter(
        title: "The Carpet-Bag.",
        paragraphs: [
            "I stuffed a shirt or two"
        ]
    ),
]

That's with only the synthesized constructors, but you can simplify this more with additional constructors and/or the ExpressibleBy*Literal protocols:

let mobyDick: Book = [
    "Loomings": [
        "Call me Ishmael. Some years ago",
        "There is now your insular city",
    ],
    "The Carpet-Bag.": [
        "I stuffed a shirt or two"
    ],
]

(Probably still not how you want to write a book, but it's semantically precise.)

Once you represent books semantically, the code to display chapter titles becomes straightforward and contained:

extension Book {
    enum ChapterStyle {
        case none
        case number
        case spelledOut
    }
    func render(style: ChapterStyle) -> HTML {
        let divs = zip(1..., chapters)
            .map { number, chapter -> HTMLNode in
                let header: String?
                switch style {
                case none:
                    header = nil
                case number:
                    header = "\(number). \(chapter.title)"
                case spelledOut:
                    header = "Chapter \(number). \(chapter.title)"
                }

                let paragraphs = chapter.paragraphs.map { paragraph([$0]) }
                if let header = header {
                    return [header1(header)] + paragraphs
                } else {
                    return paragraphs
                }
            }
            .map(division)
        return body(divs)
    }
}

This removes certain classes of bugs (e.g. incorrect chapter numbers, inconsistency between chapters, etc.), separates concerns, improves testability, and doesn't require new language features.

Coming up with good examples is hard, but I think the proposal ought to do better to demonstrate the real world need for result builders by giving an example where result builders are the best solution.

10 Likes
14

I agree given that result has meaning in swift.

I'm not sure why @builder isn't a simple name that covers what we mean

2 Likes
15

This is really just an argument against DSLs (or really, an argument against a Book DSL). It also somewhat misses the point of the example, which was more about building HTML than a Book (the Book was just what was being built into HTML). I also find it somewhat ironic that you complain about this feature being too magical (could you be more specific?) while providing an example which uses another of Swift's more magical features, ExpressibleBy*Literal. Additionally, what limitations do you think could be lifted to get close to something like this feature instead of just providing this feature?

However, given that supporting DSLs in some form has long been a goal of Swift's designers, it seems more productive to instead consider whether this proposal properly accomplishes that goal.

10 Likes
16

Personally, I think the naming of this feature (like most naming) should accomplish two things: to inform the developer what it is and what it does. The is part should help those trying to find something like the DSL builders of other languages and the does part should help developers trying to implement one to understand just what they're working towards. @builder kind of accomplishes the first goal but doesn't answer the second at all and so ends up being pretty vague. Alternatively, @dslBuilder does a good job of describing what the feature is but basically nothing about what it does. @resultBuilder (or my favorite @valueBuilder) does a better job of describing what the feature is (something that builds results) and what it does (it builds results), at least as much as a two word phrase can.

7 Likes
17

I also like "composer".

The proposal states, "The set of statements that are permitted within a transformed function are intentionally limited to those that are "strictly structural", and could reasonably be thought of as being part of a single, functional expression, aggregating values but without complicated control flow."

I do not think the name "resultBuilder" conveys that this feature aggregates values. Functions also "build results", but those results cannot always be considered aggregates. The feature being proposed always returns an aggregate value according to a set of aggregation rules.

Given the nature of the feature (i.e. when you define a @_functionBuilder you are defining a set of rules to "aggregate values" in a particular way), I would be okay with the following names:

@aggregator
@composer

There are probably other synonyms that could work as well.

6 Likes
18

Also, as with compactMap, I'm sure I'll get used to whatever name is chosen and be happy with it soon enough. I'm very much looking forward to when @whateverItIsCalled is accepted and part of the standard distribution.

8 Likes
19

I completely agree too. Both names are misleading, given functions and Results are an existing entity in the language and have little to do with the purpose of this proposal.

As others pointed out, just @builder would be a great improvement, although I'm personally more fond of @composable given how most other attributes in Swift are adjectives too (e.g. @inlineable, @available, @frozen, @dynamicCallable) @composer, which describes the exact purpose of the feature: value composition.

5 Likes
20

Everything is a result of some computation, so adding "result" to the name does not accomplish providing any additional information that wouldn't be already known - the fact that it builds something out of something based on some rules and conventions. What this "something" is will be expressed by the type that implements the builder functionality, like ViewBuilder, not by the annotation itself.

But I agree with @Daniel_Steinberg that we will get used to whatever :disappointed: