-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[ObjectiveC] Make NSObject.hashValue non-overridable #20129
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
[ObjectiveC] Make NSObject.hashValue non-overridable #20129
Conversation
We deprecated overriding it in 4.2 with a custom compiler warning. This removes the ability to override it entirely. The correct way to customize hashing for NSObject subclasses is to override the `hash` property.
@swift-ci please test |
@swift-ci please test source compatibility |
|
||
override func hash(into hasher: inout Hasher) { // expected-error {{overriding non-open instance method outside of its defining module}} expected-error {{overriding declarations in extensions is not supported}} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something here should show the correct way to override hash
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! The warning we have in 4.2 is extremely helpful. I have a pending task to improve error diagnostics -- it's not ABI impacting though.
warning: override of 'NSObject.hashValue' is deprecated; override 'NSObject.hash' to get consistent hashing behavior
@swift-ci please test source compatibility |
Luckily, the ReactiveCocoa/ReactiveSwift failures seem unrelated. |
Swift 4.2 deprecated overriding
NSObject.hashValue
, by way of a custom compiler warning. This PR removes the ability to override it entirely.The correct way to customize hashing for
NSObject
subclasses is to override thehash
property. Classes that overridehashValue
instead breakNSObject
expectations, and will not work correctly in Foundation'sNSSet
,NSDictionary
and similar hashed collections.Note: This PR does not remove the compiler warning, as swift-corelibs-foundation still defines
NSObject.hashValue
as open.rdar://problem/42623458