Warning on the '(T...) -> U' to '(T...) -> ()' conversion


(Joe Groff) #1

Javier Soto brought up a case where his ReactiveCocoa code silently did the wrong thing by implicitly discarding the result of a closure:

https://twitter.com/Javi/status/695680700033306624

Since it sounds like we plan on making `@warn_unused_result` the default, it seems natural to also warn when a closure has its result type converted away to Void. If there's one 'return' statement, we could dive into the return expression to decide whether to warn based on the function whose result is getting ignored.

-Joe


(Joe Groff) #2

There's another axis of intent in whether the closure was written with an explicit return or not. `{ foo($0) }` is ambivalent to whether it intends to propagate the value returned by `foo` or not, whereas `{ return foo($0) }` is explicitly trying to do so. IMO we have a stronger case to warn if the `return` is explicit in the closure body.

-Joe

···

On Feb 8, 2016, at 7:25 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Javier Soto brought up a case where his ReactiveCocoa code silently did the wrong thing by implicitly discarding the result of a closure:

https://twitter.com/Javi/status/695680700033306624

Since it sounds like we plan on making `@warn_unused_result` the default, it seems natural to also warn when a closure has its result type converted away to Void. If there's one 'return' statement, we could dive into the return expression to decide whether to warn based on the function whose result is getting ignored.


(Jordan Rose) #3

We added the conversion to Void specifically because a lot of things return values that the caller doesn't care about. The most common case was

dispatch_sync(queue) { [weak sefl]
  self?.doTheThing()
}

I think it only makes sense to warn on conversion to Void if the expression being converted is itself one we would have warned on with @warn_unused_result, and in fact we do this already even in Swift 2.1.

func test() -> Bool { return false }
@warn_unused_result
func test2() -> Bool { return true }

func use(_: () -> Void) {}

use { test() }
use { test2() }

So the real problem is that != isn't marked @warn_unused_result, which is SR-245 <https://bugs.swift.org/browse/SR-245>.

(Although I agree that if there's an explicit return and the expression doesn't have Void type, we should warn on the Void conversion as well. Or even error.)

Jordan

···

On Feb 8, 2016, at 19:25 , Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Javier Soto brought up a case where his ReactiveCocoa code silently did the wrong thing by implicitly discarding the result of a closure:

https://twitter.com/Javi/status/695680700033306624

Since it sounds like we plan on making `@warn_unused_result` the default, it seems natural to also warn when a closure has its result type converted away to Void. If there's one 'return' statement, we could dive into the return expression to decide whether to warn based on the function whose result is getting ignored.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(James Hillhouse IV) #4

+1 for this idea.

···

On Feb 8, 2016, at 9:25 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Javier Soto brought up a case where his ReactiveCocoa code silently did the wrong thing by implicitly discarding the result of a closure:

https://twitter.com/Javi/status/695680700033306624

Since it sounds like we plan on making `@warn_unused_result` the default, it seems natural to also warn when a closure has its result type converted away to Void. If there's one 'return' statement, we could dive into the return expression to decide whether to warn based on the function whose result is getting ignored.

-Joe
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Erica Sadun) #5

if foo is -> Void, it's a gimme.
if foo is -> T with @warn_unused_result, it's a gimme
if foo is -> T without @warn_unused_result, dunno

-- E

···

On Feb 8, 2016, at 8:32 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Feb 8, 2016, at 7:25 PM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

Javier Soto brought up a case where his ReactiveCocoa code silently did the wrong thing by implicitly discarding the result of a closure:

https://twitter.com/Javi/status/695680700033306624

Since it sounds like we plan on making `@warn_unused_result` the default, it seems natural to also warn when a closure has its result type converted away to Void. If there's one 'return' statement, we could dive into the return expression to decide whether to warn based on the function whose result is getting ignored.

There's another axis of intent in whether the closure was written with an explicit return or not. `{ foo($0) }` is ambivalent to whether it intends to propagate the value returned by `foo` or not, whereas `{ return foo($0) }` is explicitly trying to do so. IMO we have a stronger case to warn if the `return` is explicit in the closure body.


(Vanderlei Martinelli) #6

@EricaSadun I’d like to say that many of us are not native English
speakers. I had to find what is the meaning of “gimme” and “dunno”. And it
is a good thing, so I can learn more English words and slangs. But at the
same time sometimes I read these messages in a place where it is not
possible to consult dictionaries or when I just do not have time to do this.

OK... Let’s explain the words:

gimme. noun. a thing that is very easy to perform or obtain, especially in
a game or sport: the kick would hardly be a gimme in that wind

dunno. contraction. (I) do not know. origin mid 19th cent.: representing an
informal pronunciation.

:wink:

Dunno, but the more we use internationally known words may be a gimme to
track the subject that is Swift evolution.

···

On Tue, Feb 9, 2016 at 1:47 AM, Erica Sadun via swift-evolution < swift-evolution@swift.org> wrote:

> On Feb 8, 2016, at 8:32 PM, Joe Groff via swift-evolution < > swift-evolution@swift.org> wrote:
>
>
>> On Feb 8, 2016, at 7:25 PM, Joe Groff via swift-evolution < > swift-evolution@swift.org> wrote:
>>
>> Javier Soto brought up a case where his ReactiveCocoa code silently did
the wrong thing by implicitly discarding the result of a closure:
>>
>> https://twitter.com/Javi/status/695680700033306624
>>
>> Since it sounds like we plan on making `@warn_unused_result` the
default, it seems natural to also warn when a closure has its result type
converted away to Void. If there's one 'return' statement, we could dive
into the return expression to decide whether to warn based on the function
whose result is getting ignored.
>
> There's another axis of intent in whether the closure was written with
an explicit return or not. `{ foo($0) }` is ambivalent to whether it
intends to propagate the value returned by `foo` or not, whereas `{ return
foo($0) }` is explicitly trying to do so. IMO we have a stronger case to
warn if the `return` is explicit in the closure body.

if foo is -> Void, it's a gimme.
if foo is -> T with @warn_unused_result, it's a gimme
if foo is -> T without @warn_unused_result, dunno

-- E

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Erica Sadun) #7

I sincerely apologize for the lack of professionalism in my response. I will try to maintain higher standards in future communication.

Best regards. -- E

···

On Feb 8, 2016, at 9:41 PM, Vanderlei Martinelli <vmartinelli@alecrim.com> wrote:

@EricaSadun I’d like to say that many of us are not native English speakers. I had to find what is the meaning of “gimme” and “dunno”. And it is a good thing, so I can learn more English words and slangs. But at the same time sometimes I read these messages in a place where it is not possible to consult dictionaries or when I just do not have time to do this.

OK... Let’s explain the words:

gimme. noun. a thing that is very easy to perform or obtain, especially in a game or sport: the kick would hardly be a gimme in that wind

dunno. contraction. (I) do not know. origin mid 19th cent.: representing an informal pronunciation.

:wink:

Dunno, but the more we use internationally known words may be a gimme to track the subject that is Swift evolution.

On Tue, Feb 9, 2016 at 1:47 AM, Erica Sadun via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

> On Feb 8, 2016, at 8:32 PM, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>
>
>> On Feb 8, 2016, at 7:25 PM, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>
>> Javier Soto brought up a case where his ReactiveCocoa code silently did the wrong thing by implicitly discarding the result of a closure:
>>
>> https://twitter.com/Javi/status/695680700033306624
>>
>> Since it sounds like we plan on making `@warn_unused_result` the default, it seems natural to also warn when a closure has its result type converted away to Void. If there's one 'return' statement, we could dive into the return expression to decide whether to warn based on the function whose result is getting ignored.
>
> There's another axis of intent in whether the closure was written with an explicit return or not. `{ foo($0) }` is ambivalent to whether it intends to propagate the value returned by `foo` or not, whereas `{ return foo($0) }` is explicitly trying to do so. IMO we have a stronger case to warn if the `return` is explicit in the closure body.

if foo is -> Void, it's a gimme.
if foo is -> T with @warn_unused_result, it's a gimme
if foo is -> T without @warn_unused_result, dunno

-- E

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Javier Soto) #8

Hi!

I'd like to resurface this thread. Is this something that deserves a
fully-fledged proposal? Should we add this to the @warn_unused_result
by-default proposal? https://github.com/apple/swift-evolution/pull/191/files

Thanks!

···

On Mon, Feb 8, 2016 at 8:44 PM Erica Sadun via swift-evolution < swift-evolution@swift.org> wrote:

I sincerely apologize for the lack of professionalism in my response. I
will try to maintain higher standards in future communication.

Best regards. -- E

On Feb 8, 2016, at 9:41 PM, Vanderlei Martinelli <vmartinelli@alecrim.com> > wrote:

@EricaSadun I’d like to say that many of us are not native English
speakers. I had to find what is the meaning of “gimme” and “dunno”. And it
is a good thing, so I can learn more English words and slangs. But at the
same time sometimes I read these messages in a place where it is not
possible to consult dictionaries or when I just do not have time to do this.

OK... Let’s explain the words:

gimme. noun. a thing that is very easy to perform or obtain, especially in
a game or sport: the kick would hardly be a gimme in that wind

dunno. contraction. (I) do not know. origin mid 19th cent.: representing an
informal pronunciation.

:wink:

Dunno, but the more we use internationally known words may be a gimme to
track the subject that is Swift evolution.

On Tue, Feb 9, 2016 at 1:47 AM, Erica Sadun via swift-evolution < > swift-evolution@swift.org> wrote:

> On Feb 8, 2016, at 8:32 PM, Joe Groff via swift-evolution < >> swift-evolution@swift.org> wrote:
>
>
>> On Feb 8, 2016, at 7:25 PM, Joe Groff via swift-evolution < >> swift-evolution@swift.org> wrote:
>>
>> Javier Soto brought up a case where his ReactiveCocoa code silently
did the wrong thing by implicitly discarding the result of a closure:
>>
>> https://twitter.com/Javi/status/695680700033306624
>>
>> Since it sounds like we plan on making `@warn_unused_result` the
default, it seems natural to also warn when a closure has its result type
converted away to Void. If there's one 'return' statement, we could dive
into the return expression to decide whether to warn based on the function
whose result is getting ignored.
>
> There's another axis of intent in whether the closure was written with
an explicit return or not. `{ foo($0) }` is ambivalent to whether it
intends to propagate the value returned by `foo` or not, whereas `{ return
foo($0) }` is explicitly trying to do so. IMO we have a stronger case to
warn if the `return` is explicit in the closure body.

if foo is -> Void, it's a gimme.
if foo is -> T with @warn_unused_result, it's a gimme
if foo is -> T without @warn_unused_result, dunno

-- E

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Javier Soto