[swift-evolution] [Review] SE-0091: Improving operator requirements in protocols

[Tony Allevato]

On Mon, May 23, 2016 at 9:58 PM Jordan Rose <jordan_rose at apple.com> wrote:

> [Proposal:
> https://github.com/apple/swift-evolution/blob/master/proposals/0091-improving-operators-in-protocols.md
> ]
>
> Hi, Tony. Thanks for working on this. I have to say I’m incredibly
> concerned with this direction, for two main reasons. I’ll try to break them
> down here. (Sorry for squeaking in at the end of the review period!)
>

No worries, thanks for the detailed feedback! I've tried to address your
concerns inline.


*Overrides*
>
> People up until now have been fairly unhappy with how operators are
> statically dispatched and can’t be overridden. We went all the way towards
> providing == that automatically calls isEqual(_:) on NSObject, but then you
> provide == for your subclass and it never gets called…no, wait, it *does* get
> called when you do a simple test, but not from the actual code that has
> NSObject as a static type.
>
> This proposal stays in that space: the proposed “trampoline” operator will
> dispatch based on the *static* type of the objects, not the dynamic type.
> Why? Consider using == on an NSURL and an NSString, both statically typed
> as NSObject. Given the definition of the trampoline from the proposal
>
> func == <T: Equatable>(lhs: T, rhs: T) -> Bool {
>
>   return T.==(lhs, rhs)
> }
>
>
> T can’t possibly be anything but NSObject. (Neither NSURL nor NSString
> matches the types of both ‘lhs’ and ‘rhs’.) This isn’t a regression from
> the current model, as you say, but it does make the current model *even
> more surprising,* since normally you’d expect methods to be dynamically
> dispatched.
>

Setting Objective-C aside for a moment, is this example consistent with
Swift's design philosophies about type safety though? Swift doesn't even
let you compare types that seem reasonably compatible, like Float and
Double. Should we expect users to be able to compare completely distinct
types like NSURL and NSString without a little extra work?

If having `==` map to `NSObject.isEqual` is important for Objective-C
compatibility, then there's no reason the runtime can't provide the
following operator:

func ==(lhs: NSObject, rhs: NSObject) -> Bool {
  return lhs.isEqual(rhs)
}

which should be more specific than the generic one and always get chosen in
the context you desire.

The situation is a little better when one of the types being compared has
the other type as a base, because then implementing the operators as class
methods instead of static methods does the right thing (with the only
caveat being you have to cast the arguments down to the correct type).


> Here’s an alternate formation of the trampoline that’s a little better
> about this…
>
> func == <T: Equatable>(lhs: T, rhs: T) -> Bool {
>
>   return lhs.dynamicType.==(lhs, rhs)
> }
>
>
> …but I’m still not convinced. (People are especially likely to get this
> wrong without the trampolines being synthesized.)
>
> One more note: at one point Joe Groff was investigating the idea that
> conformances wouldn’t be inherited onto subclasses, which would mean no
> more implicit ‘required’ initializers. Instead, the compiler would perform
> most operations by upcasting to the base class, and then converting to the
> protocol type or calling the generic function. In this world, T would
> *always* be NSObject, never a subclass, and we’d have to come up with
> something else. I think this model is still worth investigating and I
> wouldn’t want to close off our options just for the sake of “cleaning
> house”.
>
> It’s possible that there’s not actually a reason to override operators in
> practice, which would make pretty much all of these concerns go away. (==
> is special; imagine we had an operation for checking equality within types
> and one across type hierarchies and ignore it for now.) I think it’d be
> worth investigating where operators are overridden today, and not just in
> Swift, to make sure we cover those use cases too.
>
> (Please forgive all of the Foundation type examples that may soon be value
> types. They’re convenient.)
>
>
> *Assignment Operators*
>
> A mutating requirement and a static method with an inout parameter mean
> different things for a conforming class: the former can only access the
> class’s properties, while the latter can replace the caller’s *reference* as
> well.
>
> class Player { … }
>
> extension Player {
>   static func roulette(_ player: inout Player) {
>     if randomValue() > 0.1 {
>       player.roundsSurvived += 1
>     } else {
>       // Replace this player…but not any other references to them!
>       player = Player()
>     }
>   }
>
>   /*mutating*/ func solitaire() {
>     self.roundsSurvived += 1
>     // Cannot replace ‘self’
>     //self = Player()
>   }
> }
>
>
> I’m not sure if one of these is obviously better than the other (more
> capable ↔︎ more footgun). I agree with Nicola's point about mutating
> methods *looking* better than static methods taking an inout parameter,
> but that probably shouldn’t be the ultimate deciding factor.
>

You make a good point here. This is hitting on one of the fundamental
problems I've had with mutating requirements in protocols in some of my own
projects, which is that even when a class type conforms to such a protocol
(or worse, even if the protocol itself is restricted to `class` types
only), if I want to use it in a generic context with that protocol as a
constraint, I still have to pass it unnecessarily as an inout argument.

On the one hand, fixing that underlying problem might help us here as well.
An obvious alternative would be to disallow `inout` on class conformances
to assignment operators, but that introduces inconsistency that I'm trying
to avoid. I'm not sure if it's any better, either.



> I know we want to improve type-checker performance, and reducing the
> number of overloads *seems* like a way to do that, but I’m not convinced
> it actually will in a significant way (e.g. “you can now use ten operators
> in a chain instead of seven” is not enough of a win). It still seems like
> there ought to be a lot of low-hanging fruit in that area that we could
> easily clear away, like “an overload containing a struct type will never
> match any input but that struct type”.
>
> I personally really want to move operators into types, but I want to do it
> by doing member lookup on the type, and fall back to global operators only
> if something can’t be found there. That approach
>
> - also has potential to improve type-checker performance
> - also removes operators from the global namespace
> - also removes the need for “override points” (implementation methods like
> NSObject.isEqual(_:) and FloatingPoint.isLessThan(_:))
>
> It does privilege the left-hand side of a binary operator, but I think
> that’s acceptable for the operators we’ve seen in practice. (Of course we
> would need real data to back that up.)
>

As Brent pointed out in his reply, without multiple dispatch, you don't
really benefit from privileging the lhs argument, and in fact you can end
up in situations where the behavior is surprising if you don't implement
both orders. For example, in your (NSString, NSURL) example, each class
would have to be extended to explicitly support comparison with the other
in order to support commutativity if equality was an instance method. I'm
not sure that's any better for those particular types than just having the
operators global in the first place.

My argument would be that there are still significant enough benefits to
move forward:

- for value types and for binary operators that have the same typed
arguments, static methods provide a clear benefit w.r.t. consistency (the
declaration of the global operator and the static operator look the same)
and obvious semantics
- for binary operators with differently typed arguments, static operators
support commutativity much better than instance methods because both
implementations live in the most natural type for that particular case (for
example, String + Character and Character + String)
- if later on we can auto-generate trampolines, the choice of instance vs.
static methods for value types becomes a wash because the compiler is
effectively doing the same dispatch under the hood anyway
- for class types, regardless of whether one is a base of the other or both
share a common third base type, neither static nor instance methods
completely solve the problem and won't until/unless Swift supports multiple
dispatch, and the proposed behavior is not a regression in those cases



>
> I think that about sums up my concerns and my interest in an alternate
> proposal. Again, I’m sorry for coming to this so late and for skimming the
> latest discussion on it; I’m sure “my” proposal has already come up, and I
> know it has its own flaws. I think I’m just not convinced that this is
> sufficiently *better* to be worth the churn and closing off of other
> potential avenues.
>
> Best,
> Jordan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160524/d6be5ea0/attachment.html>