[Closed] Pitch: Explicit marker for autoclosure parameters

Yep. I changed "message" parameter to be named, otherwise it's less obvious:

precondition { denom != 0 } _: { "Can't divide \(num) by zero" }

IRT file / line being last parameters I don't think there's a problem: this still works:

precondition { denom != 0 } message: { "Can't divide \(num) by zero" }

and if file needs to be parametrised explicitly (as you rightly mentioned rarely needed), then either this will do:

precondition({ denom != 0 }, message: { "Can't divide \(num) by zero" }, file: "file")

or we change parameters order, putting file / line before the closure parameters:

precondition(file: "file") { denom != 0 } message: { "Can't divide \(num) by zero" }

I think you should just code defensively and assume every parameter could be autoclosure.

So any time you rely on knowing how many times foo(42) is being called (which is probably not often), then assign it to a variable before passing it as a parameter. Then you know it will be called no matter what the underlying api uses.

2 Likes

There are already major lacks of clarity at the use site in Swift that were accepted. Trailing closures, eliding return, property wrappers, inout to name a very few, ResultBuilders!

This thread is ultimately debating a (major?) source breaking change that would result in no major improvement to the language.

As with all of these lacks-of-clarity is serves its purpose and is resolved when simply reading the API.

1 Like

I think this is the equivalent mindset involved with "implementation-detail property wrappers".

Because of that, I think the motivation behind this thread isn't in-line with language evolution. Instead, the main problem is that we don't have access to autoclosures as "API-level".

Most of the mentioned examples do not contradict clarity over brevity rule (IMHO). If some feature does - we can and should fix it - ††. One feature at a time.

So this would be an improvement? At least someone agrees :)

And if we were consistent we would not see this as an improvement:

colour.getRed(red, green: green, blue: blue, alpha: alpha)

over the current:

colour.getRed(&red, green: &green, blue: &blue, alpha: &alpha)

or would we?

What makes us tolerate & in (1) and not tolerate { } in (2) in the following:

1. colour.getRed(&red, green: &green, blue: &blue, alpha: &alpha)
2. precondition { condition } message: { "message" }

As for the breaking change - we have precedents doing that, typically by deprecating the thing in question first over a few releases.

Swift API Design Guidelines clearly state as the very first two rules:

  • Clarity at the point of use is your most important goal.
  • Clarity is more important than brevity.

There are no exceptions here, such as "yes, but if the feature is so powerful-† and if we ask API designers to use it consciously with a great deal of discipline then these rules could be disregarded"-††.

† - which in case of autoclosure is not the case.

†† - Unless we change the API guidelines, basically removing the top two bullet points about clarity over brevity.

Granted, this is the API design, but having the core language feature that directly contradicts this API design rule is not wise still. About the only way to strictly adhere to the above API design rules IRT @autoclosure is not using autoclosure at all, and then autoclosure is as good as being removed from the language.

Let’s say that for some reason swift 6 introduced autoinout, which would let us write:

colour.getRed(red, green: green, blue: blue, alpha: alpha)

If i then proposed that we should require the callsite for autinout to add a decorator like %, would you say that this makes sense at all?

colour.getRed(%red, green: %green, blue: %blue, alpha: %alpha)

Or would you say that this completely defeats the purpose of autoinout, and that it would make no sense at all?

2 Likes

This thread convinced me that the pitch alternative #2 (remove/deprecate autoclosure) is the way to go.

  • If we value brevity over clarity at the point of use, then:

    • we introduce "auto-inout"
    • we leave autoclosure as it is
    • we remove the first two bullet points from the API design document (or change them to their opposite)
  • If we value clarity over brevity at the point of use, then:

    • we deprecate "auto-inout" (if we had it)
    • we deprecate autoclosure
    • the first two bullet points in the API design document are left as they are

Otherwise we are inconsistent and must admit that we do as we please in one language area compared to another.

1 Like

IMO the cited examples so far of operator short-circuiting and assert-style functions are excellent examples of adhering to the API design guidelines while using @autoclosure.

Also, based on the responses in this thread so far from members of the Language Steering Group I don't really expect there to be any appetite for a source-breaking change in this area, and certainly not one as wide-reaching as "deprecate @autoclosure."

Even considering the possibility for API author abuse IMO it doesn't nearly fulfill the "active harm" standard we've generally held source breaks to.

9 Likes

Clarity, brevity and consistency are all things to balance - they’re design guidelines not design shackles. I’d find it disappointing if we dropped autoclosures for the sake of consistency when they’re extremely useful in a number of situations and harmful in an IMHO inconsequential number of situations.

Personally I’m not bothered by these sorts of inconsistencies in language design philosophy and rarely find arguments for consistency compelling in their own right.

3 Likes

Strictly speaking auto-closures are not required for operator short-circuiting behaviour.

I invite you to provide some killers examples when auto closures are extremely useful. So far we've seen just one non assert-like example in this thread, which doesn't seem to be too convincing or losing too much when written without auto-closures:

let value = collection.valueForHorizontalSizeClass(
    regular: foo(),
    compact: bar(),
    otherwise: baz()
)

vs

let value = collection.valueForHorizontalSizeClass {
    foo()
} compact: {
    bar()
} otherwise: {
    baz()
}

Ditto for the assert-like calls.

Personally I don't think this reaches a bar of introducing a new feature ("if we didn't have auto-closures would we introduce them now" test) especially combined with the "clarity wins over brevity" rule violation.

I’m mildly inclined to agree with this, but the criteria for removing a language feature are not the same as the criteria for rejecting a new feature, so the point is moot.

2 Likes

Assuming that the new pitch is "get rid of @autoclosure", I disagree and I see no good reason to do it. The idea of adding markers to identify the "autoclosured" parameter defeats the purpose: the entire point of @autoclosure is to avoid marking parameters, and the reason is, it shouldn't matter.

The original question

is only worth asking if foo does side effects, which is not a good idea in general for a function, unless its specific purpose is to do side effects, and in that case it's unlikely that it would be involved in a @autoclosure argument. For the most part, it's a good idea for functions to be pure, which plays perfectly with @autoclosure.

It might be a matter of code style, but if a style is such that it's a problem not knowing how many times a function is called when its result is passed directly to an argument of another function, I'd suggest to rethink the style. There's an argument to be made about performance (if the function execution passed as argument is slow), but this is the same for functions with closure arguments: if the function can be smart about the evaluation of the argument, it's up to its implementation, and it should be written in a way doesn't leak the abstraction to the caller, in order to avoid complexity. Also, one can be sure by simply evaluating the passed function first, store the value in a let and them pass it to the other function.

There's actual room and utility for @autoclosure in the language. Also, its removal would be such a destructive change that the bar to do it would skyrocket. I think that almost all experienced Swift developers would agree on both things: that doesn't mean that everyone would actually use it for new code in general (I, for one, avoid it), but that's about practices and conventions.

If the pitch was about prohibiting @escaping @autoclosure, though, I would definitely agree: in my experience it's harmful and confusing.

1 Like

Your alternatives rely on a feature that did not exist when @autoclosure was added to Swift. Has the language ever lost a feature because a later feature "obsoleted" it?

2 Likes

It's like driving a car with a loose steering wheel...

Which one?

We do breaking changes every now and then. Depreciating changes are not strongly breaking, it's just a warning you'd be able to ignore for quite a few Swift releases.

More importantly, what's the perceived evolution of Swift.. do we add new features without ever removing existing features? I don't think that's sustainable long term - at some point we'd have too many features and the language becomes too big. I'd say the language is already quite big and the rate of adding new features should roughly match the rate of removal old features to not make the language even bigger.

1 Like

Multiple trailing closures is a relatively recent addition.

If I understand correctly, the concern is due to the fact that call site does not let the caller know that the call passed to it will be lazily called.

It has been my experience that @autoclosure should be used in places where the caller would expect the call to be lazy. The fact that it is lazy is almost an implementation detail. (IMO)

@autoclosure helps keep the call site looking nice and the code performs as expected. (IMO)

I have written some test helpers and have used @autoclosure so that the API matches XCTest asserts' API. The call point feels natural and the values are lazily evaluated as expected.

I would chose option #1 and keep @autoclosure as-is. My next choice would be #2 and remove @autoclosure completely. The other options are thoughtful, but don't seem to add value to the situations where I would use @autoclosure today.

Here is an example test helper that is using @autoclosure.

 func XCTCaptureError<T>(
    from block: @autoclosure () async throws -> T,
    _ message: @autoclosure () -> String = "",
    file: StaticString = #file,
    line: UInt = #line
 ) async -> Error? {
    ...
}

And here is an example call site usage:

    let capturedError = await XCTCaptureError(from: try await sut.createPoem())

without @autoclosure I guess the call site would be:

    let capturedError = await XCTCaptureError(from: { try await sut.createPoem() })

or:

    let capturedError = await XCTCaptureError { try await sut.createPoem() }

I may be biased, but I like the @autoclosure version. It seems to be more clear at the call point.

here is the full function and usage for completeness:

 func XCTCaptureError<T>(
    from block: @autoclosure () async throws -> T,
    _ message: @autoclosure () -> String = "",
    file: StaticString = #file,
    line: UInt = #line
 ) async -> Error? {
    do {
        _ = try await block()
        let description = ["XCTCaptureError failed: should have thrown an error", message()]
            .filter { !$0.isEmpty }
            .joined(separator: " - ")
        let context: XCTSourceCodeContext = .init(location: .init(filePath: file, lineNumber: line))
        record(.init(type: .assertionFailure, compactDescription: description, sourceCodeContext: context))
        return nil
    } catch {
        return error
    }
}
func test_failingGetUsername_createPoem_fails() async throws {
    let error = XCTAnyError()
    let (sut, _) = makeSUT(getUsernameResult: .failure(error))

    let capturedError = await XCTCaptureError(from: try await sut.createPoem())

    XCTAssertCastEqual(capturedError, error)
}

Repo link: TDDKit

Might well be I am. Or both of us. To me the second line is a clear winer:

    let capturedError = await XCTCaptureError(from: try await sut.createPoem())
    let capturedError = await XCTCaptureError { try await sut.createPoem() }

Note another difference: if you put try await sut.createPoem() on its own line you won't be able putting a breakpoint on that line in case of autoclosure (well, you can it just won't work), whereas it won't be a problem when it's a real closure, be it written in a full blown syntax or the "trailing closure optimised" syntax.

I'm a native English speaker, so the first one works for me. I feel like the "from" should be in there.

If there was no @autoclosure, then I would prefer the version with the parameter wrapped in curly brackets.

The trailing closure version makes the least sense to me. I am surprised this is a clear winner to you. Maybe I am in the minority.

I think I can understand where you are coming from. If there were many lines needed in the closure for the parameter, the multiline closure would be the only answer.
I do have API which often can have multiple lines in the closure. In that api I do NOT write it with @autoclosure. I require the closure to be typed out.

I think @autoclosure's intended use is that 99% of the time you DO NOT pass a closure but instead get one automatically. I could be wrong.

When I use @autoclosure I am trying to nicely wrap a single call. Although a multi-line closure could be passed here, that is not the expected usage.
This is a test helper and testing is already too verbose as it is.

I didn't think about the breakpoint issue. But if you want to put a breakpoint here, then I think you are doing it wrong. :)
I can't see any reason why you would want to put a breakpoint inside the closure. You don't often use breakpoints with TDD. If you wanted a breakpoint you could put it in the production code method. This example is test code.

As for the ability to put a breakpoint I didn't mean your example specifically. E.g. if you are tracing the call below and want to know which branch is taken and for some reason you can't put a breakpoint on the internals of foo/bar/baz (they are in the OS, or they are in the compiled library with no source, or they are called too frequently, etc):

let value = collection.valueForHorizontalSizeClass {
    foo()
} compact: {
    bar()
} otherwise: {
    baz()
}

I don‘t think @autoclosure is problematic, and although I see some vague similarity with inout, I don‘t consider it comparable.
I do not expect this thread to turn into a proposal, but afaics, simply removing @autoclosure has some unwanted consequences:
Closure syntax would be needed even in the simplest cases. a && b (with b being some constant) would become `a && { b }, unless there are some extra overloads.

1 Like