swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.31k stars 10.34k forks source link

[SR-5296] @autoclosure error: default argument value of type '(A) -> B' cannot be converted to type '(A) -> B' #47871

Closed 7156ccc1-d3b8-4460-882a-3f5802920f8f closed 7 years ago

7156ccc1-d3b8-4460-882a-3f5802920f8f commented 7 years ago
Previous ID SR-5296
Radar None
Original Reporter @DevAndArtist
Type Bug
Status Resolved
Resolution Done
Environment Swift 3.1, Xcode 8.3.3 (8E3004b)
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | @CodaFi | |Priority | Medium | md5: 91d9430944193af7b73743954fde9bf5

Issue Description:

Code to reproduce:

func foo(_ test: @autoclosure (Int) -> String = { "\($0)" }) {
    test(42)
}
benrimmington commented 7 years ago

The real issue is that an autoclosure doesn't take any arguments:

func test0(_ test: @autoclosure ()         -> String) {} // okay
func test1(_ test: @autoclosure (Int)      -> String) {} // expected-error
func test2(_ test: @autoclosure (Int, Int) -> String) {} // expected-error

Language Guide > Closures > Autoclosures

belkadan commented 7 years ago

Yeah, we should probably just reject the autoclosure here.

belkadan commented 7 years ago

There's supposed to be an error emitted in TypeChecker::checkAutoClosureAttr, but we seem to not be getting there.

7156ccc1-d3b8-4460-882a-3f5802920f8f commented 7 years ago

@belkadan Why not improving it in general? The convenience of `@autoclsosure` disables a lot of closure features which is really odd. I'd love to hear some feedback on the list about it (I opened a thread last week). Trailing closures is also a convenient way of dealing with closures. So we could potentially merge the behavior from `@autoclosure` into the type, can't we?

7156ccc1-d3b8-4460-882a-3f5802920f8f commented 7 years ago
func foo(_ test: @autoclosure () -> String) {
    print(test())
}

foo("Swift")
foo({ "Swift" }) // not possible
foo { "Swift" } // not possible

Interesting issue #1:

func foo(_ closure: @autoclosure @escaping () -> Void) {}
// Xcode 8.3 autocompletion suggests also this form
foo {
    <#code#>
}

Issue #2:

extension Bool {

    /// #&#8203;1
    func whenTrue(execute closure: () -> Void) {
        if self { closure() }
    }

    /// #&#8203;2
    func whenTrue(execute closure: @autoclosure () -> Void) {
        if self { closure() }
    }

    /// #&#8203;3
    func whenTrue<T>(execute closure: @autoclosure () -> T) -> T? {
        if self { return closure() }
        return nil
    }
}

let test: () -> Void = { }
// #&#8203;3 wins and produces a wrong type () -> (() -> Void)?, but I expect #&#8203;1 here
// () -> Void?
true.whenTrue(execute: test) 

Can `@autoclosure` create retain cycles? If so, there is no way to deal with them.

7156ccc1-d3b8-4460-882a-3f5802920f8f commented 7 years ago

Furthermore you can only pass an autoclosure to another autoclosure, but not a closure of the same signature to an autoclosure, which is not syntactic convenience anymore. I think this attribute is simply boilerplate and should be merged into the closure type so no closure features gets disabled and it really becomes a syntactic convenience for single expressions.

belkadan commented 7 years ago

There are lots of improvements we could make to autoclosure, but allowing them to take arguments isn't one of them. Please file separate bugs for the other issues!

7156ccc1-d3b8-4460-882a-3f5802920f8f commented 7 years ago

@belkadan Just one last question: can I still file an improvement issue to allow arguments on autoclosure?

I have a bunch of methods that could use all of that mentioned functionality combined. For instance one does look like this:

open func push(_ viewController: UIViewController,
                         option: Option = .animated,
                         with animator: (Transition) -> Animator = animator(for:))

// I want it to be like this:
open func push(_ viewController: UIViewController,
                         option: Option = .animated,
                         with animator: @autoclosure (Transition) -> Animator = animator(for:))

So that the API user can decide if he only needs to pass a simple init expression or a more complex closure to that method:

containerViewController.push(someVC, with: CustomAnimator(for: $0 /* transition */))

// or
containerViewController.push(someVC, with: { 
    let animator = OtherCustomAnimator(for: $0 /* transition */) 
    ...
    return animator
})

Edit: I understand that it will not be high priority. 😉

belkadan commented 7 years ago

Oh, I see what you mean. You could allow arguments on an autoclosure by having them be ignored by default. I'd personally be against such a feature (it makes the language more complicated) but it could be sound. It would have to go through swift-evolution, of course.

7156ccc1-d3b8-4460-882a-3f5802920f8f commented 7 years ago

@belkadan Well I don't want only allow arguments if there is a default for that particular closure, it's only that because my API let's the user override the default behavior of the method by making it as a default argument - it's really is an implementation/design detail only.

func withTen(_ closure: @autoclosure (Int) -> Int) {
    print(closure(10))
}

withTen($0 + 32) // prints 42

I want to allow shorthand arguments to support arguments in autoclosures in general. And sure swift-evolution is the obvious place for such a pitch, which I already did:

belkadan commented 7 years ago

No, we're definitely not going to allow writing $0 + 32 in an autoclosure spot. That'd be way too subtle when that expression was itself within a closure (an explicit one) with anonymous arguments.

CodaFi commented 7 years ago

Resolved by the merge of 10784. Turns out we did have a diagnostic for this, it just got lost in the transition from Swift 2-style decl attributes to Swift 3-style type attributes.