[4.2] Two small fixes for FloatingPoint .random( ) (#17856)

* [SR-8178] Fix BinaryFloatingPoint.random(in:) open range returning upperBound (#17794)

* Require .upperBound - .lowerBound be finite for FloatingPoint random (#17833)

This is a slightly conservative precondition; when we re-work the FloatingPoint random computation in a more principled fashion, we can relax this to only requiring that .upperBound and .lowerBound are both finite. However, the current computation will break down unless this conservative condition is used, and this is future proof--we will only relax it going forward.

Author: stephentyrone

stdlib/public/core/FloatingPoint.swift.gyb

@@ -2423,7 +2423,9 @@ where Self.RawSignificand : FixedWidthInteger {
   /// - Parameters:
   ///   - range: The range in which to create a random value.
 %   if Range == 'Range':
-  ///     `range` must not be empty.
+  ///     `range` must be non-empty and finite.
+%   else:
+  ///     `range` must be finite.
 %   end
   ///   - generator: The random number generator to use when creating the
   ///     new random value.
@@ -2438,6 +2440,15 @@ where Self.RawSignificand : FixedWidthInteger {
       "Can't get random value with an empty range"
     )
     let delta = range.upperBound - range.lowerBound
+    //  TODO: this still isn't quite right, because the computation of delta
+    //  can overflow (e.g. if .upperBound = .maximumFiniteMagnitude and
+    //  .lowerBound = -.upperBound); this should be re-written with an
+    //  algorithm that handles that case correctly, but this precondition
+    //  is an acceptable short-term fix.
+    _precondition(
+      delta.isFinite,
+      "There is no uniform distribution on an infinite range"
+    )
     let rand: Self.RawSignificand
     if Self.RawSignificand.bitWidth == Self.significandBitCount + 1 {
       rand = generator.next()
@@ -2451,16 +2462,25 @@ where Self.RawSignificand : FixedWidthInteger {
       let significandCount = Self.significandBitCount + 1
       let maxSignificand: Self.RawSignificand = 1 << significandCount
 %     if 'Closed' not in Range:
-      rand = generator.next(upperBound: maxSignificand)
+      // Rather than use .next(upperBound:), which has to work with arbitrary
+      // upper bounds, and therefore does extra work to avoid bias, we can take
+      // a shortcut because we know that maxSignificand is a power of two.
+      rand = generator.next() & (maxSignificand - 1)
 %     else:
       rand = generator.next(upperBound: maxSignificand + 1)
       if rand == maxSignificand {
         return range.upperBound
       }
 %     end
     }
-    let unitRandom = Self.init(rand) * Self.ulpOfOne / 2
-    return delta * unitRandom + range.lowerBound
+    let unitRandom = Self.init(rand) * (Self.ulpOfOne / 2)
+    let randFloat = delta * unitRandom + range.lowerBound
+%   if 'Closed' not in Range:
+    if randFloat == range.upperBound {
+      return Self.random(in: range, using: &generator)
+    }
+%   end
+    return randFloat
   }
 
   /// Returns a random value within the specified range.
@@ -2488,7 +2508,9 @@ where Self.RawSignificand : FixedWidthInteger {
   ///
   /// - Parameter range: The range in which to create a random value.
 %   if Range == 'Range':
-  ///   `range` must not be empty.
+  ///   `range` must be non-empty and finite.
+%   else:
+  ///   `range` must be finite.
 %   end
   /// - Returns: A random value within the bounds of `range`.
   @inlinable

validation-test/stdlib/Random.swift

@@ -43,8 +43,7 @@ RandomTests.test("basic random numbers") {
 
 // Random integers in ranges
 
-func integerRangeTest<T: FixedWidthInteger>(_ type: T.Type) 
-  where T.Stride: SignedInteger, T.Magnitude: UnsignedInteger {
+func integerRangeTest<T: FixedWidthInteger>(_ type: T.Type) {
 
   func testRange(_ range: Range<T>, iterations: Int = 1_000) {
     var integerSet: Set<T> = []
@@ -104,10 +103,8 @@ RandomTests.test("random integers in ranges") {
 // Random floating points in ranges
 
 func floatingPointRangeTest<T: BinaryFloatingPoint>(_ type: T.Type) 
-  where T.RawSignificand: FixedWidthInteger,
-        T.RawSignificand.Stride: SignedInteger & FixedWidthInteger,
-        T.RawSignificand.Magnitude: UnsignedInteger {
-          
+  where T.RawSignificand: FixedWidthInteger {
+  
   let testRange = 0 ..< 1_000
 
   // open range
@@ -230,6 +227,56 @@ RandomTests.test("different random number generators") {
   expectEqual(shufflePasses[0], shufflePasses[1])
 }
 
+// Random floating points with max values (SR-8178)
+
+var lcrng = LCRNG(seed: 1234567890)
+
+public struct RotatingRNG: RandomNumberGenerator {
+  public let rotation: [() -> UInt64] = [
+    { return .min },
+    { return .max },
+    { return lcrng.next() }
+  ]
+  public var rotationIndex = 0
+  
+  public mutating func next() -> UInt64 {
+    if rotationIndex == rotation.count {
+      rotationIndex = 0
+    }
+    
+    defer {
+      rotationIndex += 1
+    }
+    
+    return rotation[rotationIndex]()
+  }
+}
+
+func floatingPointRangeMaxTest<T: BinaryFloatingPoint>(_ type: T.Type)
+  where T.RawSignificand: FixedWidthInteger {
+  
+  let testRange = 0 ..< 1_000
+  
+  var rng = RotatingRNG()
+  let ranges: [Range<T>] = [0 ..< 1, 1 ..< 2, 10 ..< 11, 0 ..< 10]
+  for range in ranges {
+    for _ in testRange {
+      let random = T.random(in: range, using: &rng)
+      expectTrue(range.contains(random))
+      expectTrue(range.lowerBound <= random)
+      expectTrue(random < range.upperBound)
+    }
+  }
+}
+
+RandomTests.test("random floating point range maxes") {
+  floatingPointRangeMaxTest(Float.self)
+  floatingPointRangeMaxTest(Double.self)
+#if !os(Windows) && (arch(i386) || arch(x86_64))
+  floatingPointRangeMaxTest(Float80.self)
+#endif
+}
+
 // Uniform Distribution
 
 func chi2Test(_ samples: [Double]) -> Bool {
@@ -248,7 +295,7 @@ func chi2Test(_ samples: [Double]) -> Bool {
 
   if chi2 > cvHigh {
     return false
-  }else {
+  } else {
     return true
   }
 }