[Review] SE-0121: Remove `Optional` Comparison Operators


(Chris Lattner) #1

Hello Swift community,

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.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.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 Lattner
Review Manager


(Sean Heber) #2

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.md

  * What is your evaluation of the proposal?

I think this probably makes sense. However IMO it is critical that this not be expanded to also include ==/!= as I rely on that working when testing against optionals all of the time - but thankfully that is not included in this proposal.

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

I’ve never knowingly encountered this problem, so it’s hard to say without doing some more careful auditing.

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

I believe so - it encourages “safe by default” logic and in this case I can totally see how the current design could yield surprising results.

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

I think the closest scenario I’m familiar with is Objective-C where if you call -compare: on a nil var, you’ll get back a 0 which is NSOrderedSame when you totally didn’t expect that! This proposal should avoid that sort of thing from flying under the radar in Swift.

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

Not a lot, tbh.

l8r
Sean


(Adrian Zubarev) #3

What is your evaluation of the proposal?

I probably never came across these Optional comparison operators in any Swift code I’ve been working with. The removal of these doesn’t hurt any of my codebase, so I’d appreciate less noise inside sdtlib.
Is the problem being addressed significant enough to warrant a change to Swift?

I’d say it is. As the proposal clearly stated, reintroducing these operators later would be purely additive.
Does this proposal fit well with the feel and direction of Swift?

I feel like it does, because right now these operators are more likely useless.
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

None that I can remember of.
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Carefully read the proposal on GitHub.

···

--
Adrian Zubarev
Sent with Airmail

Am 12. Juli 2016 um 20:26:34, Chris Lattner via swift-evolution (swift-evolution@swift.org) schrieb:

Hello Swift community,

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.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.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 Lattner
Review Manager

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


(Ash Furrow) #4

* What is your evaluation of the proposal?

+1 from me. I particularly like how this adheres to the principle of least surprise. Keeping == and != is an important part of the proposal, as others have said.

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

I believe so.

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

Aye, pretty happy about its direction.

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

I have: Objective-C! Converting the code from the proposal, we have:

NSArray *ps = [peeps filter:^BOOL(Person *lhs, Person *rhs) {
return [[lhs pet] age] < [[rhs pet] age];
}];

I think that convention works in Objective-C, nil being capable of receiving messages is a cornerstone of the language. But Swift has discouraged the convention of calling functions directly on nil, through Optionals. I believe that removing the comparison operators for Optionals adheres to the same ideas that Optionals themselves are built from.

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

I read the review thoroughly, asked on Twitter, let it sit in the back of my head for an hour. I also had to spend five minutes figuring out the Objective-C block syntax for a filter function.

···

--
Ash Furrow
https://ashfurrow.com/

On July 12, 2016 at 2:26:55 PM, Chris Lattner via swift-evolution (swift-evolution@swift.org) wrote:

Hello Swift community,

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.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.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 Lattner
Review Manager

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


(Taras Zakharko) #5

  * What is your evaluation of the proposal?

+1. Fully support this change. ‚Optional' comparison operators hide the fact that the value is boxes which can lead to confusing behavior and difficult to find bugs. Personally, I always unbox my optionals before doing any kind of comparisons.

In fact, I would go a step further and remove !=<T, T?> operator as well. It can’t distinguish between a situation when inequality holds because an optional is nil or inequality holds because optional contains a different value, which can also be surprising. IMO, the only non-surprising inequality comparison is to a nil value. But I also have to admit that I don’t have a practical example where this becomes an issue.

  * 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?

Yes and yes.

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

A glance.

—T

···

On 12 Jul 2016, at 20:26, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.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.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 Lattner
Review Manager

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


(Haravikk) #6

  * What is your evaluation of the proposal?

Strongly in favour.

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

Yes, though it seems like a small change it removes an ambiguity that can be easy to trip up on. Indeed it's entirely possible there are people using optional comparison without realising it, so why there may some minor breakage, I think it's worth causing.

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

Yes, now that we have the ?? operator there is no need for these comparisons as non-optional comparisons can be used instead with explicitly defined default values, eliminating any possible confusion.

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

Quick read through, the change is pretty straightforward though.

···

On 12 Jul 2016, at 19:26, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:


(Karl) #7

+1 from me.

I recently had to fix a bug stemming from this which was easy to fix, but not easy to spot.

These operators are quite special; they are plain yes/no assertions used to filter data at a particularly broad level of abstraction. It is usually super-important that you understand exactly what they do. They can’t afford to have hidden edge-cases with potentially surprising results.

Karl

···

On 12 Jul 2016, at 20:26, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.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.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to 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 Lattner
Review Manager

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


(Brent Royal-Gordon) #8

  * What is your evaluation of the proposal?

Another quick hit. The proposal says this:

Until generics are more mature, the issue of Optional being conditionally Comparable can't be fully discussed/implemented, so it makes the most sense to remove these questionably-useful operators for now (a breaking change for Swift 3), and add them back in the future if desired.

But I'm not sure I agree with this logic. The feature is already there, and if we keep it, the current version is source-compatible with a conditional-conformance-based version. I'm not convinced the right answer is to throw it away now and potentially reintroduce a completely compatible version later.

If we're *not* going to introduce a Comparable conformance for Optional, of course, by all means get rid of this today. But I'm not convinced this is the sort of thing we should be afraid to get rid of in the future, and thus, I'm not sure it's the kind of thing we should be afraid to keep for now until we make a final decision.

···

--
Brent Royal-Gordon
Architechies


(Mike Sanderson) #9

        * What is your evaluation of the proposal?

+1. Agree the comparison can be surprising, and it also confuses some
people trying to understand optionals, apparently; I once couldn't convince
someone how it worked until shortly afterwards for another issue @jckarter
phrased it "Optional defines an ordering for itself with nil < everything."
(https://twitter.com/jckarter/status/657266671703359488 ) That Optional
defines an ordering for itself also emphasizes that nil < everything is
arbitrary.

I think the question of the proposal is exactly "what remains is to decide
whether these semantics (that nil is "less than" any non-nil value) are
actually useful and worth keeping."

The Pet example shows how an asymmetry is caused: If someone wanted to
filter people who don't have pets under 6 (so nil would be grouped with the
higher numbers), the comparison operator is not useful without further
logic-- but not when checking for people who don't have pets over 6 (nil
grouped with the lower numbers).

In an another example, it's possible when ordering a list, all nil examples
should be considered greater than, so that for example in an alphabetized
list nil values should appear at the end of the list.

It's intuitive that nil is less than everything, but in plenty of scenarios
that isn't useful.

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

Yes. Often where these are currently used, as in the proposal examples, the
results can be surprising and not useful.

-MikeSand


(Karoy Lorentey) #10

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.md

  * What is your evaluation of the proposal?

If SE-0123 is accepted, then I don’t mind keeping optional comparisons. But I also wouldn’t protest much against removing them until Optional can be made to conditionally conform to Comparable.

If SE-0123 is rejected, then definitely +1; I’ve been bitten several times by accidentally using these while handling the return value of Collection’s index(of:).

I don’t remember ever intentionally using these overloads, except ironically:

https://twitter.com/lorentey/status/657254631660236800

Note though that I often find myself wishing for Optional to implement Comparable. Writing comparison methods for little one-off Comparable structs that wrap Optionals gets tiring after a while. (As the proposal states, the existing overloads fall far short of achieving conditional conformance. I guess I could use these operators in the implementation of my wrapper structs, but I forget they exist.)

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

This depends on the outcome of SE-0123, which aims to eliminate the pitfall that makes these overloads dangerous.

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

I think Optional should implement Comparable whenever the wrapped type does. The proposal is a distinct step back from this. On the other hand, if SE-0123 fails, I think it’s worth giving up on this goal in favor of removing a common pitfall.

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

Languages with implicit optionals do allow comparisons. E.g., "NSNotFound < 42" produces no compiler diagnostic.

C++ has recently gained std::optional, which does provide </<=/>/>= operators, with the same semantics as Swift, including support for comparing optionals with non-optionals. It also has implicit promotion of values to optionals.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3672.html#rationale.relops

Rust’s std::Option<T> enum implements the std::cmp::Ord trait when T does. So Rust’s optionals are (conditionally) comparable. I believe Rust provides no implicit coercion from T to std::Option<T>, but to be honest I’m not entirely sure.
https://doc.rust-lang.org/std/option/enum.Option.html#method.cmp

So the few languages I know that support explicit optionals also make them comparable.

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

Quick reading and minimal research.

···

On 2016-07-12, at 20:26, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

--
Karoly
@lorentey


(Joseph Lord) #11

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.md

  * What is your evaluation of the proposal?

I approve. I actually filed a (rdar://19366632) in Jan 2015 about this behaviour.

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

Yes. It can catch out the unwary in several ways and is objectively surprising behaviour.

  * 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?

Well Swift is the main language with explicit optionals that I have used but comparisions with nil/NULL vary in behaviour between languages. In SQL for example NULL is taken as unknown and even an equality comparision between NULL values fails. It could be worth going as far as removing the equality between optionals for this reason (although we could still keep the NilLiteralConvertible (or Swift 3 named version) for equality checks (I would also be fine with reusing the `is` keyword but haven't thought about that deeply. i.e. `if foo is nil {`

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

Read proposal and review discussion so far but have also considered it in the past and have warned about the behaviour in talks in the past.

Joseph

···

On 12/07/2016 19:26, Chris Lattner via swift-evolution wrote:

--
Human Friendly Ltd.


(TJ Usiyan) #12

+1

Conditional conformance cannot arrive soon enough.

···

On Tue, Jul 12, 2016 at 9:11 PM, Ash Furrow via swift-evolution < swift-evolution@swift.org> wrote:

* What is your evaluation of the proposal?

+1 from me. I particularly like how this adheres to the principle of least
surprise. Keeping == and != is an important part of the proposal, as others
have said.

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

I believe so.

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

Aye, pretty happy about its direction.

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

I have: Objective-C! Converting the code from the proposal, we have:

NSArray *ps = [peeps filter:^BOOL(Person *lhs, Person *rhs) {
  return [[lhs pet] age] < [[rhs pet] age];
}];

I think that convention works in Objective-C, nil being capable of
receiving messages is a cornerstone of the language. But Swift has
discouraged the convention of calling functions directly on nil, through
Optionals. I believe that removing the comparison operators for Optionals
adheres to the same ideas that Optionals themselves are built from.

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

I read the review thoroughly, asked on Twitter, let it sit in the back of
my head for an hour. I also had to spend five minutes figuring out the Objective-C
block syntax for a filter function.

--
Ash Furrow
https://ashfurrow.com/

On July 12, 2016 at 2:26:55 PM, Chris Lattner via swift-evolution ( > swift-evolution@swift.org) wrote:

Hello Swift community,

The review of "SE-0121: Remove `Optional` Comparison Operators" begins now
and runs through July 19. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0121-remove-optional-comparison-operators.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.

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and contribute to 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 Lattner
Review Manager

_______________________________________________
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


(Fabian Ehrentraud) #13

* What is your evaluation of the proposal?

Very welcome change that should reduce unexpected behavior in some cases.

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

Yes, as it could avoid programming mistakes.

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

Definitely, the more obvious the code reads the better.

···

* 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?

I experienced Swifts behavior in this case since Swift 1.0, and it always felt wrong.