swiftlang / swift

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

[SR-1528] Swift compiler requires closures without parameter lists to reference all arguments #44137

Open swift-ci opened 8 years ago

swift-ci commented 8 years ago
Previous ID SR-1528
Radar rdar://problem/13254540
Original Reporter erica (JIRA User)
Type Bug
Environment Swift 2.2
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 10 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 2ed8141c3cd057578dc20e6b58317765

is duplicated by:

relates to:

Issue Description:

Swift closures that do not explicitly declare an internal parameter list must reference all arguments using implicit $n shorthand names. If they do not, Swift complains that the contextual type for the closure argument "expects n arguments, which cannot be implicitly ignored." This requirement diminishes the efficacy of Swift's $n syntactic sugar. Eliminating the requirement means:

All the following closures should be valid and compile without error or warning:

{{let _: () -> Void = {}
let _: (Int) -> Void = {}
let _: (Int, Int) -> Int = { 5 }
let _: (Int, Int) -> Int = { $0 }
let _: (Int, Int) -> Int = { $1 }

doThing(withCompletion: {})
let x: (T) -> Void = {}}}

Chris L writes: "I consider this to be an obvious bug in the compiler. I don’t think it requires a proposal. Unfortunately it is non-trivial to fix…"

belkadan commented 8 years ago

I'm not convinced this is an "obvious bug". We deliberately put in checking for this even whene there is plenty of context because we didn't want people to accidentally ignore parameters. Maybe the compromise is to remove that check for single-expression closures?

swift-ci commented 8 years ago

Comment by erica sadun (JIRA)

Does stuff still work after removing that check? If so, that works for me. However, I'd clear the policy with coreswift team. I just happen to have a proposal if you want to submit it to them.

belkadan commented 8 years ago

I don't actually know if it works. I was (mildly) objecting to removing it for all closures without going through evolution.

jckarter commented 8 years ago

This was discussed on swift-evolution, and Chris declared it an "obvious bug" there.

swift-ci commented 8 years ago

Comment by erica sadun (JIRA)

https://gist.github.com/erica/3731e24fc252c8e66850e0e02f491281

davidbjames commented 6 years ago

This is a very annoying bug. I don't see why we need to protect people from "accidentally" ignoring parameters. In regular functions it's possible to ignore params without error (not that it makes sense).

John McCall also considers this a bug, via:
https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160118/007095.html

See also duplicate SR-6612 for other code examples and inexplicable error messages.

davidbjames commented 6 years ago

An example: one may have a closure which is a factory of sorts. The closure may provide parameters from which to construct a Something. Alternatively, one may also pull that information from surrounding lexical scope to create a Something OR maybe just return a default Something.

func factory(_ closure:(Context) -> Something) { .. }
foo.factory { return Something() } // ERROR: "expects 1 argument which cannot be implicitly ignored" 

In any case, using the input data should optional. Sometimes you use it, sometimes you don't.

Pulling another example from the dupe ^^, an iterator that provides an index. Sometimes you reference the index, sometimes you don't:

func each(_ closure: (T, Int) -> Void) { .. }
// should be able to call using first parameter only:
foo.each { $0.doSomething } // ERROR: "expects 2 arguments, but 1 was used"
pcantrell commented 6 years ago

Copied from another bug marked as a dup, concise test cases which may be helpful:

func foo(_ closure:(Int,Int,Int)->Void) {
    closure(1,2,3)
}

// All of these should work: (last one is debatable; others are not IMO)
foo { print($0) } // ERROR: "expects 3 arguments, but 1 was used"
foo { print($1) } // ERROR: "expects 3 arguments, but 2 were used" Huh? There was only 1 used.
foo { print($0,$1) } // ERROR: "expects 3 arguments, but 2 were used"
foo { print($0,$1,$2) } // OK
foo { print($2) } // OK
foo { } // ERROR: "expects 3 arguments, which cannot be implicitly ignored"
jeremyabannister commented 5 years ago

I would enjoy seeing this change implemented. I've wanted this in the past when I wanted to create an array of reference type instances in one line. The example code I wished would work is this:

let numberOfViews = 20
let views = (0 ..< numberOfViews).map { UIView() }

I forget now what my solution was, but I think I ended up finding a different one-liner for this purpose. But it definitely wasn't:

let views = Array(repeating: UIView(), count: numberOfViews)

which of course yields an array with repeated references to the same UIView instance.

jeremyabannister commented 5 years ago

Oh, right, the fix is just to explicitly ignore the input:

let views = (0 ..< numberOfViews).map { _ in UIView() }

Regardless, would be nice to get rid of that extra little bit of boilerplate.

swift-ci commented 5 years ago

Comment by Michaell Long (JIRA)

Just noting that this issue appears all of the time when using RxSwift.

let saved = saving
  .filter { $0.contact != nil }
  .map { _ in "Contact information saved." }

let processing = Observable
  .from([validating.map { _ in true }, saved.map { _ in false }])
  .merge()

In both of the above cases I don't care about the incoming value and I want to replace the value in the stream with a new value. In other words, the code should look like:

let saved = saving
  .filter { $0.contact != nil }
  .map { "Contact information saved." }

let processing = Observable
  .from([validating.map { true }, saved.map { false }])
  .merge()

But which you can't do, because Swift requires the boilerplate "_ in" when the value is ignored.

swift-ci commented 4 years ago

Comment by Steven Van Impe (JIRA)

The Swift Programming Language states that:

“If you use these shorthand argument names within your closure expression, you can omit the closure’s argument list from its definition, and the number and type of the shorthand argument names will be inferred from the expected function type.”

If this is indeed a bug, can you bump its priority a bit? My students run into this issue so often that I actually have to include it in my course, and try to sell it as a safety feature.

davidbjames commented 3 years ago

This is still a major hole in closure usage. Given multiple arguments using shorthand argument names, the compiler requires the last argument to be declared.

// example with 2 argument closure
.test { $0.doSomething() // error }
.test { $1.doOtherThing() // ok }

This also kills code completion until you type the last argument. Sometimes you have to type the last argument on it's own line just so you can get code completion to work temporarily.

So there are two usability problems, the general limitation and code completion.

Since the limitation seems artificial (noted above as an "obvious bug") I suggest we resolve it now.

NOTE: fixing this would be non-breaking. Existing code would just work and new code could take advantage of the improvement. Can we do it now?

focusit905 commented 2 years ago

Just found an even sillier part of this.

If you write: func doSomething((T, Int) -> Int) {}

You've gotta use both params to use that closure

But if you write it as: func doSomething(((T, Int)) -> Int) {}

You now have a tuple (note the extra pair of parenthesis). Code that uses both arguments will keep working as swift will transparently turn that tuple into the arguments of the closure, no problem. If you then want to use only the first argument, you can refer to it as $0.0, treating the tuple as a tuple, and it'll work too, and won't complain.

Quite a silly workaround. Can we fix this please?