Merge pull request #4887 from eeckstein/dict-set

stdlib: some small improvements for Dictionary and Set

Author: eeckstein

stdlib/public/core/HashedCollections.swift.gyb

@@ -2523,7 +2523,7 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> {
 
   @_versioned
   internal var _capacity: Int {
-    return _body.capacity
+    return _assumeNonNegative(_body.capacity)
   }
 
   @_versioned
@@ -2532,7 +2532,7 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> {
       _body.count = newValue
     }
     get {
-      return _body.count
+      return _assumeNonNegative(_body.count)
     }
   }
 
@@ -2703,7 +2703,7 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
   @_versioned
   @inline(__always)
   internal func key(at i: Int) -> Key {
-    _precondition(i >= 0 && i < capacity)
+    _sanityCheck(i >= 0 && i < capacity)
     _sanityCheck(isInitializedEntry(at: i))
 
     let res = (keys + i).pointee
@@ -2713,7 +2713,7 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
 
   @_versioned
   internal func isInitializedEntry(at i: Int) -> Bool {
-    _precondition(i >= 0 && i < capacity)
+    _sanityCheck(i >= 0 && i < capacity)
     return initializedEntries[i]
   }
 
@@ -2747,7 +2747,7 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
   }
 
   internal func setKey(_ key: Key, at i: Int) {
-    _precondition(i >= 0 && i < capacity)
+    _sanityCheck(i >= 0 && i < capacity)
     _sanityCheck(isInitializedEntry(at: i))
 
     (keys + i).pointee = key
@@ -2804,8 +2804,9 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
   }
 
   @_versioned
+  @inline(__always) // For performance reasons.
   internal func _bucket(_ k: Key) -> Int {
-    return _squeezeHashValue(k.hashValue, 0..<capacity)
+    return _squeezeHashValue(k.hashValue, capacity)
   }
 
   @_versioned
@@ -2933,6 +2934,7 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
   }
 
   internal func assertingGet(_ i: Index) -> SequenceElement {
+    _precondition(i.offset >= 0 && i.offset < capacity)
     _precondition(
       isInitializedEntry(at: i.offset),
       "attempting to access ${Self} elements using an invalid Index")

stdlib/public/core/Hashing.swift

@@ -147,51 +147,31 @@ func _mixInt(_ value: Int) -> Int {
 #endif
 }
 
-/// Given a hash value, returns an integer value within the given range that
-/// corresponds to a hash value.
+/// Given a hash value, returns an integer value in the range of
+/// 0..<`upperBound` that corresponds to a hash value.
+///
+/// The `upperBound` must be positive and a power of 2.
 ///
 /// This function is superior to computing the remainder of `hashValue` by
 /// the range length.  Some types have bad hash functions; sometimes simple
 /// patterns in data sets create patterns in hash values and applying the
 /// remainder operation just throws away even more information and invites
-/// even more hash collisions.  This effect is especially bad if the length
-/// of the required range is a power of two -- applying the remainder
-/// operation just throws away high bits of the hash (which would not be
-/// a problem if the hash was known to be good).  This function mixes the
-/// bits in the hash value to compensate for such cases.
+/// even more hash collisions.  This effect is especially bad because the
+/// range is a power of two, which means to throws away high bits of the hash
+/// (which would not be a problem if the hash was known to be good). This
+/// function mixes the bits in the hash value to compensate for such cases.
 ///
 /// Of course, this function is a compressing function, and applying it to a
 /// hash value does not change anything fundamentally: collisions are still
 /// possible, and it does not prevent malicious users from constructing data
 /// sets that will exhibit pathological collisions.
 public // @testable
-func _squeezeHashValue(_ hashValue: Int, _ resultRange: Range<Int>) -> Int {
-  // Length of a Range<Int> does not fit into an Int, but fits into an UInt.
-  // An efficient way to compute the length is to rely on two's complement
-  // arithmetic.
-  let resultCardinality =
-    UInt(bitPattern: resultRange.upperBound &- resultRange.lowerBound)
-
-  // Calculate the result as `UInt` to handle the case when
-  // `resultCardinality >= Int.max`.
-  let unsignedResult =
-    _squeezeHashValue(hashValue, UInt(0)..<resultCardinality)
-
-  // We perform the unchecked arithmetic on `UInt` (instead of doing
-  // straightforward computations on `Int`) in order to handle the following
-  // tricky case: `startIndex` is negative, and `resultCardinality >= Int.max`.
-  // We cannot convert the latter to `Int`.
-  return
-    Int(bitPattern:
-      UInt(bitPattern: resultRange.lowerBound) &+ unsignedResult)
-}
+func _squeezeHashValue(_ hashValue: Int, _ upperBound: Int) -> Int {
+  _sanityCheck(_isPowerOf2(upperBound))
+  let mixedHashValue = _mixInt(hashValue)
 
-public // @testable
-func _squeezeHashValue(_ hashValue: Int, _ resultRange: Range<UInt>) -> UInt {
-  let mixedHashValue = UInt(bitPattern: _mixInt(hashValue))
-  let resultCardinality: UInt = resultRange.upperBound - resultRange.lowerBound
-  if _isPowerOf2(resultCardinality) {
-    return mixedHashValue & (resultCardinality - 1)
-  }
-  return resultRange.lowerBound + (mixedHashValue % resultCardinality)
+  // As `upperBound` is a power of two we can do a bitwise-and to calculate
+  // mixedHashValue % upperBound.
+  return mixedHashValue & (upperBound &- 1)
 }
+

test/SILOptimizer/no_traps_in_dict_loopup.swift

@@ -0,0 +1,13 @@
+// RUN: %target-swift-frontend -O -emit-ir -parse-as-library -primary-file %s | %FileCheck %s
+// REQUIRES: swift_stdlib_no_asserts,optimized_stdlib
+
+
+// A dictionary lookup should not contain any trap. Nothing can go wrong
+// from the user's perspective.
+
+// CHECK-NOT: llvm.trap
+
+public func testit(_ s: [Int : Int]) -> Int? {
+  return s[27]
+}
+

validation-test/stdlib/Hashing.swift

@@ -72,54 +72,20 @@ HashingTestSuite.test("_mixInt/GoldenValues") {
 
 HashingTestSuite.test("_squeezeHashValue/Int") {
   // Check that the function can return values that cover the whole range.
-  func checkRange(_ r: Range<Int>) {
-    var results = [Int : Void]()
-    for _ in 0..<(14 * (r.upperBound - r.lowerBound)) {
+  func checkRange(_ r: Int) {
+    var results = Set<Int>()
+    for _ in 0..<(14 * r) {
       let v = _squeezeHashValue(randInt(), r)
-      expectTrue(r ~= v)
-      if results[v] == nil {
-        results[v] = Void()
-      }
+      expectTrue(v < r)
+      results.insert(v)
     }
-    expectEqual(r.upperBound - r.lowerBound, results.count)
+    expectEqual(r, results.count)
   }
-  checkRange(Int.min..<(Int.min+10))
-  checkRange(0..<4)
-  checkRange(0..<8)
-  checkRange(-5..<5)
-  checkRange((Int.max-10)..<(Int.max-1))
-
-  // Check that we can handle ranges that span more than `Int.max`.
-#if arch(i386) || arch(arm)
-  expectEqual(-0x6e477d37, _squeezeHashValue(0, Int.min..<(Int.max - 1)))
-  expectEqual(0x38a3ea26, _squeezeHashValue(2, Int.min..<(Int.max - 1)))
-#elseif arch(x86_64) || arch(arm64) || arch(powerpc64) || arch(powerpc64le) || arch(s390x)
-  expectEqual(0x32b24f688dc4164d, _squeezeHashValue(0, Int.min..<(Int.max - 1)))
-  expectEqual(-0x6d1cc14f97aa822, _squeezeHashValue(1, Int.min..<(Int.max - 1)))
-#else
-  fatalError("unimplemented")
-#endif
-}
-
-HashingTestSuite.test("_squeezeHashValue/UInt") {
-  // Check that the function can return values that cover the whole range.
-  func checkRange(_ r: Range<UInt>) {
-    var results = [UInt : Void]()
-    let cardinality = r.upperBound - r.lowerBound
-    for _ in 0..<(10*cardinality) {
-      let v = _squeezeHashValue(randInt(), r)
-      expectTrue(r ~= v)
-      if results[v] == nil {
-        results[v] = Void()
-      }
-    }
-    expectEqual(Int(cardinality), results.count)
-  }
-  checkRange(0..<4)
-  checkRange(0..<8)
-  checkRange(0..<10)
-  checkRange(10..<20)
-  checkRange((UInt.max-10)..<(UInt.max-1))
+  checkRange(1)
+  checkRange(2)
+  checkRange(4)
+  checkRange(8)
+  checkRange(16)
 }
 
 HashingTestSuite.test("String/hashValue/topBitsSet") {