[Review] SE-0185 - Synthesizing Equatable and Hashable conformance

Hello Swift community,

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance" begins now and runs through August 15, 2017. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.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 Lattner
Review Manager

Hello Swift community,

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance" begins now and runs through August 15, 2017. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.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?

+1, it addresses one of the biggest pain points that cause people to reach for metaprogramming facilities.

I have one clarifying point and tweak. The proposal states:

A struct T: Hashable that satisfies the conditions above will receive a synthesized implementation of var hashValue: Int { get } that uses an unspecified hash function† to compute the hash value by incorporating the hash values of the fields as its terms, in definition order.

This means that if the hash-combine operation is not commutative, then hashValues are not stable modulo member reordering. Was this scenario explicitly thought through? I don’t think it’s likely to be an issue in practice, but it's an artifact of implicit and ordered member-wise synthesis. It’s not obvious that declaration order is clearly the best order, e.g. declaration order is not necessarily memory-layout order. It might be better to leave the precise order unspecified, so long as it's guaranteed consistent during the execution of a program.

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

It's a reasonably scoped chunk of functionality for the biggest pain points, so yes.

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

Haskell and Rust both have “deriving" constructs for generating these kinds of things, and that simplifies a lot of code.

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

Thought about it a bit; read it quickly.

···

On Aug 9, 2017, at 4:09 PM, Chris Lattner <clattner@nondot.org> wrote:

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-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

Hello Swift community,

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance"
begins now and runs through August 15, 2017. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/
proposals/0185-synthesize-equatable-hashable.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?

Brilliant. However, I believe that this is a typo of some importance to
clarify:

The compiler synthesizes P's requirements for a struct with one or more

stored properties if and only if all of the types of all of its stored
properties conform to P.

I think the author means to write "...conform to {Equatable | Hashable}";
it is unclear what it means for a struct P to have stored properties that
"conform to P" (which would be an odd restriction in any case). The same
issue occurs in preceding bulleted list with respect to enums.

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

This is a wonderful addition.

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

Yes, the author did an excellent job paralleling the existing rules for
Codable synthesis; I agree fully with that approach as it makes this
feature more easily learnable and predictable for all users.

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

I haven't in recent times, but I do know that other languages do this.

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

A quick reading today; an in-depth study previously.

···

On Wed, Aug 9, 2017 at 6:08 PM, Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

  • What is your evaluation of the proposal?

Generally positive, but I don't feel that conforming to Equatable/Hashable is sufficient as an opt-in. Consider for example someone who declares their type as Equatable/Hashable, then sets to work on the details of their type, forgetting to implement the actual equatable/hashable behaviour they want.

I would suggest either that there be a keyword/attribute to indicate when the synthesised solution be used, or that there be alternate protocols such as AutoEquatable and AutoHashable; the idea being that the more explicit declaration results in the synthesised behaviour, or produces errors if it isn't possible or if a custom implementation is provided. An advantage of these methods is that the synthesis could potentially then be allowed for extensions (unless there's some other reason they aren't?), since they're absolutely explicit in what they want.

I'm just wary of these being implemented automatically cases where someone forgets to do it themselves, or if they accidentally define the requirements incorrectly (e.g- they provide a hash_value property instead of hashValue). I think for this to count as opt-in behaviour the developer should be specifically opting in to synthesised implementations, rather than just declaring conformance as normal and having it kick-in in certain circumstances.

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

While it's a somewhat minor problem, it does commonly involve a lot of boiler-plate where it can also be easy to introduce mistakes. So I do feel a change is warranted.

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

Other than the means of opting in I'd say so, yes.

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

Quick re-read of the proposal, otherwise been following discussion for a while.

···

On 10 Aug 2017, at 00:08, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

[Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.md]

Hi, Tony. Glad to see this back again!

Overall I'm an enthusiastic +1. The restrictions and future work you've listed make sense, and I think this is the right starting place. I just have one thing I'd want to clarify:

Any user-provided implementations of == or hashValue will override the default implementations that would be provided by the compiler.

Does this include implementations in (possibly constrained) protocol extensions? I assume yes, but that's probably worth calling out explicitly. Still, it could be confusing to some users.

Jordan

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance".
The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/
proposals/0185-synthesize-equatable-hashable.md

        • What is your evaluation of the proposal?

Overall a worthwhile addition. Specific comments:

   1. Opt in a good choice, see 2 below.
   2. Opt in should be for trivial enums as well, even though this is a
   breaking change it is clearer.
   3. The hash function should be more tightly specified, without
   specifying what it does it is hard to know if it is appropriate to use in a
   given application. I am not particularly advocating one hash function over
   another, but rather an explanation of what the user can expect. For example
   is hash consistent across invocations on the same machine and is it
   dependent upon the order of stored property declaration. It should also
   state that the hash function is not consistent across machines, since 32
   and 64 bit machines will have a different hash (as an implementation detail
   the hash function should be required to be different on different machines
   of the same architecture so that people do not mistakenly assume that for
   example all 64 bit machines produce the same hash). There has been a number
   of problems with the Java, Python, etc hash functions because these items
   were not specified.
   4. Specifying the exact hash function to be used could be considered, if
   the hash function is known then dictionary and dictionary like structures
   can be optimized.
   5. Synthesis by extensions in the same file should be considered, now
   that private extends to the file. (As an aside: also Codeable.)
   6. It is quite possible to have class types automatically synthesize
   hash and equality by calling super.hashValue and super.equals(Self). The
   omission seems at odds with treating classes equally to values.
   7. Same for tuples (should be included).
   8. Transient is a useful marker in Java and therefore should be
   considered (would also work well with Codeable).

Hope the above doesn't read negatively, the proposal as is would be a great
addition - the above are suggested improvements rather than show stopping
musts.

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

Yes, I even have a library function that mimics this because I got fed up
of writing boiler plate.

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

Yes, part of rounding off the language.

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

Yes, see comments above based on experience with other languages

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

Something I have wanted for a while because I have used this feature in
other languages.

-- Howard.

Huge +1

This should have been there from Swift 1, and basically forced me to use Sourcery to automatically generate all that code.

Elviro

···

Il giorno 10 ago 2017, alle ore 01:09, Chris Lattner via swift-evolution <swift-evolution@swift.org> ha scritto:

Hello Swift community,

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance" begins now and runs through August 15, 2017. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.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 Lattner
Review Manager

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

  • What is your evaluation of the proposal?

Very large +1 in general. I really wanted to see this happen in Swift 4. I’m very happy that it’s up for review right at the beginning of the Swift 5 process.

That said, I do think the concern others have voiced regarding implicit synthesis has some merit. Most languages I am familiar with that synthesize memberwise implementations do so using an explicit request (`deriving` or similar). It adds a small amount of boilerplate in exchange for precise control. It seems to me that this tradeoff is in line with Swift’s motto of clarity over concision. If we do make a change to this proposal we should also make the same change for basic enums as well as Codable for the sake of consistency.

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

Yes. Manually writing memberwise implementations is a big enough annoyance that it can influence designs. For example, a library author may be more likely to try and avoid requiring user types to conform when users are required to write those conformances manually.

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

Very much so.

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

It is very similar, although as mentioned, it might make sense to require explicit opt-in to memberwise synthesis.

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

Quick glance this time around, but I have participated heavily in the previous discussions.

<https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.md>

## What is your evaluation of the proposal? ##

+1

When `Codable` can be synthesized in a same-file extension, the same should hopefully be possible with `Equatable` and `Hashable`.

  "We can relax this restriction soon; it will just require some additional implementation that's not ready yet."
  <https://github.com/apple/swift/pull/9758#issuecomment-302752673>

An empty same-file extension will be sufficient to opt-in, so `AutoEquatable` and `AutoHashable` aren't necessary.

  extension Token: Hashable {}

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

Yes, eliminating boilerplate and bug-prone code is a worthwhile contribution.

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

Yes, it follows the existing rules for `Codable` synthesis.

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

A quick reading.

Hello Swift community,

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance"
begins now and runs through August 15, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.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?

Brilliant. However, I believe that this is a typo of some importance to
clarify:

> The compiler synthesizes P's requirements for a struct with one or more
stored properties if and only if all of the types of all of its stored
properties conform to P.

I think the author means to write "...conform to {Equatable | Hashable}";
it is unclear what it means for a struct P to have stored properties that
"conform to P" (which would be an odd restriction in any case). The same
issue occurs in preceding bulleted list with respect to enums.

In that statement, P is intended to be the protocol (Equatable or
Hashable), not the struct—but I can see how "P's requirements" might be
interpreted either way. In an earlier paragraph I state "For brevity, let P
represent either the protocol Equatable or Hashable in the descriptions
below." In other words:

"The compiler synthesizes {Equatable|Hashable}'s requirements for a struct
with one or more stored properties if and only if all of the types of all
of its stored properties conform to {Equatable|Hashable}."

Hopefully that clarifies, but I can update the write-up if necessary.

···

On Wed, Aug 9, 2017 at 4:36 PM Xiaodi Wu via swift-evolution < swift-evolution@swift.org> wrote:

On Wed, Aug 9, 2017 at 6:08 PM, Chris Lattner via swift-evolution < > swift-evolution@swift.org> wrote:

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

This is a wonderful addition.

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

Yes, the author did an excellent job paralleling the existing rules for
Codable synthesis; I agree fully with that approach as it makes this
feature more easily learnable and predictable for all users.

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

I haven't in recent times, but I do know that other languages do this.

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

A quick reading today; an in-depth study previously.

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

Hello Swift community,

The review of SE-0185 - "Synthesizing Equatable and Hashable conformance"
begins now and runs through August 15, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.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?

+1, it addresses one of the biggest pain points that cause people to reach
for metaprogramming facilities.

I have one clarifying point and tweak. The proposal states:

A struct T: Hashable that satisfies the conditions above will receive a
synthesized implementation of var hashValue: Int { get } that uses an
unspecified hash function† to compute the hash value by incorporating the
hash values of the fields as its terms, in definition order.

This means that if the hash-combine operation is not commutative, then
hashValues are not stable modulo member reordering. Was this scenario
explicitly thought through? I don’t think it’s likely to be an issue in
practice, but it's an artifact of implicit and ordered member-wise
synthesis. It’s not obvious that declaration order is clearly the best
order, e.g. declaration order is not necessarily memory-layout order. It
might be better to leave the precise order unspecified, so long as it's
guaranteed consistent during the execution of a program.

The stdlib documentation for hashValue states "*Hash values are not
guaranteed to be equal across different executions of your program. Do not
save hash values to use during a future execution.*" So, we don't need to
provide member-reordering stability since the only way to reorder members
is to recompile the code (at this time).

That being said, we can certainly change the wording from "in definition
order" to "in an arbitrary, implementation-defined order" without changing
the behavior of the proposal and keeping the possibility open for future
optimizations.

···

On Wed, Aug 9, 2017 at 4:59 PM Michael Ilseman via swift-evolution < swift-evolution@swift.org> wrote:

On Aug 9, 2017, at 4:09 PM, Chris Lattner <clattner@nondot.org> wrote:

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

It's a reasonably scoped chunk of functionality for the biggest pain
points, so yes.

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

Haskell and Rust both have “deriving" constructs for generating these
kinds of things, and that simplifies a lot of code.

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

Thought about it a bit; read it quickly.

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-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

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

Hi Haravikk,

I don't feel that conforming to
Equatable/Hashable is sufficient as an opt-in. Consider for example
someone who declares their type as Equatable/Hashable, then sets to
work on the details of their type, forgetting to implement the actual
equatable/hashable behaviour they want.

This is no different than a default implementation of the protocol.
In fact, the proposal just adds something like this:

     extension Struct<A, B, ...>: Equatable where A: Equatable, B: Equatable, ... {
         static func ==(lhs: Self, rhs: Self) -> Bool { /* insert implementation here */ }
     }

We don't require/support some special keywords for other protocols with default implementation either,
so I see no reason to add them here.

Your concerns are orthogonal to this proposal and should be discussed independently.

···

Am 2017-08-10 11:07, schrieb Haravikk via swift-evolution:

--
Martin

[Proposal:
https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.md
]

Hi, Tony. Glad to see this back again!

Overall I'm an enthusiastic +1. The restrictions and future work you've
listed make sense, and I think this is the right starting place. I just
have one thing I'd want to clarify:

Any user-provided implementations of == or hashValue will override the
default implementations that would be provided by the compiler.

Does this include implementations in (possibly constrained) protocol
extensions? I assume yes, but that's probably worth calling out explicitly.
Still, it could be confusing to some users.

Yes, manual implementations added in extensions override the
compiler-synthesized default:

Without constraints:
(swift) struct Foo: Equatable { let x: Int }
(swift) Foo(x: 5) == Foo(x: 6)
// r0 : Bool = false
(swift) Foo(x: 5) == Foo(x: 5)
// r1 : Bool = true
(swift) extension Foo { static func ==(lhs: Foo, rhs: Foo) -> Bool { return
lhs.x % 2 == rhs.x % 2 } }
(swift) Foo(x: 5) == Foo(x: 6)
// r2 : Bool = false
(swift) Foo(x: 5) == Foo(x: 7)
// r3 : Bool = true

With constraints:
(swift) struct Foo<T: Equatable>: Equatable { let t: T }
(swift) extension Foo where T == String { static func ==(lhs: Foo<T>, rhs:
Foo<T>) -> Bool { return lhs.t.characters.count == rhs.t.characters.count }
}
(swift) Foo(t: "foo") == Foo(t: "bar")
// r0 : Bool = true
(swift) Foo(t: 5) == Foo(t: 7)
// r1 : Bool = false

I can update the text to make this explicit.

···

On Thu, Aug 10, 2017 at 11:05 AM Jordan Rose <jordan_rose@apple.com> wrote:

Jordan

I disagree.

This is not the same as a default protocol implementation, as a default implementation can only act upon properties/methods that are clearly defined by the protocol itself. This feature by comparison is an automatic implementation that by design must make assumptions about the contents of a concrete type.

Consider a type that contains some kind of "volatile" data, i.e- properties that are not necessarily crucial to the type, and so should not be considered during equality. If I make a mistake and end up using the synthesised test for equality, then two values that should be identified as equal, will not be equated as such because the synthesised option cannot account for implementation details that it has no awareness of.

This has come up in discussion in the past, where I argued for some means of marking values to prevent them from being included in the synthesised method(s). This is covered in alternatives as "transient" values, but I disagree with the conclusion that this can simply be added later, as it is IMO crucial to the way this feature would work. Even if there were a @transient attribute or whatever from the start, I'd still prefer to set this within the context of having explicitly asked for Equatable/Hashable to be synthesised, and without such a feature it only makes it more crucial that the opt-in occur in a manner more explicit than conforming to Equatable/Hashable.

My disagreement pretty much boils down to the fact that I do not feel that opting in simply by conforming is sufficient given that the desired behaviour is assumed; I strongly feel that a more explicit declaration of "I specifically want to use this automatic behaviour" is required, either via attribute or conforming to a more explicit protocol.

I actually like the fact that conforming to Equatable requires me to implement ==, as it ensures that I do-so, and prefer that that behaviour remain, as it forces me to do something about it. Even with synthesised behaviour as an option, it should be exactly that, an option; i.e- when my conformance to Equatable fails, I can choose either to implement ==, or opt-in to an automatic substitute.

···

On 10 Aug 2017, at 10:32, Martin Waitz <tali@admingilde.org> wrote:

Hi Haravikk,

Am 2017-08-10 11:07, schrieb Haravikk via swift-evolution:

I don't feel that conforming to
Equatable/Hashable is sufficient as an opt-in. Consider for example
someone who declares their type as Equatable/Hashable, then sets to
work on the details of their type, forgetting to implement the actual
equatable/hashable behaviour they want.

This is no different than a default implementation of the protocol.
In fact, the proposal just adds something like this:

   extension Struct<A, B, ...>: Equatable where A: Equatable, B: Equatable, ... {
       static func ==(lhs: Self, rhs: Self) -> Bool { /* insert implementation here */ }
   }

We don't require/support some special keywords for other protocols with default implementation either,
so I see no reason to add them here.

Your concerns are orthogonal to this proposal and should be discussed independently.

Ah, that's not quite the example I meant, and your example isn't a correct demonstration for the REPL. If you want to test the == that's used in the Equatable conformance, you have to call a function that's generic on Equatable.

Anyway, this is the example I meant:

protocol Annoying {}
extension Annoying {
  static func ==(lhs: Self, rhs: Self) -> Bool {
    print("annoying")
    return true
  }
}
struct Foo: Equatable, Annoying {
  let x: Int
}
print(Foo(x: 5) == Foo(x: 6))

I think the correct behavior here is to call the version from Annoying, but I can also see how that would be surprising.

Jordan

···

On Aug 10, 2017, at 14:48, Tony Allevato <tony.allevato@gmail.com> wrote:

On Thu, Aug 10, 2017 at 11:05 AM Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:
[Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.md]

Hi, Tony. Glad to see this back again!

Overall I'm an enthusiastic +1. The restrictions and future work you've listed make sense, and I think this is the right starting place. I just have one thing I'd want to clarify:

Any user-provided implementations of == or hashValue will override the default implementations that would be provided by the compiler.

Does this include implementations in (possibly constrained) protocol extensions? I assume yes, but that's probably worth calling out explicitly. Still, it could be confusing to some users.

Yes, manual implementations added in extensions override the compiler-synthesized default:

Without constraints:
(swift) struct Foo: Equatable { let x: Int }
(swift) Foo(x: 5) == Foo(x: 6)
// r0 : Bool = false
(swift) Foo(x: 5) == Foo(x: 5)
// r1 : Bool = true
(swift) extension Foo { static func ==(lhs: Foo, rhs: Foo) -> Bool { return lhs.x % 2 == rhs.x % 2 } }
(swift) Foo(x: 5) == Foo(x: 6)
// r2 : Bool = false
(swift) Foo(x: 5) == Foo(x: 7)
// r3 : Bool = true

With constraints:
(swift) struct Foo<T: Equatable>: Equatable { let t: T }
(swift) extension Foo where T == String { static func ==(lhs: Foo<T>, rhs: Foo<T>) -> Bool { return lhs.t.characters.count == rhs.t.characters.count } }
(swift) Foo(t: "foo") == Foo(t: "bar")
// r0 : Bool = true
(swift) Foo(t: 5) == Foo(t: 7)
// r1 : Bool = false

I can update the text to make this explicit.

Do you mean something like this, then?

struct Foo: Equatable {
  let x: Int
}

func test<T: Equatable>(_ lhs: T, _ rhs: T) -> Bool {
  return lhs == rhs
}

extension Foo {
  static func == (lhs: Foo, rhs: Foo) -> Bool {
    return lhs.x % 2 == rhs.x % 2
  }
}

print(test(Foo(x: 5), Foo(x: 7)))  // true

That seems to work.

I just tested the Annoying example as well and yes, the version from the
Annoying extension is what gets called in my implementation.

I agree with you that that seems like the correct behavior—the manually
written version takes precedence over the synthesized version, even though
it comes from a different protocol extension; synthesized versions should
always be the last resort because they're not controlled by the user,
correct?

This also seems consistent with the way regular methods are resolved if we
take synthesis completely out of the equation:

protocol Fooquatable {
  static func foo(lhs: Self, rhs: Self) -> Bool
}

protocol Annoying {}
extension Annoying {
  static func foo(lhs: Self, rhs: Self) -> Bool {
    print("annoying")
    return true
  }
}
struct Foo: Fooquatable, Annoying {
  let x: Int
}

func foo<T: Fooquatable>(_ lhs: T, _ rhs: T) -> Bool {
  return T.foo(lhs: lhs, rhs: rhs)
}

print(foo(Foo(x: 5), Foo(x: 6)))  // annoying, true

Does this seems reasonable? (Assuming I'm testing the right thing. :slight_smile:

···

On Thu, Aug 10, 2017 at 2:58 PM Jordan Rose <jordan_rose@apple.com> wrote:

On Aug 10, 2017, at 14:48, Tony Allevato <tony.allevato@gmail.com> wrote:

On Thu, Aug 10, 2017 at 11:05 AM Jordan Rose <jordan_rose@apple.com> > wrote:

[Proposal:
https://github.com/apple/swift-evolution/blob/master/proposals/0185-synthesize-equatable-hashable.md
]

Hi, Tony. Glad to see this back again!

Overall I'm an enthusiastic +1. The restrictions and future work you've
listed make sense, and I think this is the right starting place. I just
have one thing I'd want to clarify:

Any user-provided implementations of == or hashValue will override the
default implementations that would be provided by the compiler.

Does this include implementations in (possibly constrained) protocol
extensions? I assume yes, but that's probably worth calling out explicitly.
Still, it could be confusing to some users.

Yes, manual implementations added in extensions override the
compiler-synthesized default:

Without constraints:
(swift) struct Foo: Equatable { let x: Int }
(swift) Foo(x: 5) == Foo(x: 6)
// r0 : Bool = false
(swift) Foo(x: 5) == Foo(x: 5)
// r1 : Bool = true
(swift) extension Foo { static func ==(lhs: Foo, rhs: Foo) -> Bool {
return lhs.x % 2 == rhs.x % 2 } }
(swift) Foo(x: 5) == Foo(x: 6)
// r2 : Bool = false
(swift) Foo(x: 5) == Foo(x: 7)
// r3 : Bool = true

With constraints:
(swift) struct Foo<T: Equatable>: Equatable { let t: T }
(swift) extension Foo where T == String { static func ==(lhs: Foo<T>, rhs:
Foo<T>) -> Bool { return lhs.t.characters.count == rhs.t.characters.count }
}
(swift) Foo(t: "foo") == Foo(t: "bar")
// r0 : Bool = true
(swift) Foo(t: 5) == Foo(t: 7)
// r1 : Bool = false

I can update the text to make this explicit.

Ah, that's not quite the example I meant, *and* your example isn't a
correct demonstration for the REPL. If you want to test the == that's used
in the Equatable conformance, you have to call a function that's generic on
Equatable.

Anyway, this is the example I meant:

protocol Annoying {}
extension Annoying {

  static func ==(lhs: Self, rhs: Self) -> Bool {

    print("annoying")
    return true
  }
}
struct Foo: Equatable, Annoying {
  let x: Int
}
print(Foo(x: 5) == Foo(x: 6))

I think the correct behavior here is to call the version from Annoying,
but I can also see how that would be surprising.

Jordan

Hi Haravikk,

I don't feel that conforming to
Equatable/Hashable is sufficient as an opt-in. Consider for example
someone who declares their type as Equatable/Hashable, then sets to
work on the details of their type, forgetting to implement the actual
equatable/hashable behaviour they want.

This is no different than a default implementation of the protocol.
In fact, the proposal just adds something like this:

   extension Struct<A, B, ...>: Equatable where A: Equatable, B:
Equatable, ... {
       static func ==(lhs: Self, rhs: Self) -> Bool { /* insert
implementation here */ }
   }

We don't require/support some special keywords for other protocols with
default implementation either,
so I see no reason to add them here.

Your concerns are orthogonal to this proposal and should be discussed
independently.

I disagree.

This is not the same as a default protocol implementation, as a default
implementation can only act upon properties/methods that are clearly
defined by the protocol itself. This feature by comparison is an automatic
implementation that by design must make assumptions about the contents of a
concrete type.

Consider a type that contains some kind of "volatile" data, i.e-
properties that are not necessarily crucial to the type, and so should not
be considered during equality. If I make a mistake and end up using the
synthesised test for equality, then two values that should be identified as
equal, will not be equated as such because the synthesised option cannot
account for implementation details that it has no awareness of.

This has come up in discussion in the past, where I argued for some means

of marking values to prevent them from being included in the synthesised
method(s). This is covered in alternatives as "transient" values, but I
disagree with the conclusion that this can simply be added later, as it is
IMO crucial to the way this feature would work.

Thanks for your feedback, Haravikk!

My hunch is that structs and enums where such volatile data exists are the
exception, not the norm, when it comes to considering equality. This
proposal is very much intended to be useful syntactic sugar for the common
case while not preventing any existing behavior (providing your own
implementation) and alluding to future directions where the behavior can be
extended to other use cases.

I disagree that @transient is crucial for this feature to work—that feature
only applies to the set of types that contain volatile data, that volatile
data and all the other fields are all Equatable/Hashable, and the user
wants to synthesize Eq/Hash for a subset of its fields. I certainly agree
that such a set is non-empty, but I it's also small enough that it can be
considered at a later date.

It's important for a proposal like this to be as focused and targeted as
possible, and leave features that are not absolutely required for future
directions. For the record, I would *love* to see something like @transient
proposed after this (which is why I call it out in the proposal), but such
a feature would also affect Codable and this proposal should not mix
concerns.

Even if there were a @transient attribute or whatever from the start, I'd
still prefer to set this within the context of having explicitly asked for
Equatable/Hashable to be synthesised, and without such a feature it only
makes it more crucial that the opt-in occur in a manner more explicit than
conforming to Equatable/Hashable.

My disagreement pretty much boils down to the fact that I do not feel that
opting in simply by conforming is sufficient given that the desired
behaviour is assumed; I strongly feel that a more explicit declaration of
"I specifically want to use this automatic behaviour" is required, either
via attribute or conforming to a more explicit protocol.

I actually *like* the fact that conforming to Equatable requires me to
implement ==, as it ensures that I do-so, and prefer that that behaviour
remain, as it forces me to do something about it. Even with synthesised
behaviour as an option, it should be exactly that, an option; i.e- when my
conformance to Equatable fails, I can choose either to implement ==, or
opt-in to an automatic substitute.

I sympathize with these concerns, and a great deal of thought and
discussion was done around the best way to surface this feature in the last
couple of mailing list threads about this topic. But at the time this
proposal and its implementation were written, the synthesis for Codable had
already been implemented and this proposal adopts the same synthesis
conditions that were used by Codable. Core team members expressed that the
implementation for Eq/Hash should follow those existing practices as
closely as possible; I agreed that consistency is important so that we
don't have to document intricate nuances between the implementations of
protocol synthesis.

Given that constraint, IMO it would be incorrect for this proposal to
create an inconsistent set of synthesis conditions compared to Codable, and
it is out of scope for this proposal to suggest changes to Codable. If you
feel that synthesis should be made more explicit across Equatable,
Hashable, and Codable, please do propose that!

···

On Thu, Aug 10, 2017 at 3:40 AM Haravikk via swift-evolution < swift-evolution@swift.org> wrote:

On 10 Aug 2017, at 10:32, Martin Waitz <tali@admingilde.org> wrote:
Am 2017-08-10 11:07, schrieb Haravikk via swift-evolution:

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

Actually, I could easily imagine that a future version of Swift with macro support might do this with a default protocol implementation:

  extension Equatable {
    #generated static func == (lhs: Self, rhs: Self) -> Bool {
      switch self {
      case let self as Struct:
        let propertyPairs = properties.map { property in
          guard let property = property as? Property<Generated.Equatable> else {
            throw SynthesisError.invalidType(property, expected: Generated.Equatable.self)
          }
          return DependentExpressionPair<Generated.Equatable, SameType>(
            original: Expression { lhs.#property },
            dependent: Expression { rhs.#property }
          )
        }
        return FunctionBlock { return #equated(propertyPairs) }
        
      case let self as Enum:
        guard !all.isEmpty else {
          throw SynthesisError.invalid(self, message: "is an empty enum")
        }
        
        let cases: SwitchBody<(Generated.Self, Generated.Self)> = all.map { aCase in
          let valueCaptures = aCase.associatedValues.map { value in
            guard let value = value as? AssociatedValue<Generated.Equatable> else {
              throw SynthesisError.invalidType(value, expected: Generated.Equatable.self)
            }
            return DependentExpressionPair<Generated.Equatable, SameType>(
              original: Variable(type: value.type),
              dependent: Variable(type: value.type)
            )
          }
          
          return SwitchBody {
            case let (
              #aCase.pattern(capturingInto: valueCaptures.map(\.original)),
              #aCase.pattern(capturingInto: valueCaptures.map(\.dependent))
            ):
              return #equated(valueCaptures)
          }
        }.joined()
        
        return FunctionBlock { switch (lhs, rhs) { #cases } }
        
      default:
        throw SynthesisError.invalid(self, message: "is not a struct or enum")
      }
    }
  }
  
  private #func equated(_ exprPairs: [DependentExpressionPair<Generated.Equatable, SameType>]) -> Expression<Generated.Bool> {
    return exprPairs.map { pair in
      Expression { #pair.original == #pair.dependent }
    }
    .reduce(Expression { true }) { earlier, this in
      Expression { #earlier && #this }
    }
  }

If the only difference were whether the default implementation was generated by a macro or not, would you still think auto-derivation should be marked with a keyword?

···

On Aug 10, 2017, at 3:40 AM, Haravikk via swift-evolution <swift-evolution@swift.org> wrote:

This is not the same as a default protocol implementation

--
Brent Royal-Gordon
Architechies

Do you mean something like this, then?

struct Foo: Equatable {
  let x: Int
}

func test<T: Equatable>(_ lhs: T, _ rhs: T) -> Bool {
  return lhs == rhs
}

extension Foo {
  static func == (lhs: Foo, rhs: Foo) -> Bool {
    return lhs.x % 2 == rhs.x % 2
  }
}

print(test(Foo(x: 5), Foo(x: 7)))  // true

That seems to work.

Ah, yes, this works in a source file. It may even work in the REPL, if the REPL delays evaluating input until it sees an actual statement. However, the original REPL transcript you pasted in would not have worked.

I just tested the Annoying example as well and yes, the version from the Annoying extension is what gets called in my implementation.

I agree with you that that seems like the correct behavior—the manually written version takes precedence over the synthesized version, even though it comes from a different protocol extension; synthesized versions should always be the last resort because they're not controlled by the user, correct?

This also seems consistent with the way regular methods are resolved if we take synthesis completely out of the equation:

protocol Fooquatable {
  static func foo(lhs: Self, rhs: Self) -> Bool
}

protocol Annoying {}
extension Annoying {
  static func foo(lhs: Self, rhs: Self) -> Bool {
    print("annoying")
    return true
  }
}
struct Foo: Fooquatable, Annoying {
  let x: Int
}

func foo<T: Fooquatable>(_ lhs: T, _ rhs: T) -> Bool {
  return T.foo(lhs: lhs, rhs: rhs)
}

print(foo(Foo(x: 5), Foo(x: 6)))  // annoying, true

Does this seems reasonable? (Assuming I'm testing the right thing. :slight_smile:

Yep, that's why I think this is the right behavior as well. It does mean that this synthesis doesn't quite behave like a usual default implementation, because that would make this code ambiguous. If we really wanted it to match that behavior we'd have to rank it like a kind of protocol extension member. I think it's okay not to do that, though.

Jordan

···

On Aug 10, 2017, at 15:34, Tony Allevato <tony.allevato@gmail.com> wrote:

My hunch is that structs and enums where such volatile data exists are the
exception, not the norm, when it comes to considering equality. This
proposal is very much intended to be useful syntactic sugar for the common
case while not preventing any existing behavior (providing your own
implementation) and alluding to future directions where the behavior can be
extended to other use cases.

I have similar concerns to Haravikk and I also understand the reasoning you
outline.

Reviewing code of ours and thinking back in time to prior (pre-swift
projects) I currently think that automatic synthesis by simple statement of
adopting Equat/Hashable would only be possible for the simplest of data
structs and classes. It is often that more (slightly) complex structs and
classes have various bookkeeping, metadata, and/or caching properties that
should not be considered in the synthesized code however theses object are
maintained in data structures that depend on Equat/Hashable conformance. In
other words I would have to write an implementation myself (despite a
synthesized version could be done if a subset of properties could be
excluded) or possibly I would adopt one of those protocols and mistakenly
forget to write my own implementation because automatic synthesis kicks in
causing a lurking problem (so hopefully be quickly discovered).

Also for data structs/classes built to deal with data exchange, say with a
server API using JSON, the resulting data objects that leverage the new
codable stuff in Swift 4 will almost universally have things that would
allow automatic synthesis of Equat/Hashable however equally universally the
equality of two objects decoded would depend on a small subset of the
properties contained in the object (say a server id and some number of
additional scoping ids). So I doubt that simply using something that would
opt-out elements from automatic coding would map directly to opt-out of
automatic compatibility, it wouldn't be a 1:1 mapping... likely seldom
would it be in my experience.

So at this time this feature would likely be of little utility to me in my
code.

With all that said... I want this feature +1 but with some futur things
being omitted I won't gain much near term benefit form it.

-Shawn