Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-6696] Value types bridged to Objective-C must be Hashable for -isEqual: to work #49245

Open
nolanw opened this issue Jan 4, 2018 · 9 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. runtime The Swift Runtime standard library Area: Standard library umbrella

Comments

@nolanw
Copy link

nolanw commented Jan 4, 2018

Previous ID SR-6696
Radar rdar://problem/36319754
Original Reporter @nolanw
Type Bug

Attachment: Download

Environment

Xcode Version 9.2 (9C40b)
Default toolchain

Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug, Runtime
Assignee @lorentey
Priority Medium

md5: b462c8bc866585f5bb9e00b013767a87

Issue Description:

When using a Swift value type in Objective-C, it seems that the value type must be Hashable in order for isEqual: to call through to the underlying Swift == implementation:

BOOL ClandIsEqual(id a, id b)
{
    return [a isEqual: b];
}
struct S: Equatable {
    let a: String

    static func == (lhs: S, rhs: S) -> Bool { return lhs.a == rhs.a }
}

let s1 = S(a: "hi")
let s2 = S(a: "hi")

print("# Just Equatable")
print("swift says \(s1 == s2)")
print("c says \(ClandIsEqual(s1, s2))")

print()

struct T: Hashable {
    let a: String

    static func == (lhs: T, rhs: T) -> Bool { return lhs.a == rhs.a }
    var hashValue: Int { return a.hashValue }
}

let t1 = T(a: "hi")
let t2 = T(a: "hi")

print("# Also Hashable")
print("swift says: \(t1 == t2)")
print("c says \(ClandIsEqual(t1, t2))")

outputs:

# Just Equatable
swift says true
c says false

# Also Hashable
swift says: true
c says true

Ideally I'd like something Equatable bridged to Objective-C to respond to isEqual: by calling the underlying == implementation. If Hashable is a hard requirement for some reason, then I guess this is a request for clearer documentation around that fact.

I've attached an Xcode project with the above code. I'm pretty sure it happens when building outside of Xcode but I didn't bother figuring out how to use a bridging header from the command line.

@belkadan
Copy link
Contributor

belkadan commented Jan 5, 2018

cc @jckarter. We have a Radar for this, right?

@jckarter
Copy link
Member

jckarter commented Jan 5, 2018

I can't find it right now, but it is a known issue.

@jckarter
Copy link
Member

jckarter commented Jan 5, 2018

@swift-ci create

@lorentey
Copy link
Member

The problem here is that any class that overrides -[NSObject isEqual:] must also define a corresponding implementation for -[NSObject hash], and we can't provide a good implementation of the latter for Swift types that only conform to Equatable. Cocoa objects must be both equatable and hashable; we can't have one but not the other.

Defining isEqual(_🙂 but keeping the default hash implementation is not an option. One way to resolve this would be to provide a hash override in these cases that simply traps on call. However, I think this would go against the spirit of NSObject, and could cause backward compatibility issues for Cocoa frameworks. (As in, frameworks can't evolve to start storing user-supplied objects in NSSet/NSDictionary/NSCache/etc instances if they did not already did so in their initial release.)

https://bugs.swift.org/browse/SR-7284 is a related issue, dealing with the problem that NSObject subclasses are always hashable, but the Swift values they represent may not be – so sets and dictionaries bridged to Swift may contain elements that aren't Hashable.

@belkadan
Copy link
Contributor

We've pointed out in the past that a hash value of 0 would be valid for any such classes.

@lorentey
Copy link
Member

That seems questionable advice to me; ideally we should not continue propagating it. The constant 0 is not really an appropriate hash value for any type that includes more than one distinct value. Such a hash value is attractive as a quick fix, but it causes more problems down the road, by defeating the entire point of hashing. For classes that might get hashed (i.e., all NSObject subclasses except perhaps those whose instances never get passed to frameworks outside the author's control), the only safe option is to implement proper hashing – i.e., to derive a hash value from exactly the same components that the class looks at in its isEqual implementation.

A better advice would be that types conforming to Equatable should also conform to Hashable. Adding the extra conformance usually requires minimal effort, but it unlocks a lot more functionality, and it ensures seamless interoperability with Objective-C interfaces.

@belkadan
Copy link
Contributor

Things are a lot better now than they used to be in that regard, but it's really common to just want to make something Equatable, and not need to put it in an NSDictionary or NSSet. Right now we have a usability trap where equality silently fails. I think it's worth replacing that with a usability trap that hashing is bad.

How about splitting the difference: warning (once per type) if the bad -hash method is actually called?

@lorentey
Copy link
Member

That could work!

Although I wonder how much of an overkill it would be if we would produce a compile-time warning whenever an Equatable but not Hashable type is wrapped in a _SwiftValue.

@belkadan
Copy link
Contributor

I don't think we know when that happens. Consider myVal as Any as AnyObject.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. runtime The Swift Runtime standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

4 participants