Make non-void functions @warn_unused_result by default

Evolution Discussion
1

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

Motivation

It is a rare case for a result of a function to be unused – and most often it's caused by programmer having too little knowledge of the API. This, in my opinion, is a reasonable area of improvement for the compiler.

I also noticed that many of stdlib's functions are marked with this attribute, which supports my point of this being the default behavior.

Example code

The following should be default behavior:

func square(x: Int) -> Int { return x * x }
square(2) // warning: result of call to 'square' unused

Currently, unless annotated by @warn_unused_result, the compiler will not trigger any warnings.

Supporting old behavior

If, sometimes, it is okay to not use the result, an opposite @suppress_unused_result attribute could be used instead.

@suppress_unused_result func square(x: Int) -> Int { return x * x }
square(2) // no warning

Impact on existing code

The existing bare @warn_unused_result attributes will become redundant and can be easily removed by migrator. However, if an existing attribute uses message or mutable_variant argument, it may be left intact, since it provides non-obvious informational value.

Implementing the above proposal would definitely make the code clearer and intentional, without compromising any major use cases.

I would be happy to hear your thought on this.

Regards,
Adrian Kashivskyy

2

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

+1, this would be great. It would also be a great starter project for someone who wants to get into hacking on the Swift compiler.

-Chris

···

On Dec 10, 2015, at 2:58 PM, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

Motivation

It is a rare case for a result of a function to be unused – and most often it's caused by programmer having too little knowledge of the API. This, in my opinion, is a reasonable area of improvement for the compiler.

I also noticed that many of stdlib's functions are marked with this attribute, which supports my point of this being the default behavior.

Example code

The following should be default behavior:

func square(x: Int) -> Int { return x * x }
square(2) // warning: result of call to 'square' unused

Currently, unless annotated by @warn_unused_result, the compiler will not trigger any warnings.

Supporting old behavior

If, sometimes, it is okay to not use the result, an opposite @suppress_unused_result attribute could be used instead.

@suppress_unused_result func square(x: Int) -> Int { return x * x }
square(2) // no warning

Impact on existing code

The existing bare @warn_unused_result attributes will become redundant and can be easily removed by migrator. However, if an existing attribute uses message or mutable_variant argument, it may be left intact, since it provides non-obvious informational value.

Implementing the above proposal would definitely make the code clearer and intentional, without compromising any major use cases.

I would be happy to hear your thought on this.

Regards,
Adrian Kashivskyy

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

3

There are a number of operations that sensibly return values, but are frequently used only for side effects, such as `pop` on a stack. I agree they're in the minority, and @warn_unused_result's polarity is backwards, but they're a big enough minority that the inverse attribute is valuable.

-Joe

···

On Dec 10, 2015, at 3:28 PM, Matthew Johnson via swift-evolution <swift-evolution@swift.org> wrote:

Why should unused results be acceptable? Why not require callers to explicitly throw away the result:

let _ = funcReturningValueThatIsIgnored()

Yes this is extra syntax, but it also makes it clear that the value was intended to be ignored and the function was only executed for side-effects.

4

I strongly don’t like this idea. This being said, if it happens ann attribute that says "unused result are acceptable" must be created. Makes me wince.

Guillaume Lessard

···

On 10 déc. 2015, at 15:58, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

5

We have discussed this internally in the past, and we found that
non-mutating methods on structs and enums are usually @warn_unused_result,
it is a quite strong heuristic. With classes, especially Cocoa ones, it
might not be the case. It would be good if someone investigated how this
change would affect a class-heavy library, e.g., Foundation.

Dmitri

···

On Thu, Dec 10, 2015 at 2:58 PM, Adrian Kashivskyy via swift-evolution < swift-evolution@swift.org> wrote:

I think non-void functions should warn about their return value being
unused *by default*, thus eliminating the need to attribute them with
@warn_unused_result.

*Motivation*

It is a rare case for a result of a function to be unused – and most often
it's caused by programmer having too little knowledge of the API. This, in
my opinion, is a reasonable area of improvement for the compiler.

I also noticed that many of stdlib's functions are marked with this
attribute, which supports my point of this being the default behavior.

*Example code*
The following should be default behavior:

func square(x: Int) -> Int { return x * x }
square(2) // warning: result of call to 'square' unused

Currently, unless annotated by @warn_unused_result, the compiler will not
trigger any warnings.

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/

6

Why should unused results be acceptable? Why not require callers to explicitly throw away the result:

let _ = funcReturningValueThatIsIgnored()

Yes this is extra syntax, but it also makes it clear that the value was intended to be ignored and the function was only executed for side-effects.

···

On Dec 10, 2015, at 5:23 PM, Guillaume Lessard via swift-evolution <swift-evolution@swift.org> wrote:

On 10 déc. 2015, at 15:58, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

I strongly don’t like this idea. This being said, if it happens ann attribute that says "unused result are acceptable" must be created. Makes me wince.

Guillaume Lessard

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

7

FWIW, since patterns can exist on the left side of an assignment - a common example would be "(x,y) = foo()", you don’t need the let. This is sufficient:

  _ = funcReturningValueThatIsIgnored()

-Chris

···

On Dec 10, 2015, at 3:28 PM, Matthew Johnson via swift-evolution <swift-evolution@swift.org> wrote:

Why should unused results be acceptable? Why not require callers to explicitly throw away the result:

let _ = funcReturningValueThatIsIgnored()

Yes this is extra syntax, but it also makes it clear that the value was intended to be ignored and the function was only executed for side-effects.

8

There are a number of operations that sensibly return values, but are frequently used only for side effects, such as `pop` on a stack. I agree they're in the minority, and @warn_unused_result's polarity is backwards, but they're a big enough minority that the inverse attribute is valuable.

Sure, that makes sense as long as it’s used relatively sparingly! :)

9

Matthew, I actually like your idea better than using an opposite "@suppress_unused_result" attribute. In addition, it will even be easier to implement, since "let _" syntax is already supported in the language.

Pozdrawiam – Regards,
Adrian Kashivskyy

···

Wiadomość napisana przez Matthew Johnson <matthew@anandabits.com> w dniu 11.12.2015, o godz. 00:28:

Why should unused results be acceptable? Why not require callers to explicitly throw away the result:

let _ = funcReturningValueThatIsIgnored()

Yes this is extra syntax, but it also makes it clear that the value was intended to be ignored and the function was only executed for side-effects.

On Dec 10, 2015, at 5:23 PM, Guillaume Lessard via swift-evolution <swift-evolution@swift.org> wrote:

On 10 déc. 2015, at 15:58, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

I strongly don’t like this idea. This being said, if it happens ann attribute that says "unused result are acceptable" must be created. Makes me wince.

Guillaume Lessard

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

10

I'm sure there are plenty of methods in Foundation that return a value which can be ignored. However, as Joe noticed, such functions are generally in minority.

I'd rather see a couple of `@allow_unused_result` than a dozen of `@warn_unused_result`.

Pozdrawiam – Regards,
Adrian Kashivskyy

···

Wiadomość napisana przez Dmitri Gribenko <gribozavr@gmail.com> w dniu 11.12.2015, o godz. 01:41:

On Thu, Dec 10, 2015 at 2:58 PM, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

Motivation

It is a rare case for a result of a function to be unused – and most often it's caused by programmer having too little knowledge of the API. This, in my opinion, is a reasonable area of improvement for the compiler.

I also noticed that many of stdlib's functions are marked with this attribute, which supports my point of this being the default behavior.

Example code

The following should be default behavior:

func square(x: Int) -> Int { return x * x }
square(2) // warning: result of call to 'square' unused

Currently, unless annotated by @warn_unused_result, the compiler will not trigger any warnings.

We have discussed this internally in the past, and we found that non-mutating methods on structs and enums are usually @warn_unused_result, it is a quite strong heuristic. With classes, especially Cocoa ones, it might not be the case. It would be good if someone investigated how this change would affect a class-heavy library, e.g., Foundation.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com <mailto:gribozavr@gmail.com>>*/

11

I'm curious, what code did you look at to develop this heuristic? I've
noticed that the stdlib uses a lot of @warn_unused_result, but I don't
think I've ever actually seen anyone bother to use it in any of the third-
party Swift code that I've inspected.

My general feeling here is that, while most non-mutating functions that
return results probably aren't worth calling if you ignore the results,
that doesn't mean that @warn_unused_result is actually something that
should necessarily be expected to put on the declaration. Where it would
primarily be expected is on functions that look like they might be
mutating but aren't (think `sort()` vs `sortInPlace()`), or on functions
that return an object whose deinit triggers behavior that negates the
function call (e.g. some sort of `open()` that returns a File object). I
think the stdlib uses it a disproportionate amount because the stdlib is
used by everybody, and adding the attribute to stdlib declarations
therefore has a huge reach for little effort and in particular it may
help neophyte programmers.

I will accept that most calls to nonmutating methods on structs and
enums are probably good candidates for @warn_unused_result, I'm just
arguing that in most cases outside of the stdlib there's low reward for
adding it, and I worry that if we make it the default then we run the
risk of punishing users of side-effectful APIs where the API designer
never actually tried ignoring the results.

I do support Kametrixom's idea of a @pure attribute that implies both
@effects(readnone) and @warn_unused_result.

-Kevin Ballard

···

On Thu, Dec 10, 2015, at 04:41 PM, Dmitri Gribenko via swift-evolution wrote:

We have discussed this internally in the past, and we found that non-
mutating methods on structs and enums are usually @warn_unused_result,
it is a quite strong heuristic. With classes, especially Cocoa ones,
it might not be the case. It would be good if someone investigated
how this change would affect a class-heavy library, e.g., Foundation.

12

strong +1 from me. @warn_unused_result is almost always the right answer in the standard library, and the few cases where it isn’t are very easy to notice (when the call has side-effects and the return value is incidental).

-Dave

···

On Dec 10, 2015, at 2:58 PM, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

Motivation

It is a rare case for a result of a function to be unused – and most often it's caused by programmer having too little knowledge of the API. This, in my opinion, is a reasonable area of improvement for the compiler.

I also noticed that many of stdlib's functions are marked with this attribute, which supports my point of this being the default behavior.

Example code

The following should be default behavior:

func square(x: Int) -> Int { return x * x }
square(2) // warning: result of call to 'square' unused

Currently, unless annotated by @warn_unused_result, the compiler will not trigger any warnings.

Supporting old behavior

If, sometimes, it is okay to not use the result, an opposite @suppress_unused_result attribute could be used instead.

@suppress_unused_result func square(x: Int) -> Int { return x * x }
square(2) // no warning

Impact on existing code

The existing bare @warn_unused_result attributes will become redundant and can be easily removed by migrator. However, if an existing attribute uses message or mutable_variant argument, it may be left intact, since it provides non-obvious informational value.

Implementing the above proposal would definitely make the code clearer and intentional, without compromising any major use cases.

I would be happy to hear your thought on this.

Regards,
Adrian Kashivskyy

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

13

It is a rare case for a result of a function to be unused – and most often it's caused by programmer having too little knowledge of the API.

I don't think this is a real life problem. When you have a class with something like "append" that doesn't change the receiver but returns a new object, it could be a valuable hint, but I don't like systems which assume that their users don't know what they are doing; and if this is actually the case, you are fighting a tough battle where a little warning wont help.

Please also note that ignoring a non-void return value can be absolutely ok in many situations: One example is returning self instead of useless void, which allows to build fluent interfaces - just because Cocoa didn't adopt the concept of method chaining from Smalltalk, it is still a interesting pattern which is used by other frameworks.

Imho warnings should be reserved for things that can be dangerous, and not to educate programmers...

14

Hi All.

Being naive here (also very difficult to say this clearly and succinctly), but why is this the choice of the writer of the function rather than the person using the function?

That is, I would think that if I were worried about unused return results in my code, I would have some kind of flag on the client side, and not on the provider side.

Does this question make any sense?

-Kenny

···

On Dec 10, 2015, at 2:58 PM, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

Motivation

It is a rare case for a result of a function to be unused – and most often it's caused by programmer having too little knowledge of the API. This, in my opinion, is a reasonable area of improvement for the compiler.

I also noticed that many of stdlib's functions are marked with this attribute, which supports my point of this being the default behavior.

Example code

The following should be default behavior:

func square(x: Int) -> Int { return x * x }
square(2) // warning: result of call to 'square' unused

Currently, unless annotated by @warn_unused_result, the compiler will not trigger any warnings.

Supporting old behavior

If, sometimes, it is okay to not use the result, an opposite @suppress_unused_result attribute could be used instead.

@suppress_unused_result func square(x: Int) -> Int { return x * x }
square(2) // no warning

Impact on existing code

The existing bare @warn_unused_result attributes will become redundant and can be easily removed by migrator. However, if an existing attribute uses message or mutable_variant argument, it may be left intact, since it provides non-obvious informational value.

Implementing the above proposal would definitely make the code clearer and intentional, without compromising any major use cases.

I would be happy to hear your thought on this.

Regards,
Adrian Kashivskyy

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

15

My initial gut reaction is that I don’t mind this change happening, but I would much prefer using a @suppress_unused_result attribute than forcing the user to "let _ = “. The API designer should know whether the return value is necessary or not, and make it easy for the consumer to decide whether they want to use it or not.

···

On Dec 10, 2015, at 3:30 PM, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

Matthew, I actually like your idea better than using an opposite "@suppress_unused_result" attribute. In addition, it will even be easier to implement, since "let _" syntax is already supported in the language.

Pozdrawiam – Regards,
Adrian Kashivskyy

Wiadomość napisana przez Matthew Johnson <matthew@anandabits.com <mailto:matthew@anandabits.com>> w dniu 11.12.2015, o godz. 00:28:

Why should unused results be acceptable? Why not require callers to explicitly throw away the result:

let _ = funcReturningValueThatIsIgnored()

Yes this is extra syntax, but it also makes it clear that the value was intended to be ignored and the function was only executed for side-effects.

On Dec 10, 2015, at 5:23 PM, Guillaume Lessard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On 10 déc. 2015, at 15:58, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

I strongly don’t like this idea. This being said, if it happens ann attribute that says "unused result are acceptable" must be created. Makes me wince.

Guillaume Lessard

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

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

16

We are interested in seeing the actual data.

Dmitri

···

On Thu, Dec 10, 2015 at 4:47 PM, Adrian Kashivskyy <adrian.kashivskyy@me.com> wrote:

I'm sure there are plenty of methods in Foundation that return a value which
can be ignored. However, as Joe noticed, such functions are generally in
minority.

I'd rather see a couple of `@allow_unused_result` than a dozen of
`@warn_unused_result`.

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/

17

FWIW, if there was a widespread problem, we could have the importer slap @suppress_unused_result on all imported decls (at least, ones that didn’t have the clang warn_unused_result attribute).

This would allow swift code to have the “right” default policy without causing a ton of noise from imported APIs.

-Chris

···

On Dec 10, 2015, at 4:49 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

On Thu, Dec 10, 2015 at 4:47 PM, Adrian Kashivskyy > <adrian.kashivskyy@me.com> wrote:

I'm sure there are plenty of methods in Foundation that return a value which
can be ignored. However, as Joe noticed, such functions are generally in
minority.

I'd rather see a couple of `@allow_unused_result` than a dozen of
`@warn_unused_result`.

We are interested in seeing the actual data.

18

I’m in favor of this change, and I would rather see this go the route of an ‘@suppress_unused_result' attribute.

I’m in agreement that it makes sense to let the designer of the API determine whether they want to let people ignore the return value. Ignoring the return value need not be made explicit at the point of use with `let _ = …` if it is already established in the documentation that the return value may not need to be used.

FKL

···

On Dec 10, 2015, at 6:30 PM, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org> wrote:

Matthew, I actually like your idea better than using an opposite "@suppress_unused_result" attribute. In addition, it will even be easier to implement, since "let _" syntax is already supported in the language.

Pozdrawiam – Regards,
Adrian Kashivskyy

Wiadomość napisana przez Matthew Johnson <matthew@anandabits.com <mailto:matthew@anandabits.com>> w dniu 11.12.2015, o godz. 00:28:

Why should unused results be acceptable? Why not require callers to explicitly throw away the result:

let _ = funcReturningValueThatIsIgnored()

Yes this is extra syntax, but it also makes it clear that the value was intended to be ignored and the function was only executed for side-effects.

On Dec 10, 2015, at 5:23 PM, Guillaume Lessard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On 10 déc. 2015, at 15:58, Adrian Kashivskyy via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I think non-void functions should warn about their return value being unused by default, thus eliminating the need to attribute them with @warn_unused_result.

I strongly don’t like this idea. This being said, if it happens ann attribute that says "unused result are acceptable" must be created. Makes me wince.

Guillaume Lessard

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

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

19

Sure, I will be happy to conduct an investigation of iOS Foundation framework over the weekend and come back with numeric results.

Pozdrawiam – Regards,
Adrian Kashivskyy

···

Wiadomość napisana przez Dmitri Gribenko <gribozavr@gmail.com> w dniu 11.12.2015, o godz. 01:49:

On Thu, Dec 10, 2015 at 4:47 PM, Adrian Kashivskyy > <adrian.kashivskyy@me.com> wrote:

I'm sure there are plenty of methods in Foundation that return a value which
can be ignored. However, as Joe noticed, such functions are generally in
minority.

I'd rather see a couple of `@allow_unused_result` than a dozen of
`@warn_unused_result`.

We are interested in seeing the actual data.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/

20

I think we actually have two things colliding here: Think about it, why would we need a @suppress_unused_result attribute? Because of side effects! That's the only reason we would ever need to have this attribute. Now we already have the @effects(readnone) attribute to enable optimizations for pure functions. Now we can combine both of those into one: @pure. This attribute does this:
- It indicates that the function is purely functional (= no side-effects, same result for same input)
- It implicitly adds the @effects(readnone) attribute to enable such optimizations
- It implicitly adds the @warn_unused_result attribute

To make sure that the function actually is pure we need to check that every called function within is pure itself and no external variables get accessed.

Now the only thing to do is to make the compiler implicitly add the @pure keyword where the conditions are satisfied.

Something I haven't thought about yet: Not being able to read external variables (@effects(readonly)) are also functions without side effects, but they're not pure, since the result doesn't have to be the same for the same input. I don't want to suggest another keyword for that (we have too many already) but I believe this is actually what we want here.

Let me know what you think, FP ftw

– Kame

···

On 11 Dec 2015, at 02:31, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 4:49 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

On Thu, Dec 10, 2015 at 4:47 PM, Adrian Kashivskyy >> <adrian.kashivskyy@me.com> wrote:

I'm sure there are plenty of methods in Foundation that return a value which
can be ignored. However, as Joe noticed, such functions are generally in
minority.

I'd rather see a couple of `@allow_unused_result` than a dozen of
`@warn_unused_result`.

We are interested in seeing the actual data.

FWIW, if there was a widespread problem, we could have the importer slap @suppress_unused_result on all imported decls (at least, ones that didn’t have the clang warn_unused_result attribute).

This would allow swift code to have the “right” default policy without causing a ton of noise from imported APIs.

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