Skip to content

[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

Merged
merged 1 commit into from
Oct 30, 2018

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Oct 29, 2018

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 the hash property. Classes that override hashValue instead break NSObject expectations, and will not work correctly in Foundation's NSSet, 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

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.
@lorentey
Copy link
Member Author

@swift-ci please test

@lorentey
Copy link
Member Author

@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}}
}
}
Copy link
Contributor

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?

Copy link
Member Author

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

@parkera
Copy link
Contributor

parkera commented Oct 29, 2018

cc @michael-lehew

@lorentey
Copy link
Member Author

@swift-ci please test source compatibility

@lorentey
Copy link
Member Author

Luckily, the ReactiveCocoa/ReactiveSwift failures seem unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants