Skip to content

[Sema] Emit a deprecation warning if a Hashable type only implements hashValue #21057

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 4 commits into from
Dec 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4263,6 +4263,10 @@ WARNING(non_exhaustive_switch_warn,none, "switch must be exhaustive", ())
WARNING(override_nsobject_hashvalue,none,
"override of 'NSObject.hashValue' is deprecated; "
"override 'NSObject.hash' to get consistent hashing behavior", ())
WARNING(hashvalue_implementation,none,
"'Hashable.hashValue' is deprecated as a protocol requirement; "
"conform type %0 to 'Hashable' by implementing 'hash(into:)' instead",
(Type))

#ifndef DIAG_NO_UNDEF
# if defined(DIAG)
Expand Down
9 changes: 6 additions & 3 deletions lib/Sema/DerivedConformanceEquatableHashable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
// The hashValue failure will produce a diagnostic elsewhere.
return nullptr;
}
if (hashValueDecl && hashValueDecl->isImplicit()) {
if (hashValueDecl->isImplicit()) {
// Neither hashValue nor hash(into:) is explicitly defined; we need to do
// a full Hashable derivation.

Expand Down Expand Up @@ -1209,8 +1209,11 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
llvm_unreachable("Attempt to derive Hashable for a type other "
"than a struct or enum");
} else {
// We can always derive hash(into:) if hashValue has an explicit
// implementation.
// hashValue has an explicit implementation, but hash(into:) doesn't.
// Emit a deprecation warning, then derive hash(into:) in terms of
// hashValue.
TC.diagnose(hashValueDecl->getLoc(), diag::hashvalue_implementation,
Nominal->getDeclaredType());
return deriveHashable_hashInto(*this,
&deriveBodyHashable_compat_hashInto);
}
Expand Down
3 changes: 3 additions & 0 deletions stdlib/public/core/Hashable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ public protocol Hashable : Equatable {
///
/// Hash values are not guaranteed to be equal across different executions of
/// your program. Do not save hash values to use during a future execution.
///
/// - Important: `hashValue` is deprecated as a `Hashable` requirement. To
/// conform to `Hashable`, implement the `hash(into:)` requirement instead.
var hashValue: Int { get }

/// Hashes the essential components of this value by feeding them into the
Expand Down
54 changes: 54 additions & 0 deletions test/Sema/struct_equatable_hashable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,60 @@ protocol ImplierMain: Equatable {}
struct ImpliedMain: ImplierMain {}
extension ImpliedOther: ImplierMain {}


// Hashable conformances that rely on a manual implementation of `hashValue`
// should produce a deprecation warning.
struct OldSchoolStruct: Hashable {
static func ==(left: OldSchoolStruct, right: OldSchoolStruct) -> Bool {
return true
}
var hashValue: Int { return 42 }
// expected-warning@-1{{'Hashable.hashValue' is deprecated as a protocol requirement; conform type 'OldSchoolStruct' to 'Hashable' by implementing 'hash(into:)' instead}}
}
enum OldSchoolEnum: Hashable {
case foo
case bar

static func ==(left: OldSchoolEnum, right: OldSchoolEnum) -> Bool {
return true
}
var hashValue: Int { return 23 }
// expected-warning@-1{{'Hashable.hashValue' is deprecated as a protocol requirement; conform type 'OldSchoolEnum' to 'Hashable' by implementing 'hash(into:)' instead}}
}
class OldSchoolClass: Hashable {
static func ==(left: OldSchoolClass, right: OldSchoolClass) -> Bool {
return true
}
var hashValue: Int { return -9000 }
// expected-warning@-1{{'Hashable.hashValue' is deprecated as a protocol requirement; conform type 'OldSchoolClass' to 'Hashable' by implementing 'hash(into:)' instead}}
}

// However, it's okay to implement `hashValue` as long as `hash(into:)` is also
// provided.
struct MixedStruct: Hashable {
static func ==(left: MixedStruct, right: MixedStruct) -> Bool {
return true
}
func hash(into hasher: inout Hasher) {}
var hashValue: Int { return 42 }
}
enum MixedEnum: Hashable {
case foo
case bar
static func ==(left: MixedEnum, right: MixedEnum) -> Bool {
return true
}
func hash(into hasher: inout Hasher) {}
var hashValue: Int { return 23 }
}
class MixedClass: Hashable {
static func ==(left: MixedClass, right: MixedClass) -> Bool {
return true
}
func hash(into hasher: inout Hasher) {}
var hashValue: Int { return -9000 }
}

// FIXME: Remove -verify-ignore-unknown.
// <unknown>:0: error: unexpected error produced: invalid redeclaration of 'hashValue'
// <unknown>:0: error: unexpected note produced: candidate has non-matching type '(Foo, Foo) -> Bool'
Expand Down