SE-0494: Add `isIdentical(to:)` Methods for Quick Comparisons to Concrete Types

[vanvoorden (Rick van Voorden)]

@grynspan

One thing that stood out from the proposal in re Optional conformance:

Because this extension needs no private or internal symbols from Standard Library, we can omit this extension from our proposal. Product engineers that want this extension can choose to implement it for themselves.

I know there's no formal protocol defined in the proposal, but we would not give this sort of advice if there were one because of the dangers of competing retroactive conformances. Since the implementation of Optional.isIdentical(to:) is trivial, why not provide it and save developers the duplicated effort?

I'm not completely sure there's a strong community consensus that adding more methods to Optional is even desirable. For example:

@FlorianPircher

I feel uneasy about adding methods to Optional. Optionals are mostly interacted with using overloads, operators, and syntax sugar in Swift. Correctly, every time I use map on an Optional feels like I am making the code more difficult to read for future me.

For example, an Int does not have a map method, so if I read something like counter.map { $0 + set.count } I get that counter is actually an Int?. But String, Array, etc. also have map methods and now when I am bug hunting I am reading such code much more slowly, checking if (…).map { … } should have been (…)?.map { … }.

A product engineer might choose to implement Optional identical tests with a free function. I don't personally have a very strong opinion which approach is better to the extent I am ready to ship that opinion in Standard Library.

We don't discuss "backdeployment" too much in library evolution… but as of right now I do believe the plan is to implement these methods with alwaysEmitIntoClient or backDeployed wherever possible. So AFAIK there could be a legit impact to binary footprint if we choose to include a method on Optional.

I'm not completely opposed to adding the Optional methods. If there is a strong consensus here in proposal review I'm not going to push back strongly on that. But as of right now I see more arguments in favor of not shipping the Optional methods in Standard Library at this time.

---

@ksluder

struct SubSet<Element: Equatable> {
  let parent: Set<Element>
  var index: Int
  var length: Int

  func isIdentical(to other: SubSet<Element>) -> Bool {
    parent == other.parent && (
      (index == other.index && length == other.length) ||
      (index + length == other.index && other.index + other.length == index)
  )

  func ==(other: SubSet<Element>) {
    if isIdentical(to: other) { return true}
    if length != other.length { return false }

    let ours: [Element]
    if length < 0 {
      ours = _sort(parent, index-length..<index)
    } else {
      ours = _sort(parent, index..<index+length)
    }

    let theirs: [Element]
    if other.length < 0 {
      ours = _sort(other.parent, other.index-other.length..<other.index)
    } else {
      ours = _sort(other.parent, other.index..<other.index+other.length)
    }

    return ours == theirs
}

let foo = Set()
let sub1 = SubSet(parent: foo, index: 0, length: 3)
let sub2 = SubSet(parent: foo, index: 3, length: -3)
assert(sub1.isIdentical(to: sub2))

This allows one to quickly check if two SubSets refer to the exact same subset of the same parent Set without performing a full equality check. This is useful for building fast paths as shown in SubSet.==, and also when building algorithms atop data structures like SubSet.

This is why I recommended renaming this method to isKnownIdentical(to:).

I'm having trouble tracking this feedback. Given that our primary goal with this proposal is to focus on a well-defined set of concrete types, could you please help me understand what your argument is in favor of renaming isIdentical(to:) to isKnownIdentical(to:) on the concrete types from the proposal?

@ksluder

That said, I’ve also revised my post to make my point clearer: this proposal introduces isIdentical(to:) as a term of art, but explicitly does not define it in terms of a relationship to ==.

I'm not sure I follow this feedback. Are you directing this feedback at the concrete types in the proposal? Which concrete types are failing to explain the relationship to value-equality?

---

@Stan_Smida

-0.isIdentical(to: 0) returning true from fast path formally passes the axioms, but is unsound.

Our proposal is not adding isIdentical(to:) on Int and this is specifically called out in the proposal as a type we do not recommend for isIdentical(to:).

@Stan_Smida

It is reasonable to have Double.nan.isIdentical(to: .nan) returning true, yet it would violate the equality axiom.

Our proposal is not adding isIdentical(to:) on Float or Double. Neither of these types are good candidates for isIdentical(to:).

Our proposal also specifically mentions that nan is an "exceptional" value that technically violates our equivalence relations as a known edge case. This is consistent with long-established precedent from Standard Library.

@Stan_Smida

Even arguably awkward isFastEqual(to:) sounds better to me.

Our isIdentical(to:) on Array does not require the Array to be Equatable. I do not agree that putting "equal" in the name of the API is the correct decision.

@Stan_Smida

Or come up with some agreeable operator, like \== or whatever.

As discussed previously I am pushing back on any feedback requesting an operator for this proposal. This can and should be a named method and nothing I have seen so far has persuaded me to change my opinion about that.