Warn about unused Optional.some(())

Evolution Discussion
1

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan

2

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

IMHO, using ‘try?’ to ignore an error result, instead of just turning it into an optional, is an anti-pattern, and forcing users to write ‘_ = try? foo()’ might not be so bad…

···

On Jan 30, 2017, at 2:58 PM, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

What do y’all think?

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

3

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
  func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

···

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

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

4

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

Imho Swift already uses warnings excessively, and giving Optional<Void> more significance than Void feels strange.

Shortly after the new error-handling was added, I felt irritated when I first used a trowing void-function and got a warning ("hu? That function has no result to ignore — why should I declare a variable to store void?").

I'm fine with the current situation and wouldn't undo it.

5

Not sure if this requires a proposal. The code change is pretty trivial: Warn unused ()? by dduan · Pull Request #7154 · apple/swift · GitHub .

···

On Jan 30, 2017, at 3:25 PM, Slava Pestov <spestov@apple.com> wrote:

On Jan 30, 2017, at 2:58 PM, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

IMHO, using ‘try?’ to ignore an error result, instead of just turning it into an optional, is an anti-pattern, and forcing users to write ‘_ = try? foo()’ might not be so bad…

What do y’all think?

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

6

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

IMHO, using ‘try?’ to ignore an error result, instead of just turning it into an optional, is an anti-pattern, and forcing users to write ‘_ = try? foo()’ might not be so bad…

+1

···

Sent from my iPad

On Jan 30, 2017, at 5:25 PM, Slava Pestov via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 30, 2017, at 2:58 PM, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

What do y’all think?

Daniel Duan
_______________________________________________
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

7

Good to know the history. If I were to fix the inconsistency, I'd add the warning to optional chaining instead.

Deliberately make the compiler give us *less* information for esthetic reasons feels wrong to me. As I mentioned in the original email, this has cost us a few unnoticed bad patterns slipping into our production. That's the opposite of what this type of warning is supposed to achieve.

···

On Jan 31, 2017, at 1:07 AM, Alex Hoppen <alex@ateamer.de> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

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

8

Makes sense now. I remove my point about removing that fix. Optional chaining is much more useful to have behaving as expected.

···

On 31 Jan 2017, at 10:07, Alex Hoppen via swift-evolution <swift-evolution@swift.org> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan
_______________________________________________
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

9

Thanks for clarifying, but this is an interesting case actually; it seems the problem here is the ternary, as presumably the following would have worked just fine:

if true { foo?.bar() }
else { foo?.bar() }

So your example's problem seems to stem then from the fact that the ternary's type isn't inheriting the discardable nature of the two branches. I wonder then if an alternative solution might to have discardable be an inheritable property, while keeping optional chaining implicitly discardable?

For example:

@discardableResult func foo() -> Int { return 1 }
func bar() -> Int { return 2 }
struct Baz { func baz() {}}

let a:Baz? = Baz()
true ? foo() : bar() // type is Int, should produce a warning
true ? foo() : foo() // type is discardable Int, no warning necessary
true ? a?.baz() : a?.baz() // type is discardable Void, no warning necessary

The idea basically being that @discardableResult becomes a property of return types that is passed for as long as it is in common, but does not prevent two types (one discardable, one not) from being equal.

This might give a best of both? As method chaining producing implicitly discardable results would allow your example to behave as expected, but other cases can still be configured as desired with @discardableResult (or not).

···

On 31 Jan 2017, at 09:07, Alex Hoppen via swift-evolution <swift-evolution@swift.org> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

10

Amendment to the history of the bug after I had a look at the bug reports again: SR-1895 <Issues · apple/swift-issues · GitHub; explicitly asked that

let s: String? = "hi"
s.map {print($0)}

should not produce any warnings while it did so during beta 1.

– Alex

···

On 31 Jan 2017, at 09:07, Alex Hoppen via swift-evolution <swift-evolution@swift.org> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan
_______________________________________________
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

11

I think I see Alex's point here. Optional chaining is still intended to be a substitute for Objective-C's nil-swallowing, and therefore foo?.bar() should not warn if 'bar' has a discardable result, even though there is semantic information about whether the method was actually called. I think that of the three things under consideration here:

1. foo?.bar() should not warn
2. foo.map(baz) should warn
3. Ternaries should be consistent with non-ternaries

#1 is the most important, at least to me. The Swift 3 change was to sacrifice #2 in favor of #3, which I'm not sure I would have done, but I wouldn't want to sacrifice #1 in favor of #2.

I wouldn't mind the model of the type being '@discardableResult Optional<Void>' or whatever, but I think that's probably more work than anyone wants to sign up for.

Jordan

···

On Jan 31, 2017, at 08:16, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Good to know the history. If I were to fix the inconsistency, I'd add the warning to optional chaining instead.

Deliberately make the compiler give us *less* information for esthetic reasons feels wrong to me. As I mentioned in the original email, this has cost us a few unnoticed bad patterns slipping into our production. That's the opposite of what this type of warning is supposed to achieve.

On Jan 31, 2017, at 1:07 AM, Alex Hoppen <alex@ateamer.de> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan
_______________________________________________
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

12

Sent from my iPad

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

IMHO, using ‘try?’ to ignore an error result, instead of just turning it into an optional, is an anti-pattern, and forcing users to write ‘_ = try? foo()’ might not be so bad…

+1

Isn't this how it was in Swift 2.x and the first versions of 3.0? I believe this was changed only recently - which I personally found as good news. In some cases you simply do not care about the error result since it has no impact if the call fails and typing "_ =" seemed like boilerplate...

If I recall correctly, this was discussed here on the list and changed to the current behavior.

···

On Jan 31, 2017, at 1:03 AM, Matthew Johnson via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 30, 2017, at 5:25 PM, Slava Pestov via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 30, 2017, at 2:58 PM, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

What do y’all think?

Daniel Duan
_______________________________________________
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

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

13

Amendment to the history of the bug after I had a look at the bug reports again: SR-1895 explicitly asked that

let s: String? = "hi"
s.map {print($0)}

This is the anti-pattern we try to discourage. FYI.

···

On Jan 31, 2017, at 8:47 AM, Alex Hoppen <alex@ateamer.de> wrote:

should not produce any warnings while it did so during beta 1.

– Alex

On 31 Jan 2017, at 09:07, Alex Hoppen via swift-evolution <swift-evolution@swift.org> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan
_______________________________________________
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

14

This inconsistency could be solved by giving optionals a method to specifically perform side effects.
In the same way that arrays have a 'map' function for retuning new arrays and a 'forEach' to return '()' and perform a side effects. Option could have a 'forEach' method that returns '()'.

s.map(print) // would trigger a "unused result warning"

s.forEach(print) // would not trigger any warning

The naming is definitely debatable, but I think the idea is worth considering.

Andre Videla

···

On 31 Jan 2017, at 17:47, Alex Hoppen via swift-evolution <swift-evolution@swift.org> wrote:

Amendment to the history of the bug after I had a look at the bug reports again: SR-1895 explicitly asked that

let s: String? = "hi"
s.map {print($0)}

should not produce any warnings while it did so during beta 1.

– Alex

On 31 Jan 2017, at 09:07, Alex Hoppen via swift-evolution <swift-evolution@swift.org> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan
_______________________________________________
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

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

15

Apologies for re-posting this, but I got some message undelivered e-mails for it so I'm not 100% sure if it went through the first time or not:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

Thanks for clarifying, but this is an interesting case actually; it seems the problem here is the ternary, as presumably the following would have worked just fine:

if true { foo?.bar() }
else { foo?.bar() }

So your example's problem seems to stem then from the fact that the ternary's type isn't inheriting the discardable nature of the two branches. I wonder then if an alternative solution might to have discardable be an inheritable property, while keeping optional chaining implicitly discardable?

For example:

@discardableResult func foo() -> Int { return 1 }
func bar() -> Int { return 2 }
struct Baz { func baz() {}}

let a:Baz? = Baz()
true ? foo() : bar() // type is Int, should produce a warning
true ? foo() : foo() // type is discardable Int, no warning necessary
true ? a?.baz() : a?.baz() // type is discardable Void, no warning necessary
The idea basically being that @discardableResult becomes a property of return types that is passed for as long as it is in common, but does not prevent two types (one discardable, one not) from being equal.

This might give a best of both? As method chaining producing implicitly discardable results would allow your example to behave as expected, but other cases can still be configured as desired with @discardableResult (or not).

···

On 31 Jan 2017, at 09:07, Alex Hoppen via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

16

I think I see Alex's point here. Optional chaining is still intended to be a substitute for Objective-C's nil-swallowing, and therefore foo?.bar() should not warn if 'bar' has a discardable result, even though there is semantic information about whether the method was actually called. I think that of the three things under consideration here:

1. foo?.bar() should not warn
2. foo.map(baz) should warn
3. Ternaries should be consistent with non-ternaries

I 100% agree with this analysis.

#1 is the most important, at least to me. The Swift 3 change was to sacrifice #2 in favor of #3, which I'm not sure I would have done, but I wouldn't want to sacrifice #1 in favor of #2.

I wouldn't mind the model of the type being '@discardableResult Optional<Void>' or whatever, but I think that's probably more work than anyone wants to sign up for.

I’ll give this a go and report back. *crosses fingers*

···

On Feb 6, 2017, at 10:58 AM, Jordan Rose <jordan_rose@apple.com> wrote:

Jordan

On Jan 31, 2017, at 08:16, Daniel Duan via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Good to know the history. If I were to fix the inconsistency, I'd add the warning to optional chaining instead.

Deliberately make the compiler give us *less* information for esthetic reasons feels wrong to me. As I mentioned in the original email, this has cost us a few unnoticed bad patterns slipping into our production. That's the opposite of what this type of warning is supposed to achieve.

On Jan 31, 2017, at 1:07 AM, Alex Hoppen <alex@ateamer.de <mailto:alex@ateamer.de>> wrote:

This was a deliberate change between Swift 3 beta 1 and beta 2 after a friend of mine pointed the following inconsistency out to me:

struct Foo {
func bar() {}
}
let foo: Foo? = Foo()
foo?.bar() // Does not create a warning
true ? foo?.bar() : foo?.bar() // expression of type '()?' is unused

After some offline discussion at WWDC with the Swift team we decided to move to a consistent model where ()?, ()??, … is always discardable since we didn't want to take the convenience of foo?.bar() away (something that regularly occurs with weak variables, e.g. captures in closures).

So much for the history of this feature.

– Alex

On 30 Jan 2017, at 22:58, Daniel Duan via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

What do y’all think?

Daniel Duan
_______________________________________________
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

17

I guess I missed that discussion. This "feature" does more harm than good IMHO.

···

On Jan 30, 2017, at 10:16 PM, Charlie Monroe via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 31, 2017, at 1:03 AM, Matthew Johnson via swift-evolution <swift-evolution@swift.org> wrote:

Sent from my iPad

On Jan 30, 2017, at 5:25 PM, Slava Pestov via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 30, 2017, at 2:58 PM, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

IMHO, using ‘try?’ to ignore an error result, instead of just turning it into an optional, is an anti-pattern, and forcing users to write ‘_ = try? foo()’ might not be so bad…

+1

Isn't this how it was in Swift 2.x and the first versions of 3.0? I believe this was changed only recently - which I personally found as good news. In some cases you simply do not care about the error result since it has no impact if the call fails and typing "_ =" seemed like boilerplate...

If I recall correctly, this was discussed here on the list and changed to the current behavior.

What do y’all think?

Daniel Duan
_______________________________________________
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

_______________________________________________
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

18

I guess I missed that discussion. This "feature" does more harm than good IMHO.

Indeed. I find this behavior very surprising and goes against Swift's safe-by-default and explicitness philosophy.

I'd argue removing it.

···

On 31 Jan 2017, at 07:23, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 30, 2017, at 10:16 PM, Charlie Monroe via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 31, 2017, at 1:03 AM, Matthew Johnson via swift-evolution <swift-evolution@swift.org> wrote:

Sent from my iPad

On Jan 30, 2017, at 5:25 PM, Slava Pestov via swift-evolution <swift-evolution@swift.org> wrote:

On Jan 30, 2017, at 2:58 PM, Daniel Duan via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

Right now, expressions that evaluates to Optional<()>, Optional<Optional<()>>… gets special treatment when it’s unused. For example:

func f(s: String) {}
let s: String = “”
s.map(f) // no warning here, even tho the resulting type is `Optional<()>` and unused.

func g() throws {}
try? g() // no warnings here neither.

This is convenient, but encourages composing map/filter/reduce, etc with side-effect-ful functions, which we have found a few cases of in our production code recently. Granted, these cases could’ve been caught with more careful code reviews. But we wouldn’t have missed them if this “feature” didn’t exist.

I think we should remove the special treatment so that code in the example above would generate a warning about `()?` being unused. Users can silence it manually by assigning the result to `_`.

OTOH, this would undermine the convenience of `try?` when the throwing function don’t return anything.

IMHO, using ‘try?’ to ignore an error result, instead of just turning it into an optional, is an anti-pattern, and forcing users to write ‘_ = try? foo()’ might not be so bad…

+1

Isn't this how it was in Swift 2.x and the first versions of 3.0? I believe this was changed only recently - which I personally found as good news. In some cases you simply do not care about the error result since it has no impact if the call fails and typing "_ =" seemed like boilerplate...

If I recall correctly, this was discussed here on the list and changed to the current behavior.

What do y’all think?

Daniel Duan
_______________________________________________
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

_______________________________________________
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

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

19

I suspect this will entail making a new sugared type kind and then threading it carefully through the constraint solver (hence why I said it's probably more work than you want to take on).

Jordan

···

On Feb 6, 2017, at 11:08, Daniel Duan <daniel@duan.org> wrote:

On Feb 6, 2017, at 10:58 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

I think I see Alex's point here. Optional chaining is still intended to be a substitute for Objective-C's nil-swallowing, and therefore foo?.bar() should not warn if 'bar' has a discardable result, even though there is semantic information about whether the method was actually called. I think that of the three things under consideration here:

1. foo?.bar() should not warn
2. foo.map(baz) should warn
3. Ternaries should be consistent with non-ternaries

I 100% agree with this analysis.

#1 is the most important, at least to me. The Swift 3 change was to sacrifice #2 in favor of #3, which I'm not sure I would have done, but I wouldn't want to sacrifice #1 in favor of #2.

I wouldn't mind the model of the type being '@discardableResult Optional<Void>' or whatever, but I think that's probably more work than anyone wants to sign up for.

I’ll give this a go and report back. *crosses fingers*

20

I think I see Alex's point here. Optional chaining is still intended to be a substitute for Objective-C's nil-swallowing, and therefore foo?.bar() should not warn if 'bar' has a discardable result, even though there is semantic information about whether the method was actually called. I think that of the three things under consideration here:

1. foo?.bar() should not warn
2. foo.map(baz) should warn
3. Ternaries should be consistent with non-ternaries

I 100% agree with this analysis.

#1 is the most important, at least to me. The Swift 3 change was to sacrifice #2 in favor of #3, which I'm not sure I would have done, but I wouldn't want to sacrifice #1 in favor of #2.

I wouldn't mind the model of the type being '@discardableResult Optional<Void>' or whatever, but I think that's probably more work than anyone wants to sign up for.

I’ll give this a go and report back. *crosses fingers*

I suspect this will entail making a new sugared type kind and then threading it carefully through the constraint solver (hence why I said it's probably more work than you want to take on).

I was going to write a enhanced version of TypeBase::lookThroughAllAnyOptionalTypes(). Too hacky?

···

On Feb 6, 2017, at 11:35 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On Feb 6, 2017, at 11:08, Daniel Duan <daniel@duan.org <mailto:daniel@duan.org>> wrote:

On Feb 6, 2017, at 10:58 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

Jordan