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

Evolution Announcements
41

Addressing a few common themes:

Imported code from other languages should not be warn unused. That is not the assumption the code was developed under, and Swift should not be held to that contract.
Imported code from Cocoa should not be warn unused because a full audit is impractical.

It seems to me that if you create a UIView(frame: rect) for example and discard the result, that is typically more error-y than intentional. On the other hand, many Boolean results and oserr results are reasonably ignorable. As for the popViewController methods on UINavigationController, whose results are typically ignored, I'm wavering between "yes, that should raise a warning" and "yes, that method should be audited and marked as disposable".

This is one of those areas where I would prefer advice and feedback from the Core Team before modifying the proposal.

This will confuse people who do not read release notes or the documentation, and they will use the less advantageous "let _ =" instead.
"This doesn't mesh with my personal coding style"
"I'm already a careful developer and my code would not benefit"
All my code has been thoroughly pre-audited and the transition will be painful.

If adopted, this proposal will incur negative costs for some developers. I believe the benefits for the language and the wider community outweigh the negatives.

This isn't fluent interface-friendly

This one confuses me as I was under the impression that method chaining consumes each result in turn, so won't cause warnings.

There isn't an "Impact on existing code" section.

Adrian and I need to add that. At a minimum:

- The migrator should remove existing @warn_unused_results in Swift code
- The section should state how code imported from other languages (particularly C-based and Cocoa) will be treated. I want advice from the Core Team on how they think this will be best addressed before I add that to the proposal.

42

I assure you I was just trying to summarize situations people could be in, not calling anyone out.

-- E

···

On Mar 17, 2016, at 10:52 AM, Brent Royal-Gordon <brent@architechies.com> wrote:

  • All my code has been thoroughly pre-audited and the transition will be painful.

If adopted, this proposal will incur negative costs for some developers. I believe the benefits for the language and the wider community outweigh the negatives.

I think this was in response to a statement I made, so I'd like to make two clarifications:

43

I believe the scoped @discardableResult(warn|critical) attribute is a nice idea for future directions, but I'm not sure it's in scope of this proposal, which, as a first small step, aims to basically invert the @warn_unused_result standard.

cc Erica

Pozdrawiam – Regards,
Adrian Kashivskyy

···

Wiadomość napisana przez Haravikk <swift-evolution@haravikk.me> w dniu 21.03.2016, o godz. 22:28:

On 21 Mar 2016, at 20:35, Erica Sadun via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
In Swift Evolution discussions, the term @discardable was mildly preferred over @discardableResult.

Some community members requested a new attribute enabling exceptional imported functions to be properly annotated from Objective-C source.

Dany St-Amant requested two levels of compiler response: @discardableResult(warn) for simple warnings and @discardableResult(critical) which generates an error with unused results, such as when returning allocated resources

This looks good, I think that @discardableResult() makes sense until we can get @discardable on the return type itself. I think that if warn/critical is added then there should also be a corresponding “allow” or similar (which is what the default is), i.e- @discardableResult on its own implies @discardResult(allow) to issue no warnings of any kind.

I also like the idea of being able to add this attribute to types as well as functions, allowing the default behaviour to be changed on a per-type basis, as this would be useful for types that are specifically designed with method chaining for example (where most results are discardable as standard). While the choice of default will never satisfy everyone, this would make it easy to tweak for your own needs.

Allow us to do something like the following, assuming that @discardableResult(warn) is now the default:

  @discardableResult(allow)
  class MyChainableType {
    func chainableMethod() -> Self { … }
    func anotherChainableMethod() -> Self { … }

    @discardableResult(critical)
    func getResult() -> Dictionary { … }
  }

Of course, if we can get attributes on return types now or in future, then the function form of the attribute should be @discardable(warn) for .getResult().

44

For the addition of critical sure, but the ability for developers to switch between warn-by-default and ignore-by-default (current) behaviour would ease migration, and address concerns from those that prefer the current behaviour; i.e- for types designed with method chaining in mind the current default is preferable to the new one, and the new default will actually cause those types to become susceptible to the same mistakes that warn-by-default hopes to avoid.

For this reason I think that having an attribute at both function and type levels is important, as it lets developers choose for themselves what the default for their type is (or file, or extension, whatever scope makes most sense).

···

On 21 Mar 2016, at 23:21, Adrian Kashivskyy <adrian.kashivskyy@me.com> wrote:

I believe the scoped @discardableResult(warn|critical) attribute is a nice idea for future directions, but I'm not sure it's in scope of this proposal, which, as a first small step, aims to basically invert the @warn_unused_result standard.

cc Erica