Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-11 Thread Haravikk via swift-evolution

> On 10 Aug 2017, at 23:29, Brent Royal-Gordon  wrote:
> 
> 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?

Yes.

With or without a macro the issue here is still that the synthesised behaviour 
is generated based upon assumptions about a concrete type, whereas current 
default implementations are only based upon the methods/properties that are 
defined by the protocol itself, i.e- if the protocol is well-defined, and you 
implement it accordingly, then all a default implementation should be doing is 
correctly using the protocol. The problem with the synthesised behaviour is 
that it must necessarily make assumptions about parts of the concrete type that 
are not defined by the protocol and may in fact have nothing to do with it at 
all (e.g- could be properties implementing an entirely different protocol).

Whether or not this is generated by a macro is not the issue. If we assume 
macros will eventually replace features like this one, then I would still want 
a clear standard on how to distinguish between opting in to synthesised 
behaviour such as this.


To be clear as well; I didn't specifically request a keyword. In fact I'm 
leaning more towards separate protocols personally; i.e- if you just want to 
adopt the requirements you would conform to Equatable, but if you want to use 
the synthesised behaviour you would instead conform to AutoEquatable (or some 
similar naming convention). The important distinction here being that simply 
conforming to Equatable will still give you all the usual errors for failed 
requirements, at which point you can either fulfil them, or switch to 
AutoEquatable instead if you prefer.

This kind of standard would make sense for any protocol that wants to add 
default implementations that are "invasive", i.e- delve deeper into the 
concrete type than simply what the protocol itself defines as possible, i.e- 
behaviours that by their very nature must make assumptions about the concrete 
type. This might be fine for a protocol that is clearly defined as having such 
behaviour from the start, but personally I'd rather see a standard set that has 
it split off into a more specific protocol, such that we still have "pure" 
protocols which remain primarily as a set of requirements.

In other words, Equatable would contain all the basic requirements and the "non 
invasive" default implementations, while AutoEquatable would extend it with the 
macros/special behaviour that provides the synthesised defaults, allowing a 
developer to opt-in to them explicitly.

> On 10 Aug 2017, at 16:19, Tony Allevato  wrote:
> 
> 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.

I disagree with your reasons for disagreeing 

The problem is that synthesising the behaviour for such types right now will 
introduce a bug, and one that will not be exposed to developers except at 
runtime, which is something that makes me very uncomfortable. To be clear, my 
point is that a transient feature is a requirement now specifically because the 
proposal seeks to use conforming to Equatable as sufficient as an opt-in; if a 
more explicit opt-in were required then this would not be an issue, i.e- if a 
developer conforms to AutoEquatable instead of Equatable, then we can assume 
that they should know what that means, and should not do it unless they know 
their type contains no transient values.

With that kind of absolutely explicit opt-in then, yes, a @transient attribute 
or whatever can be deferred to a future feature. But with opt-in via Equatable 
conformance the lack of such a feature is much more pronounced. Though like I 
say, my preference is very much towards a more explicit opt-in, rather than an 
attribute/keyword that developers may not realise they need to use with regular 
Equatable (remember, not everyone will know that Equatable is gaining this new 
behaviour).

> 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 

Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Jordan Rose via swift-evolution


> On Aug 10, 2017, at 15:34, Tony Allevato  wrote:
> 
> Do you mean something like this, then?
> 
> ```
> struct Foo: Equatable {
>   let x: Int
> }
> 
> func test(_ 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(_ 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. :)

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

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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Tony Allevato via swift-evolution
Do you mean something like this, then?

```
struct Foo: Equatable {
  let x: Int
}

func test(_ 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(_ 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. :)


On Thu, Aug 10, 2017 at 2:58 PM Jordan Rose  wrote:

>
>
> On Aug 10, 2017, at 14:48, Tony Allevato  wrote:
>
> On Thu, Aug 10, 2017 at 11:05 AM Jordan Rose 
> 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: Equatable { let t: T }
> (swift) extension Foo where T == String { static func ==(lhs: Foo, rhs:
> Foo) -> 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
>
>
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Brent Royal-Gordon via swift-evolution
> On Aug 10, 2017, at 3:40 AM, Haravikk via swift-evolution 
>  wrote:
> 
> This is not the same as a default protocol implementation

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 else {
throw 
SynthesisError.invalidType(property, expected: Generated.Equatable.self)
}
return 
DependentExpressionPair(
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 else {
throw 
SynthesisError.invalidType(value, expected: Generated.Equatable.self)
}
return 
DependentExpressionPair(
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]) -> 
Expression {
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?

-- 
Brent Royal-Gordon
Architechies

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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Jordan Rose via swift-evolution


> On Aug 10, 2017, at 14:48, Tony Allevato  wrote:
> 
> On Thu, Aug 10, 2017 at 11:05 AM Jordan Rose  > 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: Equatable { let t: T }
> (swift) extension Foo where T == String { static func ==(lhs: Foo, rhs: 
> Foo) -> 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

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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Jordan Rose via swift-evolution
[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
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Shawn Erickson via swift-evolution
> 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
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Tony Allevato via swift-evolution
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  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: 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

Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Elviro Rocca via swift-evolution
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 
>  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

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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Haravikk via swift-evolution

> On 10 Aug 2017, at 10:32, Martin Waitz  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: 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. 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.___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Martin Waitz via swift-evolution

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: 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.


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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-10 Thread Haravikk via swift-evolution

> On 10 Aug 2017, at 00:08, Chris Lattner via swift-evolution 
>  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.
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-09 Thread Tony Allevato via swift-evolution
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:
>
>> 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.



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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

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

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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-09 Thread Jon Shier via swift-evolution
> • What is your evaluation of the proposal?

+∞ - 1 (for no extension synthesis)

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

Most certainly. This is a major pain point, especially for structs with many 
members.

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

Yes, though I don’t particularly care about compiler magic like some, 
especially in such a useful case.

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

I believe Scala with synthesize the equivalent of Equatable conformance for 
class cases, and I’ve used Mantle in Objective-C, which did the same 
dynamically for types which inherited from the right types.

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

Studied it when originally proposed, glanced for a rearview.



Jon Shier

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


Re: [swift-evolution] [Review] SE-0185 - Synthesizing Equatable and Hashable conformance

2017-08-09 Thread Chris Lattner via swift-evolution
I forgot to mention that the implementation of this feature is available here:
https://github.com/apple/swift/pull/9619

-Chris

> On Aug 9, 2017, at 4:08 PM, Chris Lattner  wrote:
> 
> 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/0182-newline-escape-in-strings.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