Skip to content

[Foundation] Modernize hashing in Foundation's Swift-only types #23832

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 32 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
668c1f0
[test] StdlibUnittest: Cosmetic changes to checkEquatable/checkHashable
lorentey Feb 9, 2019
68e6449
[StdlibUnittest] checkHashable: Add opt-in support for incomplete hashes
lorentey Apr 4, 2019
46225d7
[Foundation] IndexPath: Add explicit hash(into:) definition, hashing …
lorentey Feb 9, 2019
569e380
[Foundation] NSRange: Add an explicit definition for hash(into:)
lorentey Feb 9, 2019
73ad830
[Foundation] Calendar: Modernize hashing
lorentey Feb 9, 2019
81567ed
[Foundation] CharacterSet: Modernize hashing
lorentey Apr 4, 2019
0867f7e
[Foundation] String.Encoding: Modernize hashing
lorentey Apr 4, 2019
8b7adb3
[Foundation] AffineTransform: modernize hashing
lorentey Apr 5, 2019
741122a
[Foundation] Date: Modernize hashing
lorentey Apr 5, 2019
73be6b8
[Foundation] DateComponents: Modernize hashing
lorentey Apr 5, 2019
c1049e7
[Foundation] DateInterval: Modernize hashing
lorentey Apr 5, 2019
a000422
[Foundation] Decimal: Modernize hashing
lorentey Apr 5, 2019
9c46858
[Foundation] Locale: Modernize hashing
lorentey Apr 5, 2019
2d5520e
[Foundation] IndexSet: Modernize hashing
lorentey Apr 5, 2019
5e451e6
[Foundation] IndexPath: Modernize hashing
lorentey Apr 5, 2019
4bdc458
[Foundation] Notification: Modernize hashing
lorentey Apr 5, 2019
afa2539
[Foundation] NSRange: Modernize hashing
lorentey Apr 5, 2019
b41eb97
[Foundation] TimeZone: Modernize hashing
lorentey Apr 5, 2019
10cb05a
[Foundation] URL: Add hash(into:) implementation
lorentey Apr 5, 2019
590f407
[Foundation] URLComponents: Modernize hashing
lorentey Apr 5, 2019
23cc719
[Foundation] URLRequest: Modernize hashing
lorentey Apr 5, 2019
a23c3f3
[Foundation] UUID: Modernize hashing
lorentey Apr 5, 2019
28e4688
[Foundation] PersonNameComponents: Modernize hashing
lorentey Apr 5, 2019
6f40a4a
[Foundation] Measurement: Fix hashing
lorentey Apr 5, 2019
b711ed9
[Foundation] Data: Hash the entire contents, not just an arbitrary su…
lorentey Apr 5, 2019
f859dd1
[Foundation] URL: Fix availability of the new hash(into:) implementation
lorentey Apr 6, 2019
2ab2431
[Foundation] URL is actually already Hashable
lorentey Apr 9, 2019
7f1dd3e
[Foundation] Fix bizarre indentation issues
lorentey Apr 9, 2019
582b65b
Revert "[Foundation] Data: Hash the entire contents, not just an arbi…
lorentey Apr 9, 2019
13bc567
[Foundation] Notification: Add note on == not being reflexive and sta…
lorentey Apr 12, 2019
6647c50
[test] TestNotification: Fix checkHashable invocation
lorentey Apr 24, 2019
530687f
[test] TestPersonNameComponents: adjust OS check for hashing
lorentey Apr 25, 2019
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
30 changes: 22 additions & 8 deletions stdlib/private/StdlibUnittest/StdlibUnittest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2362,17 +2362,22 @@ internal func _checkEquatableImpl<Instance : Equatable>(
let isEqualXY = x == y
expectEqual(
predictedXY, isEqualXY,
(predictedXY
? "expected equal, found not equal\n"
: "expected not equal, found equal\n") +
"lhs (at index \(i)): \(String(reflecting: x))\n" +
"rhs (at index \(j)): \(String(reflecting: y))",
"""
\((predictedXY
? "expected equal, found not equal"
: "expected not equal, found equal"))
lhs (at index \(i)): \(String(reflecting: x))
rhs (at index \(j)): \(String(reflecting: y))
""",
stackTrace: stackTrace.pushIf(showFrame, file: file, line: line))

// Not-equal is an inverse of equal.
expectNotEqual(
isEqualXY, x != y,
"lhs (at index \(i)): \(String(reflecting: x))\nrhs (at index \(j)): \(String(reflecting: y))",
"""
lhs (at index \(i)): \(String(reflecting: x))
rhs (at index \(j)): \(String(reflecting: y))
""",
stackTrace: stackTrace.pushIf(showFrame, file: file, line: line))

if !allowBrokenTransitivity {
Expand Down Expand Up @@ -2414,6 +2419,10 @@ public func checkEquatable<T : Equatable>(
showFrame: false)
}

/// Produce an integer hash value for `value` by feeding it to a dedicated
/// `Hasher`. This is always done by calling the `hash(into:)` method.
/// If a non-nil `seed` is given, it is used to perturb the hasher state;
/// this is useful for resolving accidental hash collisions.
internal func hash<H: Hashable>(_ value: H, seed: Int? = nil) -> Int {
var hasher = Hasher()
if let seed = seed {
Expand All @@ -2429,6 +2438,7 @@ internal func hash<H: Hashable>(_ value: H, seed: Int? = nil) -> Int {
public func checkHashableGroups<Groups: Collection>(
_ groups: Groups,
_ message: @autoclosure () -> String = "",
allowIncompleteHashing: Bool = false,
stackTrace: SourceLocStack = SourceLocStack(),
showFrame: Bool = true,
file: String = #file, line: UInt = #line
Expand All @@ -2446,6 +2456,7 @@ public func checkHashableGroups<Groups: Collection>(
equalityOracle: equalityOracle,
hashEqualityOracle: equalityOracle,
allowBrokenTransitivity: false,
allowIncompleteHashing: allowIncompleteHashing,
stackTrace: stackTrace.pushIf(showFrame, file: file, line: line),
showFrame: false)
}
Expand All @@ -2457,6 +2468,7 @@ public func checkHashable<Instances: Collection>(
_ instances: Instances,
equalityOracle: (Instances.Index, Instances.Index) -> Bool,
allowBrokenTransitivity: Bool = false,
allowIncompleteHashing: Bool = false,
_ message: @autoclosure () -> String = "",
stackTrace: SourceLocStack = SourceLocStack(),
showFrame: Bool = true,
Expand All @@ -2467,6 +2479,7 @@ public func checkHashable<Instances: Collection>(
equalityOracle: equalityOracle,
hashEqualityOracle: equalityOracle,
allowBrokenTransitivity: allowBrokenTransitivity,
allowIncompleteHashing: allowIncompleteHashing,
stackTrace: stackTrace.pushIf(showFrame, file: file, line: line),
showFrame: false)
}
Expand All @@ -2480,6 +2493,7 @@ public func checkHashable<Instances: Collection>(
equalityOracle: (Instances.Index, Instances.Index) -> Bool,
hashEqualityOracle: (Instances.Index, Instances.Index) -> Bool,
allowBrokenTransitivity: Bool = false,
allowIncompleteHashing: Bool = false,
_ message: @autoclosure () -> String = "",
stackTrace: SourceLocStack = SourceLocStack(),
showFrame: Bool = true,
Expand Down Expand Up @@ -2532,12 +2546,12 @@ public func checkHashable<Instances: Collection>(
expectEqual(
x._rawHashValue(seed: 0), y._rawHashValue(seed: 0),
"""
_rawHashValue expected to match, found to differ
_rawHashValue(seed:) expected to match, found to differ
lhs (at index \(i)): \(x)
rhs (at index \(j)): \(y)
""",
stackTrace: stackTrace.pushIf(showFrame, file: file, line: line))
} else {
} else if !allowIncompleteHashing {
// Try a few different seeds; at least one of them should discriminate
// between the hashes. It is extremely unlikely this check will fail
// all ten attempts, unless the type's hash encoding is not unique,
Expand Down
9 changes: 7 additions & 2 deletions stdlib/public/Darwin/Foundation/AffineTransform.swift
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,13 @@ public struct AffineTransform : ReferenceConvertible, Hashable, CustomStringConv
return newSize
}

public var hashValue : Int {
return Int(m11 + m12 + m21 + m22 + tX + tY)
public func hash(into hasher: inout Hasher) {
hasher.combine(m11)
hasher.combine(m12)
hasher.combine(m21)
hasher.combine(m22)
hasher.combine(tX)
hasher.combine(tY)
}

public var description: String {
Expand Down
11 changes: 6 additions & 5 deletions stdlib/public/Darwin/Foundation/Calendar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -899,15 +899,16 @@ public struct Calendar : Hashable, Equatable, ReferenceConvertible, _MutableBoxi

// MARK: -

public var hashValue : Int {
// We implement hash ourselves, because we need to make sure autoupdating calendars have the same hash
public func hash(into hasher: inout Hasher) {
// We need to make sure autoupdating calendars have the same hash
if _autoupdating {
return 1
hasher.combine(false)
} else {
return _handle.map { $0.hash }
hasher.combine(true)
hasher.combine(_handle.map { $0 })
}
}

// MARK: -
// MARK: Conversion Functions

Expand Down
12 changes: 6 additions & 6 deletions stdlib/public/Darwin/Foundation/CharacterSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ fileprivate final class __CharacterSetStorage : Hashable {

// MARK: -

fileprivate var hashValue : Int {
fileprivate func hash(into hasher: inout Hasher) {
switch _backing {
case .immutable(let cs):
return Int(CFHash(cs))
hasher.combine(CFHash(cs))
case .mutable(let cs):
return Int(CFHash(cs))
hasher.combine(CFHash(cs))
}
}

fileprivate static func ==(lhs : __CharacterSetStorage, rhs : __CharacterSetStorage) -> Bool {
switch (lhs._backing, rhs._backing) {
case (.immutable(let cs1), .immutable(let cs2)):
Expand Down Expand Up @@ -754,8 +754,8 @@ public struct CharacterSet : ReferenceConvertible, Equatable, Hashable, SetAlgeb

// MARK: -

public var hashValue: Int {
return _storage.hashValue
public func hash(into hasher: inout Hasher) {
hasher.combine(_storage)
}

/// Returns true if the two `CharacterSet`s are equal.
Expand Down
10 changes: 3 additions & 7 deletions stdlib/public/Darwin/Foundation/Date.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,10 @@ public struct Date : ReferenceConvertible, Comparable, Equatable {
*/
public static let distantPast = Date(timeIntervalSinceReferenceDate: -63114076800.0)

public var hashValue: Int {
if #available(macOS 10.12, iOS 10.0, *) {
return Int(bitPattern: __CFHashDouble(_time))
} else { // 10.11 and previous behavior fallback; this must allocate a date to reference the hash value and then throw away the reference
return NSDate(timeIntervalSinceReferenceDate: _time).hash
}
Copy link
Member Author

@lorentey lorentey Apr 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: There is no need to emulate NSDate's hashing behavior, or the behavior of any previous version of Date.hashValue. Swift is very explicit that the values returned by hashValue aren't to be treated as stable, and Swift trains programmers to expect randomized hashes.

Granted, in this case (like with many other hashValues removed by this PR), 5.0 did not vend properly seeded hashes, so in theory someone could be relying on the exact values returned. I believe this is still a low-risk change; and since this is not inlinable code, we have the option to re-add specific hashValue implementations with an SDK check if it turns some of them have ABI impact.

public func hash(into hasher: inout Hasher) {
hasher.combine(_time)
}

/// Compare two `Date` values.
public func compare(_ other: Date) -> ComparisonResult {
if _time < other.timeIntervalSinceReferenceDate {
Expand Down
6 changes: 3 additions & 3 deletions stdlib/public/Darwin/Foundation/DateComponents.swift
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,10 @@ public struct DateComponents : ReferenceConvertible, Hashable, Equatable, _Mutab

// MARK: -

public var hashValue : Int {
return _handle.map { $0.hash }
public func hash(into hasher: inout Hasher) {
hasher.combine(_handle._uncopiedReference())
}

// MARK: - Bridging Helpers

fileprivate init(reference: __shared NSDateComponents) {
Expand Down
12 changes: 4 additions & 8 deletions stdlib/public/Darwin/Foundation/DateInterval.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,11 @@ public struct DateInterval : ReferenceConvertible, Comparable, Hashable, Codable
return false
}

public var hashValue: Int {
var buf: (UInt, UInt) = (UInt(start.timeIntervalSinceReferenceDate), UInt(end.timeIntervalSinceReferenceDate))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the end date here was problematic, since == compares start and duration, not start and end.

end does not always include all the information that's present in duration, so doing this results in weaker hashes.

The new code is careful to match the implementation of ==.

return withUnsafeMutablePointer(to: &buf) {
$0.withMemoryRebound(to: UInt8.self, capacity: 2 * MemoryLayout<UInt>.size / MemoryLayout<UInt8>.size) {
return Int(bitPattern: CFHashBytes($0, CFIndex(MemoryLayout<UInt>.size * 2)))
}
}
public func hash(into hasher: inout Hasher) {
hasher.combine(start)
hasher.combine(duration)
}

@available(macOS 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *)
public static func ==(lhs: DateInterval, rhs: DateInterval) -> Bool {
return lhs.start == rhs.start && lhs.duration == rhs.duration
Expand Down
8 changes: 6 additions & 2 deletions stdlib/public/Darwin/Foundation/Decimal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,12 @@ extension Decimal : Hashable, Comparable {
return _isNegative != 0 ? -d : d
}

public var hashValue: Int {
return Int(bitPattern: __CFHashDouble(doubleValue))
public func hash(into hasher: inout Hasher) {
// FIXME: This is a weak hash. We should rather normalize self to a
// canonical member of the exact same equivalence relation that
// NSDecimalCompare implements, then simply feed all components to the
// hasher.
hasher.combine(doubleValue)
}

public static func ==(lhs: Decimal, rhs: Decimal) -> Bool {
Expand Down
38 changes: 22 additions & 16 deletions stdlib/public/Darwin/Foundation/IndexPath.swift
Original file line number Diff line number Diff line change
Expand Up @@ -662,25 +662,31 @@ public struct IndexPath : ReferenceConvertible, Equatable, Hashable, MutableColl
return .orderedSame
}

public var hashValue: Int {
func hashIndexes(first: Int, last: Int, count: Int) -> Int {
let totalBits = MemoryLayout<Int>.size * 8
let lengthBits = 8
let firstIndexBits = (totalBits - lengthBits) / 2
return count &+ (first << lengthBits) &+ (last << (lengthBits + firstIndexBits))
}

public func hash(into hasher: inout Hasher) {
// Note: We compare all indices in ==, so for proper hashing, we must
// also feed them all to the hasher.
//
// To ensure we have unique hash encodings in nested hashing contexts,
// we combine the count of indices as well as the indices themselves.
// (This matches what Array does.)
switch _indexes {
case .empty: return 0
case .single(let index): return index.hashValue
case .pair(let first, let second):
return hashIndexes(first: first, last: second, count: 2)
default:
let cnt = _indexes.count
return hashIndexes(first: _indexes[0], last: _indexes[cnt - 1], count: cnt)
case .empty:
hasher.combine(0)
case let .single(index):
hasher.combine(1)
hasher.combine(index)
case let .pair(first, second):
hasher.combine(2)
hasher.combine(first)
hasher.combine(second)
case let .array(indexes):
hasher.combine(indexes.count)
for index in indexes {
hasher.combine(index)
}
}
}

// MARK: - Bridging Helpers

fileprivate init(nsIndexPath: __shared ReferenceType) {
Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/Darwin/Foundation/IndexSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ public struct IndexSet : ReferenceConvertible, Equatable, BidirectionalCollectio
_handle = _MutablePairHandle(NSIndexSet(), copying: false)
}

public var hashValue: Int {
return _handle.map { $0.hash }
public func hash(into hasher: inout Hasher) {
_handle.map { hasher.combine($0) }
}

/// Returns the number of integers in `self`.
Expand Down
7 changes: 4 additions & 3 deletions stdlib/public/Darwin/Foundation/Locale.swift
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,12 @@ public struct Locale : Hashable, Equatable, ReferenceConvertible {
// MARK: -
//

public var hashValue : Int {
public func hash(into hasher: inout Hasher) {
if _autoupdating {
return 1
hasher.combine(false)
} else {
return _wrapped.hash
hasher.combine(true)
hasher.combine(_wrapped)
}
}

Expand Down
19 changes: 17 additions & 2 deletions stdlib/public/Darwin/Foundation/Measurement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,19 @@ public struct Measurement<UnitType : Unit> : ReferenceConvertible, Comparable, E
self.unit = unit
}

public var hashValue: Int {
return Int(bitPattern: __CFHashDouble(value))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original implementation of Measurement.hashValue is not a valid hash, because it is in conflict with ==.

(A version of this change was originally submitted as #20879.)

public func hash(into hasher: inout Hasher) {
// Warning: The canonicalization performed here needs to be kept in
// perfect sync with the definition of == below. The floating point
// values that are compared there must match exactly with the values fed
// to the hasher here, or hashing would break.
if let dimension = unit as? Dimension {
// We don't need to feed the base unit to the hasher here; all
// dimensional measurements of the same type share the same unit.
hasher.combine(dimension.converter.baseUnitValue(fromValue: value))
} else {
hasher.combine(unit)
hasher.combine(value)
}
}
}

Expand Down Expand Up @@ -170,6 +181,10 @@ extension Measurement {
/// If `lhs.unit == rhs.unit`, returns `lhs.value == rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values.
/// - returns: `true` if the measurements are equal.
public static func ==<LeftHandSideType, RightHandSideType>(lhs: Measurement<LeftHandSideType>, rhs: Measurement<RightHandSideType>) -> Bool {
// Warning: This defines an equivalence relation that needs to be kept
// in perfect sync with the hash(into:) definition above. The floating
// point values that are fed to the hasher there must match exactly with
// the values compared here, or hashing would break.
if lhs.unit == rhs.unit {
return lhs.value == rhs.value
} else {
Expand Down
11 changes: 4 additions & 7 deletions stdlib/public/Darwin/Foundation/NSRange.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@
@_exported import Foundation // Clang module

extension NSRange : Hashable {
public var hashValue: Int {
#if arch(i386) || arch(arm)
return Int(bitPattern: (UInt(bitPattern: location) | (UInt(bitPattern: length) << 16)))
#elseif arch(x86_64) || arch(arm64)
return Int(bitPattern: (UInt(bitPattern: location) | (UInt(bitPattern: length) << 32)))
#endif
public func hash(into hasher: inout Hasher) {
hasher.combine(location)
hasher.combine(length)
}

public static func==(lhs: NSRange, rhs: NSRange) -> Bool {
Expand Down Expand Up @@ -230,4 +227,4 @@ extension NSRange : Codable {
try container.encode(self.location)
try container.encode(self.length)
}
}
}
4 changes: 2 additions & 2 deletions stdlib/public/Darwin/Foundation/NSStringEncodings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ extension String {
}

extension String.Encoding : Hashable {
public var hashValue : Int {
return rawValue.hashValue
public func hash(into hasher: inout Hasher) {
hasher.combine(rawValue)
}

public static func ==(lhs: String.Encoding, rhs: String.Encoding) -> Bool {
Expand Down
Loading