Xcode 10 GM: hash(into:) issue from NSObject class

Using Swift
1

import Foundation

public final class HashClass: NSObject {
var i: Int = 0

public override func hash(into hasher: inout Hasher) {
    hasher.combine(i)
}

}

Gives Overriding declarations in extensions is not supported error.

This a bug? If not then how should I do it? I have several classes that derive from NSObject with custom hash values computed.

2 Likes
2

Since NSObject has a hashValue property that Objective-C uses, I believe you have to override that one. @lorentey may have some further insights.

3

That is what I currently do but that generates deprecation warnings and I like to keep my compiles warning free.

4

Cocoa defines its own hashing API that is independent of Swift's Hashable. To customize hashing in NSObject subclasses, the correct property to override is NSObject.hash.

Trying to customize hashing by overriding hash(into:) and/or hashValue will not work correctly -- if you do that then NSDictionary, NSSet, and Foundation's other hashing collections won't work correctly with your class, often leading to severe problems.

It's rather unfortunate that hash and hashValue have distinct names; they are all too easy to confuse. In earlier Swift versions, NSObject.hashValue was accidentally left overridable, which added to the confusion.

Existing code that overrides NSObject.hashValue has been broken from the start; it needs to be fixed. To help us fix existing code and to prevent further problems, Swift 4.2 emits a deprecation warning for NSObject.hashValue overrides, and defines NSObject.hash(into:) non-overridable.

This doesn't mean that you can't use Hasher for such subclasses. For example, here is the proper way to define HashClass with custom hashing:

import Foundation

public final class HashClass: NSObject {
  var i: Int = 0

  public override func isEqual(_ other: Any) -> Bool {
    guard let other = other as? HashClass else { return false }
    return self.i == other.i
  }

  public override var hash: Int {
    var hasher = Hasher()
    hasher.combine(i)
    return hasher.finalize()
  }
}

Important note: If you override NSObject.hash, you must also override isEqual(_:). This isn't currently enforced, but forgetting to do so is another source of subtle but deadly bugs.

Any isEqual(_:) override you provide will get used by both Swift's == operator and Objective-C code that works with NSObjects.

8 Likes
5

Ugh, I got the names backwards.

Also, overriding hash but not isEqual(_:) is fine, if silly. The other way around is the "must".

6

Thanks

7

Also, overriding hash but not isEqual(_:) is fine, if silly. The other way around is the "must".

Hashing is a fickle beast that likes to turn on people who don't feed it properly.

As a general rule of thumb, in order for hash tables to work as advertised, the hash value must be calculated from exactly the same bits and pieces that are used for equality checks. Not more, not less.

  • Hashing components that aren't compared is a serious programming error. It results in duplicate keys and/or keys that cannot be found. This can be difficult to debug.
  • Comparing components that aren't hashed usually ends up turning hash tables into particularly slow unsorted arrays. This ruins performance of all but the smallest tables.

(Frustratingly, the latter this is often done deliberately in an attempt to "optimize" hashing. Please don't do that, unless you have full control over the contents of the affected Set/Dictionary instances, and you have actually measured before/after performance for all work load sizes that may occur at runtime.)

In any case, always override both NSObject.isEqual(_:) and .hash, or override neither.

4 Likes
8

Why is it like this, instead of renaming NSObject's hash as hashValue in the Swift overlay?

9

That would have been a great solution if we'd thought of it before source compatibility began. :-(

10

This seems like one of those “actively harmful” things worth breaking compatibility to fix.

7 Likes
11

I agree.

From GitHub - apple/swift-evolution: This maintains proposals for changes and user-visible enhancements to the Swift Programming Language. ...

Source-breaking changes in Swift 5 will have an even higher bar than in Swift 4, following these guidelines:

  • The current syntax/API must be shown to actively cause problems for users.
  • The new syntax/API must be clearly better and must not conflict with existing Swift syntax.
  • There must be a reasonably automated migration path for existing code.

I think we're 3/3 by those metrics.

12

Renaming hash to hashValue in the overlay would have worked before, but given that 4.2 emits the warning forcing people to migrate to hash, renaming it now would just add to the confusion. :cry:

There are two deeper issues I suggest we should talk about.

  1. In Swift 4.2+, types should conform to Hashable by implementing hash(into:), not hashValue. This is not currently possible for NSObject subclasses.
  2. There is a related, possibly even more serious confusion between isEqual(_:) and == that remains unresolved.

Consider the code below. Similar code anecdotally exists in some production codebases today.

class Foo: NSObject {
  var i: Int

  init(_ value: Int) {
    self.i = value
    super.init()
  }
  
  override var hashValue: Int { // THIS IS BROKEN, DO NOT USE
    return i.hashValue
  }

  static func ==(left: HashClass, right: HashClass) -> Bool { // THIS IS BROKEN, DO NOT USE
    return left.i == right.i
  }
}

First, let's recap the problem with that hashValue override: while it works okay in Swift, it does not get picked up by Objective-C code, which calls hash. Foo leaves hash with its original NSObject definition, which is based on object identity.

As we've seen, the Swift 4.2 compiler emits the following warning for this: (line break added for clarity)

warning: override of 'NSObject.hashValue' is deprecated; 
override 'NSObject.hash' to get consistent hashing behavior

This warning spells out the correct solution, hopefully resolving the confusion. (We expect to turn this into a hard error by 5.0.) Sadly the warning doesn't come with a fix-it yet. Adding one would make a good starter bug.

Now let's turn to the == implementation, which is even more problematic. Never mind Objective-C compatibility -- it doesn't even work in Swift!

Foo(42) == Foo(42) // false

This is because it's not Foo that implements Equatable, but rather it's NSObject. And NSObject's definition of == is non-overridable by definition -- you cannot override operators in Swift.

The == above just sits there and confuses anyone reading the code. It looks like a valid definition, but it actually does nothing. I think it may be a good idea to warn about such decoy operator definitions in any context.

(Frustrating note: NSObject also has an isEqual(to:) method, which is confusingly similar to isEqual(_:), but it is a different thing. Mistaking one for the other can lead to long debugging sessions.)

I'm not yet sure how we can enable hash(into:) overrides on NSObject subclasses. It would be possible to do it by adding extra complications to the compiler's Hashable synthesis:

extension NSObject {
  open func hash(into hasher: inout Hasher) {
    hasher.combine(self.hash)
  }
}

class Foo: NSObject {
  let i: Int = 0

  override func hash(into hasher: inout Hasher) { // Supplied manually
    hasher.combine(i)
  }

  override var hash: Int { // Automatically force-synthesized
    var hasher = Hasher()
    hasher.combine(self)
    return hasher.finalize()
  }

  override func isEqual(_ other: Any) -> Bool {
    return i == (other as? Foo)?.i
  }
}

However, this is probably unwise -- Hashable synthesis is already far too complicated, and this scheme could cause problems for subclasses of Swift classes defined in Objective-C code.

2 Likes
13

Better the momentary confusion of another set of warnings in Swift 5 than permanent confusion and bugs for all Obj-C code in Swift.

IMO, Hashable and Equatable for Obj-C objects should come as close to native Swift behavior as possible, with the few unique Obj-C behaviors (isEqual(to:)) well documented as exceptions. Anything else will have lasting negative repercussions for Swift's Obj-C interop.

14

I'm thinking whether this is something that we could address in Foundation somehow, since we own the conflicting hashing bits. I can imagine a few ways to handle them with ObjC (like checking whether a class conforms to Hashable and also doesn't override the NSObject property), but anything we need to do must also work within swift-corelibs-foundation.

1 Like