Skip to content

Commit 13ad81a

Browse files
allevatoallevato
committed
Synthed hashValue now calls _mixForSynthesizedHashValue
For enums, we now only call _mixInt on the computed hash value if it has associated values; for enums without associated values, we use the ordinal alone, as before.
1 parent 3618164 commit 13ad81a

File tree

3 files changed

+101
-91
lines changed

3 files changed

+101
-91
lines changed

‎include/swift/AST/ASTContext.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,12 +470,13 @@ class ASTContext {
470470
/// Retrieve the declaration of Swift.==(Int, Int) -> Bool.
471471
FuncDecl *getEqualIntDecl() const;
472472

473-
/// Retrieve the declaration of Swift.^= (inout Int, Int) -> Void.
474-
FuncDecl *getMutatingXorIntDecl() const;
473+
/// Retrieve the declaration of
474+
/// Swift._mixForSynthesizedHashValue (Int, Int) -> Int.
475+
FuncDecl *getMixForSynthesizedHashValueDecl() const;
475476

476477
/// Retrieve the declaration of Swift._mixInt(Int) -> Int.
477478
FuncDecl *getMixIntDecl() const;
478-
479+
479480
/// Retrieve the declaration of Array.append(element:)
480481
FuncDecl *getArrayAppendElementDecl() const;
481482

‎lib/AST/ASTContext.cpp

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ FOR_KNOWN_FOUNDATION_TYPES(CACHE_FOUNDATION_DECL)
184184
/// func ==(Int, Int) -> Bool
185185
FuncDecl *EqualIntDecl = nullptr;
186186

187-
/// func ^= (inout Int, Int) -> Void
188-
FuncDecl *MutatingXorIntDecl = nullptr;
187+
/// func _mixForSynthesizedHashValue(Int, Int) -> Int
188+
FuncDecl *MixForSynthesizedHashValueDecl = nullptr;
189189

190190
/// func _mixInt(Int) -> Int
191191
FuncDecl *MixIntDecl = nullptr;
@@ -1028,27 +1028,6 @@ FuncDecl *ASTContext::getEqualIntDecl() const {
10281028
return decl;
10291029
}
10301030

1031-
FuncDecl *ASTContext::getMutatingXorIntDecl() const {
1032-
if (Impl.MutatingXorIntDecl)
1033-
return Impl.MutatingXorIntDecl;
1034-
1035-
auto intType = getIntDecl()->getDeclaredType();
1036-
auto inoutIntType = InOutType::get(intType);
1037-
auto callback = [&](Type inputType, Type resultType) {
1038-
// Check for the signature: (inout Int, Int) -> Void
1039-
auto tupleType = dyn_cast<TupleType>(inputType.getPointer());
1040-
assert(tupleType);
1041-
return (tupleType->getNumElements() == 2 &&
1042-
tupleType->getElementType(0)->isEqual(inoutIntType) &&
1043-
tupleType->getElementType(1)->isEqual(intType) &&
1044-
resultType->isVoid());
1045-
};
1046-
1047-
auto decl = lookupOperatorFunc<16>(*this, "^=", intType, callback);
1048-
Impl.MutatingXorIntDecl = decl;
1049-
return decl;
1050-
}
1051-
10521031
FuncDecl *ASTContext::getGetBoolDecl(LazyResolver *resolver) const {
10531032
if (Impl.GetBoolDecl)
10541033
return Impl.GetBoolDecl;
@@ -1064,6 +1043,29 @@ FuncDecl *ASTContext::getGetBoolDecl(LazyResolver *resolver) const {
10641043
return decl;
10651044
}
10661045

1046+
FuncDecl *ASTContext::getMixForSynthesizedHashValueDecl() const {
1047+
if (Impl.MixForSynthesizedHashValueDecl)
1048+
return Impl.MixForSynthesizedHashValueDecl;
1049+
1050+
auto resolver = getLazyResolver();
1051+
auto intType = getIntDecl()->getDeclaredType();
1052+
1053+
auto callback = [&](Type inputType, Type resultType) {
1054+
// Look for the signature (Int, Int) -> Int
1055+
auto tupleType = dyn_cast<TupleType>(inputType.getPointer());
1056+
assert(tupleType);
1057+
return tupleType->getNumElements() == 2 &&
1058+
tupleType->getElementType(0)->isEqual(intType) &&
1059+
tupleType->getElementType(1)->isEqual(intType) &&
1060+
resultType->isEqual(intType);
1061+
};
1062+
1063+
auto decl = lookupLibraryIntrinsicFunc(
1064+
*this, "_mixForSynthesizedHashValue", resolver, callback);
1065+
Impl.MixForSynthesizedHashValueDecl = decl;
1066+
return decl;
1067+
}
1068+
10671069
FuncDecl *ASTContext::getMixIntDecl() const {
10681070
if (Impl.MixIntDecl)
10691071
return Impl.MixIntDecl;

‎lib/Sema/DerivedConformanceEquatableHashable.cpp

Lines changed: 72 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -745,51 +745,58 @@ static Expr* integerLiteralExpr(ASTContext &C, int64_t value) {
745745
return integerExpr;
746746
}
747747

748-
/// Returns a new expression that mixes the hash value of an expression into a
749-
/// variable (as a mutating assignment).
748+
/// Returns a new assignment expression that mixes the hash value of an
749+
/// expression into a variable.
750750
/// \p C The AST context.
751751
/// \p resultVar The variable into which the hash value will be mixed.
752752
/// \p exprToHash The expression whose hash value should be mixed in.
753753
/// \return The expression that mixes the hash value into the result variable.
754754
static Expr* mixInHashExpr_hashValue(ASTContext &C,
755755
VarDecl* resultVar,
756756
Expr *exprToHash) {
757-
auto intType = C.getIntDecl()->getDeclaredType();
758-
auto xorFuncInputType =
759-
TupleType::get({
760-
TupleTypeElt(intType, Identifier(), ParameterTypeFlags().withInOut(true)),
761-
TupleTypeElt(intType) }, C);
762-
763757
// <exprToHash>.hashValue
764758
auto hashValueExpr = new (C) UnresolvedDotExpr(exprToHash, SourceLoc(),
765759
C.Id_hashValue, DeclNameLoc(),
766760
/*implicit*/ true);
767761

768-
// _mixInt(<exprToHash>.hashValue)
769-
auto mixinFunc = C.getMixIntDecl();
762+
// _mixForSynthesizedHashValue(result, <exprToHash>.hashValue)
763+
auto mixinFunc = C.getMixForSynthesizedHashValueDecl();
770764
auto mixinFuncExpr = new (C) DeclRefExpr(mixinFunc, DeclNameLoc(),
771765
/*implicit*/ true);
772-
auto mixinResultExpr = CallExpr::createImplicit(C, mixinFuncExpr,
773-
{ hashValueExpr }, {});
774-
775-
// result ^= _mixInt(<exprToHash>.hashValue)
776-
auto resultExpr = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
777-
/*implicit*/ true);
778-
auto resultInoutExpr = new (C) InOutExpr(SourceLoc(), resultExpr,
779-
intType, /*implicit*/ true);
766+
auto rhsResultExpr = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
767+
/*implicit*/ true);
768+
auto mixinResultExpr = CallExpr::createImplicit(
769+
C, mixinFuncExpr, { rhsResultExpr, hashValueExpr }, {});
780770

781-
auto xorFunc = C.getMutatingXorIntDecl();
782-
auto xorFuncExpr = new (C) DeclRefExpr(xorFunc, DeclNameLoc(),
783-
/*implicit*/ true);
771+
// result = _mixForSynthesizedHashValue(result, <exprToHash>.hashValue)
772+
auto lhsResultExpr = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
773+
/*implicit*/ true);
774+
auto assignExpr = new (C) AssignExpr(lhsResultExpr, SourceLoc(),
775+
mixinResultExpr, /*implicit*/ true);
776+
return assignExpr;
777+
}
784778

785-
TupleExpr *xorArgTuple = TupleExpr::create(C, SourceLoc(),
786-
{ resultInoutExpr,
787-
mixinResultExpr },
788-
{ }, { }, SourceLoc(),
789-
/*HasTrailingClosure*/ false,
790-
/*Implicit*/ true,
791-
xorFuncInputType);
792-
return new (C) BinaryExpr(xorFuncExpr, xorArgTuple, /*implicit*/ true);
779+
/// Returns a new assignment expression that invokes _mixInt on a variable and
780+
/// assigns the result back to the same variable.
781+
/// \p C The AST context.
782+
/// \p resultVar The integer variable to be mixed.
783+
/// \return The expression that mixes the integer value.
784+
static Expr* mixIntAssignmentExpr(ASTContext &C, VarDecl* resultVar) {
785+
auto mixinFunc = C.getMixIntDecl();
786+
auto mixinFuncExpr = new (C) DeclRefExpr(mixinFunc, DeclNameLoc(),
787+
/*implicit*/ true);
788+
auto rhsResultRef = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
789+
/*implicit*/ true);
790+
// _mixInt(result)
791+
auto mixedResultExpr = CallExpr::createImplicit(C, mixinFuncExpr,
792+
{ rhsResultRef }, {});
793+
794+
// result = _mixInt(result)
795+
auto lhsResultRef = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
796+
/*implicit*/ true);
797+
auto assignExpr = new (C) AssignExpr(lhsResultRef, SourceLoc(),
798+
mixedResultExpr, /*implicit*/ true);
799+
return assignExpr;
793800
}
794801

795802
static void
@@ -844,45 +851,39 @@ deriveBodyHashable_enum_hashValue(AbstractFunctionDecl *hashValueDecl) {
844851
auto labelItem = CaseLabelItem(/*IsDefault*/ false, pat, SourceLoc(),
845852
nullptr);
846853

847-
auto hasBoundDecls = !payloadVars.empty();
854+
// If the enum has no associated values, we use the ordinal alone as the
855+
// hash value, because that is sufficient for a good distribution. If any
856+
// case do have associated values, then the ordinal is used as the first
857+
// term mixed into _mixForSynthesizedHashValue, and the final result after
858+
// mixing in the payload is passed to _mixInt to improve the distribution.
848859

849-
// If the enum has no cases with associated values, we use the ordinal
850-
// directly as the hash value. If any of the cases have associated values,
851-
// then we mix the ordinal instead to better distribute it among the hash
852-
// values of its payload.
853-
if (hasNoAssociatedValues) {
854-
// result = <ordinal>
860+
// result = <ordinal>
861+
{
855862
auto ordinalExpr = integerLiteralExpr(C, index++);
856863
auto resultRef = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
857864
/*implicit*/ true);
858865
auto assignExpr = new (C) AssignExpr(resultRef, SourceLoc(),
859866
ordinalExpr, /*implicit*/ true);
860867
mixExpressions.emplace_back(ASTNode(assignExpr));
861-
} else {
862-
// result = _mixInt(<ordinal>)
863-
auto mixinFunc = C.getMixIntDecl();
864-
auto mixinFuncExpr = new (C) DeclRefExpr(mixinFunc, DeclNameLoc(),
865-
/*implicit*/ true);
866-
auto ordinalExpr = integerLiteralExpr(C, index++);
867-
auto mixedOrdinalExpr = CallExpr::createImplicit(C, mixinFuncExpr,
868-
{ ordinalExpr }, {});
869-
auto resultRef = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
870-
/*implicit*/ true);
871-
auto assignExpr = new (C) AssignExpr(resultRef, SourceLoc(),
872-
mixedOrdinalExpr, /*implicit*/ true);
873-
mixExpressions.emplace_back(ASTNode(assignExpr));
868+
}
874869

870+
if (!hasNoAssociatedValues) {
875871
// Generate a sequence of expressions that mix the payload's hash values
876872
// into result.
877873
for (auto payloadVar : payloadVars) {
878874
auto payloadVarRef = new (C) DeclRefExpr(payloadVar, DeclNameLoc(),
879875
/*implicit*/ true);
880-
// result ^= <payloadVar>.hashValue
876+
// result = _mixForSynthesizedHashValue(result, <payloadVar>.hashValue)
881877
auto mixExpr = mixInHashExpr_hashValue(C, resultVar, payloadVarRef);
882878
mixExpressions.emplace_back(ASTNode(mixExpr));
883879
}
880+
881+
// result = _mixInt(result)
882+
auto assignExpr = mixIntAssignmentExpr(C, resultVar);
883+
mixExpressions.emplace_back(ASTNode(assignExpr));
884884
}
885885

886+
auto hasBoundDecls = !payloadVars.empty();
886887
auto body = BraceStmt::create(C, SourceLoc(), mixExpressions, SourceLoc());
887888
cases.push_back(CaseStmt::create(C, SourceLoc(), labelItem, hasBoundDecls,
888889
SourceLoc(), body));
@@ -961,13 +962,17 @@ deriveBodyHashable_struct_hashValue(AbstractFunctionDecl *hashValueDecl) {
961962
/*implicit*/ true);
962963
auto selfPropertyExpr = new (C) DotSyntaxCallExpr(propertyRef, SourceLoc(),
963964
selfRef);
964-
// result ^= <property>.hashValue
965+
// result = _mixForSynthesizedHashValue(result, <property>.hashValue)
965966
auto mixExpr = mixInHashExpr_hashValue(C, resultVar, selfPropertyExpr);
966967
statements.emplace_back(ASTNode(mixExpr));
967968
}
968969

969-
// return result
970970
{
971+
// result = _mixInt(result)
972+
auto assignExpr = mixIntAssignmentExpr(C, resultVar);
973+
statements.push_back(assignExpr);
974+
975+
// return result
971976
auto resultRef = new (C) DeclRefExpr(resultVar, DeclNameLoc(),
972977
/*implicit*/ true,
973978
AccessSemantics::Ordinary, intType);
@@ -987,16 +992,16 @@ deriveHashable_hashValue(TypeChecker &tc, Decl *parentDecl,
987992
// enum SomeEnum {
988993
// case A, B, C
989994
// @derived var hashValue: Int {
990-
// var index: Int
995+
// var result: Int
991996
// switch self {
992997
// case A:
993-
// index = 0
998+
// result = 0
994999
// case B:
995-
// index = 1
1000+
// result = 1
9961001
// case C:
997-
// index = 2
1002+
// result = 2
9981003
// }
999-
// return index.hashValue
1004+
// return result
10001005
// }
10011006
// }
10021007
//
@@ -1006,15 +1011,16 @@ deriveHashable_hashValue(TypeChecker &tc, Decl *parentDecl,
10061011
// var result: Int
10071012
// switch self {
10081013
// case A:
1009-
// result = _mixInt(0)
1014+
// result = 0
10101015
// case B(let a0):
1011-
// result = _mixInt(1)
1012-
// result ^= _mixInt(a0.hashValue)
1016+
// result = _mixForSynthesizedHashValue(result, 1)
1017+
// result = _mixForSynthesizedHashValue(result, a0.hashValue)
10131018
// case C(let a0, let a1):
1014-
// result = _mixInt(2)
1015-
// result ^= _mixInt(a0.hashValue)
1016-
// result ^= _mixInt(a1.hashValue)
1019+
// result = _mixForSynthesizedHashValue(result, 2)
1020+
// result = _mixForSynthesizedHashValue(result, a0.hashValue)
1021+
// result = _mixForSynthesizedHashValue(result, a1.hashValue)
10171022
// }
1023+
// result = _mixInt(result)
10181024
// return result
10191025
// }
10201026
// }
@@ -1024,8 +1030,9 @@ deriveHashable_hashValue(TypeChecker &tc, Decl *parentDecl,
10241030
// var y: String
10251031
// @derived var hashValue: Int {
10261032
// var result: Int = 0
1027-
// result ^= _mixInt(x.hashValue)
1028-
// result ^= _mixInt(y.hashValue)
1033+
// result = _mixForSynthesizedHashValue(result, x.hashValue)
1034+
// result = _mixForSynthesizedHashValue(result, y.hashValue)
1035+
// result = _mixInt(result)
10291036
// return result
10301037
// }
10311038
// }

0 commit comments

Comments
 (0)