Skip to content

Commit 50b26ff

Browse files
lorenteylorentey
authored
Merge pull request swiftlang#1656 from lorentey/hashing-improvements
DateComponents, URLRequest, URLComponents: Fix hashing
2 parents 4c95608 + 72a0cbe commit 50b26ff

File tree

12 files changed

+579
-42
lines changed

12 files changed

+579
-42
lines changed

‎Foundation.xcodeproj/project.pbxproj

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,8 @@
324324
6EB768281D18C12C00D4B719 /* UUID.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6EB768271D18C12C00D4B719 /* UUID.swift */; };
325325
7900433B1CACD33E00ECCBF1 /* TestNSCompoundPredicate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 790043391CACD33E00ECCBF1 /* TestNSCompoundPredicate.swift */; };
326326
7900433C1CACD33E00ECCBF1 /* TestNSPredicate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7900433A1CACD33E00ECCBF1 /* TestNSPredicate.swift */; };
327+
7D0DE86E211883F500540061 /* TestDateComponents.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7D0DE86C211883F500540061 /* TestDateComponents.swift */; };
328+
7D0DE86F211883F500540061 /* Utilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7D0DE86D211883F500540061 /* Utilities.swift */; };
327329
90E645DF1E4C89A400D0D47C /* TestNSCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 90E645DE1E4C89A400D0D47C /* TestNSCache.swift */; };
328330
9F0DD3521ECD73D000F68030 /* main.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F0041781ECD5962004138BD /* main.swift */; };
329331
9F0DD3571ECD783500F68030 /* SwiftFoundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5B5D885D1BBC938800234F36 /* SwiftFoundation.framework */; };
@@ -815,6 +817,8 @@
815817
790043391CACD33E00ECCBF1 /* TestNSCompoundPredicate.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestNSCompoundPredicate.swift; sourceTree = "<group>"; };
816818
7900433A1CACD33E00ECCBF1 /* TestNSPredicate.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestNSPredicate.swift; sourceTree = "<group>"; };
817819
7A7D6FBA1C16439400957E2E /* TestURLResponse.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestURLResponse.swift; sourceTree = "<group>"; };
820+
7D0DE86C211883F500540061 /* TestDateComponents.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestDateComponents.swift; sourceTree = "<group>"; };
821+
7D0DE86D211883F500540061 /* Utilities.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Utilities.swift; sourceTree = "<group>"; };
818822
83712C8D1C1684900049AD49 /* TestNSURLRequest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestNSURLRequest.swift; sourceTree = "<group>"; };
819823
844DC3321C17584F005611F9 /* TestScanner.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestScanner.swift; sourceTree = "<group>"; };
820824
848A30571C137B3500C83206 /* TestHTTPCookie.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = TestHTTPCookie.swift; path = TestFoundation/TestHTTPCookie.swift; sourceTree = SOURCE_ROOT; };
@@ -1489,6 +1493,8 @@
14891493
EA66F65A1BF1976100136161 /* Tests */ = {
14901494
isa = PBXGroup;
14911495
children = (
1496+
7D0DE86D211883F500540061 /* Utilities.swift */,
1497+
7D0DE86C211883F500540061 /* TestDateComponents.swift */,
14921498
6E203B8C1C1303BB003B2576 /* TestBundle.swift */,
14931499
A5A34B551C18C85D00FD972B /* TestByteCountFormatter.swift */,
14941500
2EBE67A31C77BF05006583D5 /* TestDateFormatter.swift */,
@@ -2521,6 +2527,7 @@
25212527
D5C40F331CDA1D460005690C /* TestOperationQueue.swift in Sources */,
25222528
BDBB65901E256BFA001A7286 /* TestEnergyFormatter.swift in Sources */,
25232529
5B13B32F1C582D4C00651CE2 /* TestNSGeometry.swift in Sources */,
2530+
7D0DE86E211883F500540061 /* TestDateComponents.swift in Sources */,
25242531
EA08126C1DA810BE00651B70 /* ProgressFraction.swift in Sources */,
25252532
5B13B3351C582D4C00651CE2 /* TestNSKeyedUnarchiver.swift in Sources */,
25262533
5B13B33D1C582D4C00651CE2 /* TestPipe.swift in Sources */,
@@ -2547,6 +2554,7 @@
25472554
B951B5EC1F4E2A2000D8B332 /* TestNSLock.swift in Sources */,
25482555
5B13B33A1C582D4C00651CE2 /* TestNSNumber.swift in Sources */,
25492556
5B13B3521C582D4C00651CE2 /* TestNSValue.swift in Sources */,
2557+
7D0DE86F211883F500540061 /* Utilities.swift in Sources */,
25502558
5B13B3311C582D4C00651CE2 /* TestIndexPath.swift in Sources */,
25512559
5B13B3271C582D4C00651CE2 /* TestNSArray.swift in Sources */,
25522560
5B13B3461C582D4C00651CE2 /* TestProcess.swift in Sources */,

‎Foundation/NSCalendar.swift

Lines changed: 84 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,49 +1297,94 @@ open class NSDateComponents : NSObject, NSCopying, NSSecureCoding {
12971297
public override init() {
12981298
super.init()
12991299
}
1300-
1300+
13011301
open override var hash: Int {
1302-
var calHash = 0
1303-
if let cal = calendar {
1304-
calHash = cal.hashValue
1305-
}
1306-
if let tz = timeZone {
1307-
calHash ^= tz.hashValue
1308-
}
1309-
var y = year
1310-
if NSDateComponentUndefined == y {
1311-
y = 0
1312-
}
1313-
var m = month
1314-
if NSDateComponentUndefined == m {
1315-
m = 0
1316-
}
1317-
var d = day
1318-
if NSDateComponentUndefined == d {
1319-
d = 0
1320-
}
1321-
var h = hour
1322-
if NSDateComponentUndefined == h {
1323-
h = 0
1324-
}
1325-
var mm = minute
1326-
if NSDateComponentUndefined == mm {
1327-
mm = 0
1328-
}
1329-
var s = second
1330-
if NSDateComponentUndefined == s {
1331-
s = 0
1332-
}
1333-
var yy = yearForWeekOfYear
1334-
if NSDateComponentUndefined == yy {
1335-
yy = 0
1336-
}
1337-
return calHash + (32832013 * (y + yy) + 2678437 * m + 86413 * d + 3607 * h + 61 * mm + s) + (41 * weekOfYear + 11 * weekOfMonth + 7 * weekday + 3 * weekdayOrdinal + quarter) * (1 << 5)
1302+
var hasher = Hasher()
1303+
var mask = 0
1304+
// The list of fields fed to the hasher here must be exactly
1305+
// the same as the ones compared in isEqual(_:) (modulo
1306+
// ordering).
1307+
//
1308+
// Given that NSDateComponents instances usually only have a
1309+
// few fields present, it makes sense to only hash those, as
1310+
// an optimization. We keep track of the fields hashed in the
1311+
// mask value, which we also feed to the hasher to make sure
1312+
// any two unequal values produce different hash encodings.
1313+
//
1314+
// FIXME: Why not just feed _values, calendar & timeZone to
1315+
// the hasher?
1316+
if let calendar = calendar {
1317+
hasher.combine(calendar)
1318+
mask |= 1 << 0
1319+
}
1320+
if let timeZone = timeZone {
1321+
hasher.combine(timeZone)
1322+
mask |= 1 << 1
1323+
}
1324+
if era != NSDateComponentUndefined {
1325+
hasher.combine(era)
1326+
mask |= 1 << 2
1327+
}
1328+
if year != NSDateComponentUndefined {
1329+
hasher.combine(year)
1330+
mask |= 1 << 3
1331+
}
1332+
if quarter != NSDateComponentUndefined {
1333+
hasher.combine(quarter)
1334+
mask |= 1 << 4
1335+
}
1336+
if month != NSDateComponentUndefined {
1337+
hasher.combine(month)
1338+
mask |= 1 << 5
1339+
}
1340+
if day != NSDateComponentUndefined {
1341+
hasher.combine(day)
1342+
mask |= 1 << 6
1343+
}
1344+
if hour != NSDateComponentUndefined {
1345+
hasher.combine(hour)
1346+
mask |= 1 << 7
1347+
}
1348+
if minute != NSDateComponentUndefined {
1349+
hasher.combine(minute)
1350+
mask |= 1 << 8
1351+
}
1352+
if second != NSDateComponentUndefined {
1353+
hasher.combine(second)
1354+
mask |= 1 << 9
1355+
}
1356+
if nanosecond != NSDateComponentUndefined {
1357+
hasher.combine(nanosecond)
1358+
mask |= 1 << 10
1359+
}
1360+
if weekOfYear != NSDateComponentUndefined {
1361+
hasher.combine(weekOfYear)
1362+
mask |= 1 << 11
1363+
}
1364+
if weekOfMonth != NSDateComponentUndefined {
1365+
hasher.combine(weekOfMonth)
1366+
mask |= 1 << 12
1367+
}
1368+
if yearForWeekOfYear != NSDateComponentUndefined {
1369+
hasher.combine(yearForWeekOfYear)
1370+
mask |= 1 << 13
1371+
}
1372+
if weekday != NSDateComponentUndefined {
1373+
hasher.combine(weekday)
1374+
mask |= 1 << 14
1375+
}
1376+
if weekdayOrdinal != NSDateComponentUndefined {
1377+
hasher.combine(weekdayOrdinal)
1378+
mask |= 1 << 15
1379+
}
1380+
hasher.combine(isLeapMonth)
1381+
hasher.combine(mask)
1382+
return hasher.finalize()
13381383
}
1339-
1384+
13401385
open override func isEqual(_ object: Any?) -> Bool {
13411386
guard let other = object as? NSDateComponents else { return false }
1342-
1387+
// FIXME: Why not just compare _values, calendar & timeZone?
13431388
return self === other
13441389
|| (era == other.era
13451390
&& year == other.year

‎Foundation/NSURL.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -983,6 +983,19 @@ open class NSURLComponents: NSObject, NSCopying {
983983
&& fragment == other.fragment)
984984
}
985985

986+
open override var hash: Int {
987+
var hasher = Hasher()
988+
hasher.combine(scheme)
989+
hasher.combine(user)
990+
hasher.combine(password)
991+
hasher.combine(host)
992+
hasher.combine(port)
993+
hasher.combine(path)
994+
hasher.combine(query)
995+
hasher.combine(fragment)
996+
return hasher.finalize()
997+
}
998+
986999
open func copy(with zone: NSZone? = nil) -> Any {
9871000
let copy = NSURLComponents()
9881001
copy.scheme = self.scheme

‎Foundation/NSURLRequest.swift

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,18 @@ open class NSURLRequest : NSObject, NSSecureCoding, NSCopying, NSMutableCopying
259259
&& other.allowsCellularAccess == self.allowsCellularAccess
260260
&& other.httpShouldHandleCookies == self.httpShouldHandleCookies)
261261
}
262-
262+
263+
open override var hash: Int {
264+
var hasher = Hasher()
265+
hasher.combine(url)
266+
hasher.combine(mainDocumentURL)
267+
hasher.combine(httpMethod)
268+
hasher.combine(httpBodyStream)
269+
hasher.combine(allowsCellularAccess)
270+
hasher.combine(httpShouldHandleCookies)
271+
return hasher.finalize()
272+
}
273+
263274
/// Indicates that NSURLRequest implements the NSSecureCoding protocol.
264275
open class var supportsSecureCoding: Bool { return true }
265276

‎TestFoundation/TestCalendar.swift

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,104 @@ class TestNSDateComponents: XCTestCase {
198198

199199
static var allTests: [(String, (TestNSDateComponents) -> () throws -> Void)] {
200200
return [
201+
("test_hash", test_hash),
201202
("test_copyNSDateComponents", test_copyNSDateComponents),
202203
("test_dateDifferenceComponents", test_dateDifferenceComponents),
203204
]
204205
}
205206

207+
func test_hash() {
208+
let c1 = NSDateComponents()
209+
c1.year = 2018
210+
c1.month = 8
211+
c1.day = 1
212+
213+
let c2 = NSDateComponents()
214+
c2.year = 2018
215+
c2.month = 8
216+
c2.day = 1
217+
218+
XCTAssertEqual(c1, c2)
219+
XCTAssertEqual(c1.hash, c2.hash)
220+
221+
checkHashing_NSCopying(
222+
initialValue: NSDateComponents(),
223+
byMutating: \NSDateComponents.calendar,
224+
throughValues: [
225+
Calendar(identifier: .gregorian),
226+
Calendar(identifier: .buddhist),
227+
Calendar(identifier: .chinese),
228+
Calendar(identifier: .coptic),
229+
Calendar(identifier: .hebrew),
230+
Calendar(identifier: .indian),
231+
Calendar(identifier: .islamic),
232+
Calendar(identifier: .iso8601),
233+
Calendar(identifier: .japanese),
234+
Calendar(identifier: .persian)])
235+
checkHashing_NSCopying(
236+
initialValue: NSDateComponents(),
237+
byMutating: \NSDateComponents.timeZone,
238+
throughValues: (-10...10).map { TimeZone(secondsFromGMT: 3600 * $0) })
239+
// Note: These assume components aren't range checked.
240+
checkHashing_NSCopying(
241+
initialValue: NSDateComponents(),
242+
byMutating: \NSDateComponents.era,
243+
throughValues: 0...20)
244+
checkHashing_NSCopying(
245+
initialValue: NSDateComponents(),
246+
byMutating: \NSDateComponents.year,
247+
throughValues: 0...20)
248+
checkHashing_NSCopying(
249+
initialValue: NSDateComponents(),
250+
byMutating: \NSDateComponents.quarter,
251+
throughValues: 0...20)
252+
checkHashing_NSCopying(
253+
initialValue: NSDateComponents(),
254+
byMutating: \NSDateComponents.month,
255+
throughValues: 0...20)
256+
checkHashing_NSCopying(
257+
initialValue: NSDateComponents(),
258+
byMutating: \NSDateComponents.day,
259+
throughValues: 0...20)
260+
checkHashing_NSCopying(
261+
initialValue: NSDateComponents(),
262+
byMutating: \NSDateComponents.hour,
263+
throughValues: 0...20)
264+
checkHashing_NSCopying(
265+
initialValue: NSDateComponents(),
266+
byMutating: \NSDateComponents.minute,
267+
throughValues: 0...20)
268+
checkHashing_NSCopying(
269+
initialValue: NSDateComponents(),
270+
byMutating: \NSDateComponents.second,
271+
throughValues: 0...20)
272+
checkHashing_NSCopying(
273+
initialValue: NSDateComponents(),
274+
byMutating: \NSDateComponents.nanosecond,
275+
throughValues: 0...20)
276+
checkHashing_NSCopying(
277+
initialValue: NSDateComponents(),
278+
byMutating: \NSDateComponents.weekOfYear,
279+
throughValues: 0...20)
280+
checkHashing_NSCopying(
281+
initialValue: NSDateComponents(),
282+
byMutating: \NSDateComponents.weekOfMonth,
283+
throughValues: 0...20)
284+
checkHashing_NSCopying(
285+
initialValue: NSDateComponents(),
286+
byMutating: \NSDateComponents.yearForWeekOfYear,
287+
throughValues: 0...20)
288+
checkHashing_NSCopying(
289+
initialValue: NSDateComponents(),
290+
byMutating: \NSDateComponents.weekday,
291+
throughValues: 0...20)
292+
checkHashing_NSCopying(
293+
initialValue: NSDateComponents(),
294+
byMutating: \NSDateComponents.weekdayOrdinal,
295+
throughValues: 0...20)
296+
// isLeapMonth does not have enough values to test it here.
297+
}
298+
206299
func test_copyNSDateComponents() {
207300
let components = NSDateComponents()
208301
components.year = 1987

0 commit comments

Comments
 (0)