1

Hello, Swift community.

The review of SE-0373: Lift all limitations on variables in result builders begins now and runs through October 10th, 2022.

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 me as the review manager, either via email or PM on these forums. When emailing me directly, please put "[SE-0373]" at the start of the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • 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?

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/main/process.md

Thanks for participating in this review.

John McCall
Review Manager

19 Likes
2

I think this solves many pain points I’ve experienced with SwiftUI, +1 from me. The user defaults example is also really cool!

My only issue with this proposal is that it seems like the decision to require each ResultBuilder implementer to support initializing uninitialized vars could lead to confusion for users: if they (or the author of the ResultBuilder they’re consuming) don’t add the specific build function, it won’t work. I don’t have a good alternative, but maybe there could be a special compiler diagnostic that suggests adding that function or otherwise explains why it doesn’t work in certain result builders?

3 Likes
3

What would happen if you put an @State property inside the body property in SwiftUI? That’s kind of how React works. Would that enable syntax that would just break in weird ways?

1 Like
4

It wouldn’t work. But you could already put a State property wrapper in a non-View type, or stack multiple SwiftUI property wrappers on the same var, and it wouldn’t work, in the exact same way.

I think the expectation already exists today that users of property wrappers must understand that some of them only work in certain locations/as members of specific types. Though it is unfortunate that that can’t be enforced.

4 Likes
5

We can certainly add such a diagnostic saying that a builder doesn't support assignment due to lack of buildExpression(_: Void) overload.

7 Likes
6

Almost too good to be true, if that really works.

2 Likes
7

+1 This seems good to me, especially as it aligns the implementation closer to the original proposal.

8

One question about the rule for uninitialized variables. What’s the logic around requiring builders to support the delayed initialization as opposed to treating initializations as part of the declaration (and distinct from an assignment)? IOW, in the example:

  let outcome: Outcome

  if let error {
    // error specific logic
    outcome = .failure
  } else {
    // complex computation
    outcome = .success
  }

why not have the transform leave the outcome = .failure and outcome = .success alone, like it would if they were part of an initializer expression?

2 Likes
9

@Jumhyn This is the behavior introduced by the result builder proposal. We could amend the proposal separately to support that too. I believe that could even have been discussed by language work group. Is that so, @hborla?

10

Sorry, I’m not sure I understand—isn’t this proposal introducing the ability to have out-of-line initialization for local variables? Why would we need a separate amendment to decide how these would behave?

1 Like
11

This proposal is focused on declarations only, the assignment (and initialization as a sub-case) is described by the original result builder proposal.

12

Just to clarify on my point, special-casing initialization this way would mean that semantic analysis would have to understand the distinction between the assignment and (partial) initialization which it doesn't have enough information to do, that's why I think assignment semantics need to be amended separately and in general.

1 Like
13

In principle, we could special-case assignments to lets to be completely ignored by the transform, since they're ill-formed unless they're initializations. I don't know if that would be a good change, though; it would create an inconsistency between var and let, and it would prevent builders from disallowing this kind of thing if they want to generally discourage users from writing code that looks side-effectful in their builder bodies.

1 Like
14

While I'm not certain this is a great goal in general:

I'll note that by leaving declarations alone the ship has kind of already sailed on this front since users can just write:

someBuilder {
  let _ = sideEffectCity()
  value
}

I guess it's discouraged in that users will get a speed bump from trying to write sideEffectCity() directly...

But even if we think it's good for result builder authors to be able to discourage side-effectful code, I don't think it has to be the default. Builder authors could still opt-in to this behavior by, e.g., declaring an unavailable buildExpression(_: Void).

2 Likes
15

As I understand it, that it exactly what Pavel is trying to say:

  1. This proposal allows you to use out-of-line initialization in builder-transformed functions.
  2. In order to do so, the result builder must support void expressions, because the compiler considers out-of-line initialization to be an assignment expression, which is a void expression.
  3. If you want out-of-line initialization to not be treated as an assignment, that should be a separate proposal; however, it will have complicated interactions in general.

Maybe that section of the proposal ought to be clearer.

5 Likes
16

Oh yeah I think we've gone a bit astray from the initial line of discussion, I'm convinced it's reasonable to leave this for a future proposal. I didn't think about the fact that we wouldn't have DI information at the point where we're doing the builder transform so it isn't really possible to treat "initialization" separate from "assignment" unless we do something like the var/let split you suggested.

17

I think that’d be like flipping the default. The current design prohibits code that looks side-effectful, so many API authors would likely be under that impression. It’s a reasonable future direction to give builders control over and the ability to transform properties declared within them. So it will be really confusing if we go from the current state of affairs, to allowing properties by default, to offer the option of transforming them. I agree with John that most result-builder code builds results in a mostly non side-effectful manner. It may also take library authors some time before they explicitly opt out of this feature, they considered to be already prohibited. Also, given that one of the first questions in the thread was about putting @State in SwiftUI, it’s crucial that the API author have control over variable declarations in builders.

18
  • What is your evaluation of the proposal?

-1

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

No, I think the current proposal isn't sufficiently motivated.

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

Possibly. But I consider all the examples listed in the motivation to be bad uses of result builders and/or property wrappers. I worry we're lifting restrictions that helpfully partition between imperative and declarative code just because it's technically possible even though it's not actually improving anything.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I've put a lot of time into writing up my thoughts, but didn't have a chance to read the pitch or review threads.

Let me elaborate on each example:

#1

func test(@MyBuilder builder: () -> Int?) {
  ...
}

test {
  let (result, error) = compute()

  let outcome: Outcome

  if let error {
    // error specific logic
    outcome = .failure
  } else {
    // complex computation
    outcome = .success
  }

  switch outcome {
   ...
  }
}

It's not clear to me why this example is using a result builder in the first place. This is pure imperative code and would be better served by using vanilla Swift than a result builder.

#2

var body: some View {
    GeometryReader { proxy in
        @Clamped(10...100) var width = proxy.size.width
        Text("\(width)")
    }
}

The use of the property wrapper here is overkill. I'm sure there's a design principle somewhere about using the least exotic/powerful tool to get the job done. Here that's just a method:

var width = proxy.size.width.clamped(10...100)

#3

var body: some View {
    @UserDefault(key: "user_has_ever_interacted") var hasInteracted: Bool
    ...
    Button("Browse Features") {
        ...
        hasInteracted = true
    }
    Button("Create Account") {
        ...
        hasInteracted = true
    }
}

SwiftUI uses ivars for property wrappers like @State. Why? Variables don't usually live past their lexical scope. I think it'd be more intuitive here if @UserDefault was an ivar.

Potential Confusion

Introducing the ability to write an uninitialized variable declaration introduces potential confusion around what’s valid in a builder context and breaks the impression that this DSL is a declaration of structure:

var body: some View {
    let value: Int

    VStack {
        Text("Hello, world!")
        if someBoolean {
            Text("It’s false")
            value = 10
        } else {
            Text("It’s true")
            value = 20
        }
        Text("Value is \(value)")
    }
}
3 Likes
19

To be clear, SwiftUI does not allow Void results, and so these assignments would not be allowed in that DSL. It's fair to say that the proposal therefore shouldn't use examples from SwiftUI, but it's also fair to say that SwiftUI will not suffer from any confusion about this kind of assignment. In practice, I don't think people generally confuse one result builder DSL for another and wouldn't be surprised by features being available in only certain DSLs.

Confused/erroneous use of property wrappers is a more general point.

2 Likes
20

Ya this largely mitigates my concern about SwiftUI and initialized variables, though I still worry if we came up with a compelling reason for Void results in the future, we would be forced to allow these assignments.

My meta-concern is that we're lifting these restrictions without concrete, compelling motivations.

However, I came up with a use case at least for lifting the restriction of using property wrappers in result builders that I find compelling. In particular, the @Binding property wrapper is primarily providing a syntactic illusion, and the motivation for using it in local variables within result builders is consistent with the motivation in SE-0293.

Here's a concrete example that could take advantage of this proposal:

struct StudyNowView: View {
    struct ViewState {
        private var _isStudying: Bool = false

        subscript(reviewCount n: Int) -> Bool {
            get { _isStudying && n > 0 }
            set { _isStudying = newValue }
        }
    }

    @State private var state = ViewState()
    var session: StudySession

    var body: some View {
        @Binding var isStudying = $state[reviewCount: session.reviewCount]

        OtherIrrelevantContent()
            .sheet(isPresented: $isStudying) {
                StudySessionView(session: session, onDismiss: { isStudying = false })
            }
    }
}

Sorry for the involved example, but it's from a personal app I'm working on. The StudySessionView is modally presented as long as the user hasn't explicitly dismissed it and there are still flash cards left to review. The projection via subscript is combining the model data with the local state data.

Being able to use the @Binding local variable allows me to avoid (1) repeating myself:

.sheet(isPresented: $state[reviewCount: session.reviewCount]) {
    StudySessionView(session: session, onDismiss: { state[reviewCount: session.reviewCount] = false })
}

and (2) breaking the syntactic illusion:

let isPresented = $state[reviewCount: session.reviewCount]

OtherIrrelevantContent()
    .sheet(isPresented: isPresented) { // no $, so unclear if read-only or read-write
        StudySessionView(session: session, onDismiss: { isPresented.wrappedValue = false })
    }

So you can convert me to a +1 for using lifting the restriction of using property wrappers in result builders.