SE-0202 Amendment: Remove RNG's next extension methods


(Alejandro Alonso) #1

I apologize for another SE-0202 Amendment, this is a very interesting bug that made its way past me, but hopefully this is the last one! Refer to: Trying to use the new RandomNumberGenerator protocol.

Currently on RandomNumberGenerator, there is an extension that defines two utility methods.

public protocol RandomNumberGenerator {
  mutating func next() -> UInt64
}

extension RandomNumberGenerator {
  public mutating func next<T: FixedWidthInteger & UnsignedInteger>() -> T {
    // we call next() -> UInt64 somewhere in here
  }

  public mutating func next<T: FixedWidthInteger & UnsignedInteger>(
    upperBound: T
  ) -> T {}
}

The extension method next<T>() -> T is currently causing an issue where its qualified as a default implementation of the next() -> UInt64 requirement. Implementers of RandomNumberGenerator were not required to write the next() -> UInt64 requirement, thus creating a runtime error if they hadn't defined next() -> UIn64 themselves.

struct MersenneTwister: RandomNumberGenerator {}

var mt = MersenneTwister()
let x = Int.random(in: 0 ..< 10, using: &mt) // Runtime error

This Amendment aims to remove both of those extension methods from the public interface (making both methods internal to the stdlib).

There was a question about how this affects the extensability of the random API. This change should have no effect to the extensability of the random API. These functions currently only serve to implement .random(in:using:) on the integer types. Users are encouraged to write somewhat more meaningful code like Int.random(in: 0 ..< 10, using: &generator) instead of calling direct functions on generators themselves.

// if you need to get a generic full width integer assuming
// T conforms to FixedWidthInteger and UnsignedInteger
let x: T = generator.next()
// with the next one, it doesn't have to be UnsignedInteger
// or
let x = T.random(in: .min ... .max, using: &generator)

It's a bit more verbose, but it's much clearer to the reader about what sort of value x.
Another example:

// if you need a number from 0 to upperBound (say 16) assuming
// T conforms to FixedWidthInteger and UnsignedInteger
let x: T = generator.next(upperBound: 16)
// with the next one, it doesn't have to be UnsignedInteger
// or
let x = T.random(in: 0 ..< 16, using: &generator)

This not only fixes an embarrassing bug on my part, but encourages more usage of the .random(in:using:).

Evolution PR: https://github.com/apple/swift-evolution/pull/912
Implementation: https://github.com/apple/swift/pull/19551


(Brent Royal-Gordon) #2

What exactly is the amendment? Maybe I’m missing something, but it seems like you’re describing a problem but not the solution.

If you’re suggesting removing one of the next() methods, could we use the @available(*, unavailable) trick to provide a “better” default that will cause an error?


(Alejandro Alonso) #3

The current RNG protocol looks like this:

public protocol RandomNumberGenerator {
  mutating func next() -> UInt64
}

// The amendment is aiming to remove both of these from public
extension RandomNumberGenerator {
  public mutating func next<T: FixedWidthInteger & UnsignedInteger>() -> T {
    // we call next() -> UInt64 somewhere in here
  }

  public mutating func next<T: FixedWidthInteger & UnsignedInteger>(
    upperBound: T
  ) -> T {}
}

The issue with the current design is that next<T>() -> T currently qualifies as a default implementation of the next() -> UInt64 requirement. Therefore, users don't have to implement anything when conforming to RNG. The amendment wants to essentially remove those extension methods from public access level to internal for the stdlib. As of right now (for the standard library), the only (real) use of these functions are towards the implementations of T.random(in:using:) where T conforms to FixedWidthInteger.


(Alejandro Alonso) #4

@brentdax I reworded the OP to hopefully clear up any confusion. Thanks!