[Review] SE-0047 Defaulting non-Void functions so they warn on unused results

Evolution Announcements
1

Hello Swift community,

The review of “Defaulting non-Void functions so they warn on unused results” begins now and runs through March 21, 2016. The proposal is available here:

  swift-evolution/0047-nonvoid-warn.md at master · apple/swift-evolution · GitHub

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at:
  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

Thank you,

-Chris
Review Manager

2

• What is your evaluation of the proposal?

+1

  • Is the problem being addressed significant enough to warrant a change to Swift?

Probably, yes. It’s not a problem in the sense that something is broken, but I believe that the philosophy of swift regarding safety and the avoidance of bugs justifies this change.

  • Does this proposal fit well with the feel and direction of Swift?

Yes.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I don’t believe so.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read the proposal and followed the discussion on the ML.

- Will

3

The review of “Defaulting non-Void functions so they warn on unused results” begins now and runs through March 21, 2016. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md

  • What is your evaluation of the proposal?

Strongly in favor of this. I’ve added the existing @warn_unused_result all over the place in my own code and it helps keep me honest - but I’m kind of tired of the boilerplate!

  • Is the problem being addressed significant enough to warrant a change to Swift?

Calling it a “problem” is probably not the right term in this case, but yes, I think change is warranted due to Swift’s willingness to help developers avoid bugs by omission.

  • Does this proposal fit well with the feel and direction of Swift?

Yes, I believe so. Swift already alerts you when you don’t mutate a var or cover all possible switch cases and this feels like a very similar situation.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I don’t think I’ve used any with this sort of default.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Followed the mailing list discussion, read the proposal, encountered the issue in my own code.

l8r
Sean

4

  • What is your evaluation of the proposal?

I am in favor of the semantic, but I don't like `@discardableResult`; it's long and unwieldy. I would prefer that we either use a shorter word than "discardable", attach the keyword to the return type as discussed in "Future Directions", or both.

I also don't like that this proposal doesn't include an "Impact on existing code" section. We ought to decide whether the migrator will add `@discardableResult` to existing symbols or not.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes. I should use `@warn_unused_result`, but never bother because it's just too much of a hassle. My code will be safer with this change.

  • Does this proposal fit well with the feel and direction of Swift?

Yes. This kind of safety and explicitness is very Swifty.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I believe most languages I used either had "every function returns its last expression" semantics, or had an optional warning but no well-publicized way to suppress it for a given function.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read the proposal and participated in some of the discussion.

···

--
Brent Royal-Gordon
Architechies

5

After using Swift all day, every day since day #1, I'm strongly in favor, e.g. +1 here!

Now bring the @pure! :)

R+

···

Sent from my iPhone

On 16 Mar 2016, at 20:36, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The review of “Defaulting non-Void functions so they warn on unused results” begins now and runs through March 21, 2016. The proposal is available here:

   https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at:
   https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

   • What is your evaluation of the proposal?
   • Is the problem being addressed significant enough to warrant a change to Swift?
   • Does this proposal fit well with the feel and direction of Swift?
   • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
   • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris
Review Manager

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

6

  • What is your evaluation of the proposal?

Strong +1.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes, as it should help prevent mistakes (i.e- not adding it). It should also clean up code as I find myself adding the attribute a lot more than I omit it. In fact cases where I have a return value but don’t want a warning are very rare in my experience.

  • Does this proposal fit well with the feel and direction of Swift?

Yes, as it should prevent mistakes by making the less common case the exception.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Quickly re-read the proposal, but participated in the discussion.

7

In theory, I like this idea, and I think it fits with the general direction of Swift in terms of making APIs and their usage more explicit, particularly custom and third-party ones, but I’m concerned about what would happen if system APIs that were intended to have discardable results aren’t annotated at the same time that the feature is rolled out — while the intent is laudable, if the net result was that a bunch of spurious warnings are generated, then it makes the warning system less valuable.

That said, the problem isn’t necessarily difficult to solve — it’s just that it’s important that it actually be solved at the same time the feature is rolled out. The most obvious examples are the popViewController methods on UINavigationController — it’s very common, probably more common than not, to ignore the return value of these methods, but it would be unfortunate to need to explicitly ignore the results. (I suppose a counter argument here is that a lot of people may not even realize that those methods have a return value, but even if that’s the case, I suspect the typical pattern is to ignore the return.)

Assuming that most common APIs are audited (and that APIs like popViewController methods would be marked as discardable), then I think the proposal would have a positive impact. Perhaps one way to figure out the scope of the auditing would be to determine how many Apple SDK methods that return values aren’t already marked as warn_unused_result.

···

On Mar 16, 2016, at 3:36 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Defaulting non-Void functions so they warn on unused results” begins now and runs through March 21, 2016. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/maout ster/proposals/0047-nonvoid-warn.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at:
  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris
Review Manager

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

8

  • What is your evaluation of the proposal?

Great idea

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes I think so, currently the declaration line is very cluttered with lots of noise up front that hides what the line actually does. This proposal removes some of that noise.

  • Does this proposal fit well with the feel and direction of Swift?

Yes, Swift is clutter free and tries to pick as the default the most common case

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I have used languages with the current Swift behaviour of having to explicitly initiate the warning and they like Swift tended to be a pain. Either the declaration got cluttered or the annotation was omitted.

I have used products like lint for C and appreciated that it automatically warned about unused returned values.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Have followed along on Swift Evolution

9

  • What is your evaluation of the proposal?

+1

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

  • Does this proposal fit well with the feel and direction of Swift?

Yes.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I really don't like language compilers/interpreters that do not warn when a return value hasn't been consumed. It is one of the few aspects of Ruby that I dislike, and it causes no end of frustration. However, this enforces a policy while allowing a opt-out behavior, which is a plus. Either way, the developer was forced to consider the return value.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Quick read.

10

What is your evaluation of the proposal?

I worry about imported libraries. I think the default is right for code that was written directly in Swift, but for C and Objective-C code that was written in discartable-by-default environment, the APIs were designed with discardable results that in mind.

I also think it is imperative that the attribute is put just before the return type. Placing the attribute near the return type helps with the discoverability of the feature, which is important. It also makes it easier not to forget the attribute when adding a return type, which will reduce annoyances later.

Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?

Maybe, but I'm not sure. It might cause annoyances for people who aren't aware of the feature. Either they'll feel forced to use "_ = foo()" repeatedly (what the compiler will suggest to them), or they'll feel annoyed enough that they'll just remove the return value from the function. So whether this is a convenience or an inconvenience depends entirely on people's awareness of the feature.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Followed the discussion, read the proposal. Also checked whether adding @warn_unused_result everywhere in my code where appropriate would find bugs: it didn't.

···

--
Michel Fortin
https://michelf.ca

11

I am +1 on this proposal, which helps counter a common programmer error and encourages better-designed APIs.

Developers and users have three powerful ways to deal with this new behavior:

- Values that are meant to be optionally returned to the caller if the caller cares can be denoted by the API vendor using an inout argument of type T?.
- The API vendor can add the 'allow unused result' attribute to explicitly document that the API's semantics allow the return value to be ignored safely.
- The API consumer can use the "_ = returnsSomething()" pattern to denote that they explicitly don't care about the return value, regardless of the API vendor's intent.

Yes, it's true that any programmer can write incorrect code regardless of language features if they are sufficiently determined, but in my opinion this proposal moves the defaults towards better safety without laying a disproportionately onerous burden upon users and API vendors.

Best,
Austin

···

On Mar 17, 2016, at 3:36 AM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The review of “Defaulting non-Void functions so they warn on unused results” begins now and runs through March 21, 2016. The proposal is available here:

   https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at:
   https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager. When replying, please try to keep the proposal link at the top of the message:

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

   • What is your evaluation of the proposal?
   • Is the problem being addressed significant enough to warrant a change to Swift?
   • Does this proposal fit well with the feel and direction of Swift?
   • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
   • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris
Review Manager

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

12

      • What is your evaluation of the proposal?

+1 for default error/warning on ignoring returned value in regular code, -1 for
this behavior in playgrounds.

In production code, ignored return values are rare. In the cases where they are
ignored, prepending "_ =" makes the code clearer and more explicit.
In research code (playgrounds), as mentioned in the proposal, it's common to
write expressions just to see the evaluated result. Having to prepend "_ ="
everywhere would be bothersome and clutter the code. I guess it would make sense
to allow opting-in to this behavior in playgrounds.

      • Is the problem being addressed significant enough to warrant a change
      to Swift?

Yes, it will remove an entire class of bugs.

      • How much effort did you put into your review? A glance, a quick
      reading, or an in-depth study?

A quick reading.

13

One advantage that I see with the alternative of decorating the return type with @discardable, is that the attribute could be reused later if/when the out-only function parameters come to the language.

Something like:

func head(count: Int, tail: @discardable out String) -> String

To allow it to be safely called either as:

head = str.head(count:10, tail:&_)
head = str.head(count:10)

No need to discuss this possible usage now, just something to keep in mind when choosing the syntax.

Dany

···

Le 16 mars 2016 à 15:36, Chris Lattner via swift-evolution <swift-evolution@swift.org> a écrit :

Hello Swift community,

The review of “Defaulting non-Void functions so they warn on unused results” begins now and runs through March 21, 2016. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at:
  https://lists.swift.org/mailman/listinfo/swift-evolution

14

        • What is your evaluation of the proposal?

+1

Although I am not perfectly sure, I think it may be better to raise an
error instead of a warning when the code lacks both
`@discardableResult` and consuming a return value . When someone gets
the warning, we expect them to consume the result or ignore it
explicitly by `_ =`. If they keep the warning, it will make it easier
to miss other warnings. It is undesirable. Unlike other warnings, I
can't think of any good cases in which they should keep their codes
with the warning. If we always expect them to modify their codes,
raising an error seems better.

        • Is the problem being addressed significant enough to warrant a change to Swift?

Yes. I think it will not give so large impact on existing codes and
syntax. The benefits are relatively larger enough.

        • Does this proposal fit well with the feel and direction of Swift?

Yes. It will make our codes cleaner and safer. I think that is the
direction of Swift. I feel `@discardableResult` is natural.

        • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I think I have not used other languages with a similar feature.

        • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick reading. I also roughly searched the discussion for something
about warning/error and I found nothing.

-- Yuta

15

  • What is your evaluation of the proposal?

-1
This clearly makes me part of a small minority, but I doubt that the participants of this discussion are representative for all current and future users of Swift:
Those who enjoy tinkering with the design of languages naturally strive for a tool that can't be used in wrong ways — but that goal is just impossible, and we shouldn't sacrifice ease of use for a safety measure that as little value in reality.
Swift is rather young, so I don't think that analyzing existing code bases is a valid way to prove that superfluous "unused-result" warnings won't become an annoyance later.

There are situations where warn_unused_result is really useful, but the examples presented in this thread fail to illustrate why the behavior should be made default:
Sure, it is obviously strange when someone creates an object and doesn't do anything with it — so this should be a warning (or even an error), right?
I say no, because obvious errors are easy to find, and a compiler should focus on subtle ones.
Swift isn't a pure functional language, and despite their bad reputation here, the side effects might have been the motivation for a call — and its illusive that anyone knows in advance why a non-pure method is called.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes, but I disagree with the proposed change.
The default should be kept, but it should be more comfortable to activate the warning — or to deactivate it, if the default is changed:
This is all personal preference, so it should be up to the author of a method to decide how important its result is, and there shouldn't be strong pressure to either direction.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Took part in the discussion in an early stage, read most posts

Tino

16

• What is your evaluation of the proposal?
+1

        • Is the problem being addressed significant enough to warrant a
change to Swift?
Yes

        • Does this proposal fit well with the feel and direction of Swift?
Yes.
        • If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?
        • How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?
`=` vs `==` is a common and subtle case. I consider the all of the
comparison operators and our overloads thereof to be the most compelling
argument in favor of this change. It only occurred to me recently that I
should add `@warn_unused_result` on *every* custom implementation of `==`
and `<` This proposal remedies that problem.

···

On Wed, Mar 16, 2016 at 1:36 PM, Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Defaulting non-Void functions so they warn on unused
results” begins now and runs through March 21, 2016. The proposal is
available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md

Reviews are an important part of the Swift evolution process. All reviews
should be sent to the swift-evolution mailing list at:
        https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager. When replying, please try to keep the proposal link at the
top of the message:

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift. When writing your review, here are some questions you might want to
answer in your review:

        • What is your evaluation of the proposal?
        • Is the problem being addressed significant enough to warrant a
change to Swift?
        • Does this proposal fit well with the feel and direction of Swift?
        • If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?
        • How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris
Review Manager

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

17

   https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md

   • What is your evaluation of the proposal?

+1

   • Is the problem being addressed significant enough to warrant a change to Swift?

Yes, I believe so.

   • Does this proposal fit well with the feel and direction of Swift?

Yes, this should encourage better practices, without forcing any pattern.

   • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No.

   • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thorough reading of the proposal, quick reading of the discussion thread.

18

What is your evaluation of the proposal?

-0.5 if the annotation is verbose (much longer than @discardable).
+0.5 if the annotation is pleasant and concise, like @discardable

Is the problem being addressed significant enough to warrant a change to Swift?

The warn-by-default behavior is mostly useless. The only reason to make the change is because “if we were designing Swift from scratch, this would probably be a slightly better default”.

Most non-void functions are called for their result, and nobody ever forgets to use that result; if they do, it's like if they forgot to call the function altogether — trivial to find, not useful as a compiler diagnostic at all.

The new default is better for:

- (A) classes that provide both mutating and non-mutating methods;
- (B) methods where forgetting to use the result produces a bug (a download task that needs to be resumed, an alert that needs to be displayed, a listener that needs to be stored somewhere, etc).

The old default is better for:

- (C) fluid APIs and other similar DSL scenarios;
- (D) methods that are mainly invoked for their side effect and return a value “just in case”, like removing/adding elements, scheduling tasks, ...

I've just scanned the entire Swift codebase I wrote since that fateful WWDC'14 (~20 kLOC excluding comments and blanks). I only have a handful of classes/methods in each category above, and annotating them one way or another is a trivial matter. Some of them *are not* currently annotated with @warn_unused_result, which is a point in favor of this proposal.

Does this proposal fit well with the feel and direction of Swift?

I think the safe-by-default, explicit-opt-out behavior is a better default for Swift, although, like I've said, in practice it doesn't really matter much.

Perhaps it's most useful for newcomers; you can easily skip over @warn_unused_result when learning Swift, but you won't be able ignore @discardable.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Haven't ever seen this in a language.

Golang had a (not very informative) discussion on this topic at Redirecting to Google Groups

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Closer to an in-depth study; looked at my Swift codebase, and read through all the discussions.

On a side note, let me once again point to a dangerous trend in this mailing list: not looking at (or reporting on) how potential changes affect actual, specific, production code bases. We need a lot more of that in our reviews.

Thanks,

A.

19

• What is your evaluation of the proposal?

Great idea

• Is the problem being addressed significant enough to warrant a change to
Swift?

Yes I think so, currently the declaration line is very cluttered with lots
of noise up front that hides what the line actually does. This proposal
removes some of that noise.

• Does this proposal fit well with the feel and direction of Swift?

Yes, Swift is clutter free and tries to pick as the default the most common
case

• If you have used other languages or libraries with a similar feature, how
do you feel that this proposal compares to those?

I have used languages with the current Swift bevahiour of having to
explicitly initiate the warning and they like Swift tended to be a pain.
Either the declaration got cluttered or the annotation was omitted.

I have used products like lint and appreciated this feature.

• How much effort did you put into your review? A glance, a quick reading,
or an in-depth study?

···

--
-- Howard.

20

  • What is your evaluation of the proposal?

I am in favor of the semantic, but I don't like `@discardableResult`; it's long and unwieldy. I would prefer that we either use a shorter word than "discardable", attach the keyword to the return type as discussed in "Future Directions", or both.

1. The keyword will be used rarely. I don't mind if it's slightly hard to type.
2. When used, it should be as clear as possible, both in understanding what it does and visually standing out.
3. The most popular keyword requested was actually @allowUnusedResult followed by @suppressUnusedResultWarning.
   Both of these are longer. I felt @discardableResult was more descriptive than @allowUnusedResult. I wanted to avoid
   the word "suppress" as it is appears on many frequently misspelled words lists.

   I believe discardable (the number one choice by *far* for a type annotation, with ignorable as its second) perfectly describes
   how the return value/result should be treated. When included, the behavior mimics:

        let _ = resultReturningFunctionOfSomeType()
   
   which basically discards the result (an active decision) rather than ignores it (a passive one).

I also don't like that this proposal doesn't include an "Impact on existing code" section. We ought to decide whether the migrator will add `@discardableResult` to existing symbols or not.

This is a good point and Adrian and I will be happy to add this in. I do not believe it should, although I'll let Adrian answer
on his own. The entire point of this exercise is to reduce likely error points. Simply changing the behavior without fixits
should help accomplish that in existing code. If you add @discardableResult, we mask the advantage this behavior
should address.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes. I should use `@warn_unused_result`, but never bother because it's just too much of a hassle. My code will be safer with this change.

And that's why I don't think the migrator should make any code changes.

-- E

···

On Mar 16, 2016, at 5:27 PM, Brent Royal-Gordon via swift-evolution <swift-evolution@swift.org> wrote: