Skip to content

Commit 233b9fc

Browse files
JGiolaJGiola
committed
Warn about derived Hashable implementation if there's a custom Equatable
1 parent 5674b6e commit 233b9fc

File tree

5 files changed

+139
-6
lines changed

5 files changed

+139
-6
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7356,6 +7356,15 @@ WARNING(executor_enqueue_unused_implementation, none,
73567356
"'Executor.enqueue(ExecutorJob)' will never be used, due to the presence of "
73577357
"'enqueue(UnownedJob)'",
73587358
())
7359+
WARNING(synthesized_hashable_may_not_match_custom_equatable, none,
7360+
"automatically generated 'Hashable' implementation for type %0 "
7361+
"may not match the behavior of custom '==' operator", (Type))
7362+
NOTE(add_custom_hashable, none, "add a custom 'hash(into:)' method", ())
7363+
7364+
WARNING(hashvalue_implementation,Deprecation,
7365+
"'Hashable.hashValue' is deprecated as a protocol requirement; "
7366+
"conform type %0 to 'Hashable' by implementing 'hash(into:)' instead",
7367+
(Type))
73597368

73607369
//------------------------------------------------------------------------------
73617370
// MARK: property wrapper diagnostics

‎lib/Sema/DerivedConformance/DerivedConformanceEquatableHashable.cpp

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
229229
auto rhsVar = rhsPayloadVars[varIdx];
230230
auto rhsExpr = new (C) DeclRefExpr(rhsVar, DeclNameLoc(),
231231
/*Implicit*/true);
232-
auto guardStmt = DerivedConformance::returnFalseIfNotEqualGuard(C,
232+
auto guardStmt = DerivedConformance::returnFalseIfNotEqualGuard(C,
233233
lhsExpr, rhsExpr);
234234
statementsInCase.emplace_back(guardStmt);
235235
}
@@ -496,7 +496,7 @@ static CallExpr *createHasherCombineCall(ASTContext &C,
496496
// hasher.combine(_:)
497497
auto *combineCall = UnresolvedDotExpr::createImplicit(
498498
C, hasherExpr, C.Id_combine, {Identifier()});
499-
499+
500500
// hasher.combine(hashable)
501501
auto *argList = ArgumentList::forImplicitUnlabeled(C, {hashable});
502502
return CallExpr::createImplicit(C, combineCall, argList);
@@ -978,14 +978,14 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
978978
if (hashValueDecl->isImplicit()) {
979979
// Neither hashValue nor hash(into:) is explicitly defined; we need to do
980980
// a full Hashable derivation.
981-
981+
982982
// Refuse to synthesize Hashable if type isn't a struct or enum, or if it
983983
// has non-Hashable stored properties/associated values.
984984
auto hashableProto = Context.getProtocol(KnownProtocolKind::Hashable);
985+
auto nominalTy = Nominal->getDeclaredType();
985986
if (!canDeriveConformance(getConformanceContext(), Nominal,
986987
hashableProto)) {
987-
ConformanceDecl->diagnose(diag::type_does_not_conform,
988-
Nominal->getDeclaredType(),
988+
ConformanceDecl->diagnose(diag::type_does_not_conform, nominalTy,
989989
hashableProto->getDeclaredInterfaceType());
990990
// Ideally, this would be diagnosed in
991991
// ConformanceChecker::resolveWitnessViaLookup. That doesn't work for
@@ -1000,6 +1000,27 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
10001000
if (checkAndDiagnoseDisallowedContext(requirement))
10011001
return nullptr;
10021002

1003+
// Warn the user that having a custom == almost surely require a
1004+
// custom Hashable conformance even if, for source stability reasons,
1005+
// it will be synthesized
1006+
auto equatableProto = C.getProtocol(KnownProtocolKind::Equatable);
1007+
auto eqConformance = TypeChecker::conformsToProtocol(nominalTy,
1008+
equatableProto, getParentModule());
1009+
if (eqConformance) {
1010+
DeclName equalsName(C, DeclBaseName(C.Id_EqualsOperator),
1011+
{Identifier(), Identifier()});
1012+
ConcreteDeclRef witness = eqConformance.getWitnessByName(
1013+
nominalTy->getRValueType(), equalsName);
1014+
1015+
auto equalsDeclaration = witness.getDecl();
1016+
if (equalsDeclaration && !equalsDeclaration->isImplicit()) {
1017+
equalsDeclaration->diagnose(
1018+
diag::synthesized_hashable_may_not_match_custom_equatable,
1019+
nominalTy);
1020+
equalsDeclaration->diagnose(diag::add_custom_hashable);
1021+
}
1022+
}
1023+
10031024
if (auto ED = dyn_cast<EnumDecl>(Nominal)) {
10041025
std::pair<BraceStmt *, bool> (*bodySynthesizer)(
10051026
AbstractFunctionDecl *, void *);
@@ -1015,7 +1036,7 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
10151036
&deriveBodyHashable_struct_hashInto);
10161037
else // This should've been caught by canDeriveHashable above.
10171038
llvm_unreachable("Attempt to derive Hashable for a type other "
1018-
"than a struct or enum");
1039+
"than a struct or enum");
10191040
} else {
10201041
// hashValue has an explicit implementation, but hash(into:) doesn't.
10211042
// Emit a deprecation warning, then derive hash(into:) in terms of

‎test/Sema/Inputs/external_equatable_conformance.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
public struct ImplicitEquatable: ExpressibleByIntegerLiteral, Equatable {
2+
public init(integerLiteral: Int) {}
3+
}
4+
5+
public struct ExplicitEquatable: ExpressibleByIntegerLiteral, Equatable {
6+
public init(integerLiteral: Int) {}
7+
8+
public static func == (lhs: ExplicitEquatable, rhs: ExplicitEquatable) -> Bool {
9+
return true
10+
}
11+
}

‎test/Sema/diag_custom_equatable_synthesized_hashable.swift

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -o %t/NormalLibrary.swiftmodule %S/Inputs/external_equatable_conformance.swift
3+
4+
// RUN: %target-typecheck-verify-swift -I %t
5+
6+
import NormalLibrary
7+
8+
// All conformance synthesized no warning emitted
9+
struct NoCustomEquatable: Hashable {
10+
let a:Int
11+
}
12+
13+
// Equatable implemented and Hashable synthesized raise a warning
14+
enum CustomEquatable: Hashable {
15+
case a
16+
17+
static func == (lhs: CustomEquatable, rhs: CustomEquatable) -> Bool {
18+
// expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'CustomEquatable' may not match the behavior of custom '==' operator}}
19+
// expected-note@-2 {{add a custom 'hash(into:)' method}}
20+
return true
21+
}
22+
}
23+
24+
// All conformance implemented no warning emitted
25+
enum CustomHashable: Hashable {
26+
case a
27+
28+
func hash(into hasher: inout Hasher) {}
29+
30+
static func == (lhs: CustomHashable, rhs: CustomHashable) -> Bool {
31+
return true
32+
}
33+
}
34+
35+
struct ExtendedConformance: Hashable {
36+
let a:Int
37+
}
38+
39+
extension ExtendedConformance {
40+
static func == (lhs: ExtendedConformance, rhs: ExtendedConformance) -> Bool {
41+
// expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'ExtendedConformance' may not match the behavior of custom '==' operator}}
42+
// expected-note@-2 {{add a custom 'hash(into:)' method}}
43+
return true
44+
}
45+
}
46+
47+
enum ImportedConformance: ImplicitEquatable {
48+
case x = 1
49+
}
50+
51+
enum ImportedExplicitConformance: ExplicitEquatable {
52+
case x = 1
53+
}
54+
55+
protocol DefaultedImplementationProtocol: Equatable {}
56+
extension DefaultedImplementationProtocol {
57+
static func == (lhs: Self, rhs: Self) -> Bool { true }
58+
// expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'DefaultedImplementation' may not match the behavior of custom '==' operator}}
59+
// expected-note@-2 {{add a custom 'hash(into:)' method}}
60+
}
61+
62+
struct DefaultedImplementation: DefaultedImplementationProtocol, Hashable {
63+
let a: Int
64+
}
65+
66+
protocol ConditionalImplementationProtocol {}
67+
extension ConditionalImplementationProtocol where Self: Equatable {
68+
static func == (lhs: Self, rhs: Self) -> Bool { true }
69+
// expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'ConditionalImplementation' may not match the behavior of custom '==' operator}}
70+
// expected-note@-2 {{add a custom 'hash(into:)' method}}
71+
}
72+
73+
struct ConditionalImplementation: ConditionalImplementationProtocol, Hashable {
74+
let a: Int
75+
}
76+
77+
protocol ConditionalHashableImplementationProtocol {}
78+
extension ConditionalHashableImplementationProtocol where Self: Equatable {
79+
static func == (lhs: Self, rhs: Self) -> Bool { true }
80+
// expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'ConditionalHashableImplementation' may not match the behavior of custom '==' operator}}
81+
// expected-note@-2 {{add a custom 'hash(into:)' method}}
82+
}
83+
84+
extension ConditionalHashableImplementationProtocol where Self: Hashable {
85+
func hash(into hasher: Hasher) {}
86+
}
87+
88+
struct ConditionalHashableImplementation: ConditionalHashableImplementationProtocol, Hashable {
89+
let a: Int
90+
}

‎test/Sema/enum_conformance_synthesis.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ public enum Medicine {
210210
extension Medicine : Equatable {}
211211

212212
public func ==(lhs: Medicine, rhs: Medicine) -> Bool {
213+
// expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'Medicine' may not match the behavior of custom '==' operator}}
214+
// expected-note@-2 {{add a custom 'hash(into:)' method}}
213215
return true
214216
}
215217

0 commit comments

Comments
 (0)