Today, I (unfortunately) learned this is valid Swift and compiles:
func foo(_: () -> Void) {}
foo { return 42 }
please note that the closure is declared as () -> Void and we're returning 42 which is an Int. This works with any return value and the compiler just implicitly converts the closure's type to () -> Void and throws away the return value. It does issue a warning but I don't think that's enough.
The only thing I can see this doing is creating bugs out of thin air. Btw, it also prevents you from using XCTAssertNoThrow(try doSomething { ... } as Void) as a convenient way to check that the return value is actually Void in certain cases. I filed this as https://bugs.swift.org/browse/SR-7478 but it 'works as designed' and therefore I would like pitching to change the design.
Whilst we're here: What was the rationale for this when it was designed?
Please let me know what you think.
While I agree that this is a bit wonky and ideally should be an error, I don't see how it can introduce bugs at the call site. The only thing I can think of is the writer of foo didn't actually want void. But it should be immediate as soon as they write the rest of their function they got the signature wrong.
I put an example in the ticket: In SwiftNIO we have an EventLoop.submit<T>(_ body: () -> T) -> EventLoopFuture<T> function which returns a future that is fulfilled when the operation has returned. If you now submit an operation that itself returns a future, say channel.connect(...) -> EventLoopFuture<Void> this will look like this:
eventLoop.submit {
someChannel.connect(...)
}
the overall return value of this will be EventLoopFuture<EventLoopFuture<Void>>. And in unit tests you often wait to synchronously wait until that has happened which you might want to write as
() = try eventLoop.submit {
someChannel.connect(...)
}.wait()
this looks all okay, compiles (even without a warning(!) for me) but you just created a race condition because you don't want to wait for the connect to start executing but rather for it to have finished executing. So what you actually want to write is
try eventLoop.submit {
someChannel.connect(...)
}.wait().wait()
(note the second .wait()).
Sure, we should totally have a submit method which does func submit<T>(_ body: () -> EventLoopFuture<T>) -> EventLoopFuture<T> which flatMap/thens the futures but that isn't the point here. The point is that you can write totally correct looking code (and even "make sure" it's Void by doing as Void or () = and still you're dropping a value on the floor).
In synchronous programming, Swift is (rightly) really pedantic about you consuming all return values unless they're Void or you decorate the function as @discardableResult.
2 Likes
I see. I agree that is a potential pitfall, but I don't think we can really change this, can we? For example what happens when calling functions that return @discardableResult? Not that it really matters that it's @discardableResult, but in this case we don't really care about the return value.
With this new change, would this be an error now?
func takesFunc(_ fun: () -> ()) { }
@discardableResult
func returnsInt() -> Int { return 5 }
takesFunc { returnsInt() }
1 Like
Thanks, that's is an interesting question that should be discussed. I'd be happy either way. If it's marked as @discardableResult I guess I can see that it might be expected for the closure's type to be implicitly changed to () -> Void.
I think I vaguely recall that not doing this implicit conversion was causing a lot of trouble when interacting with single line closures implicitly returning their result. This would have been a long time ago though, so perhaps language changes have made this less of an issue now. Or maybe the changes to warn on discarding return values that aren't @discardableResult have made this implicit conversion less desirable.