apple / swift-numerics

Advanced mathematical types and functions for Swift
Apache License 2.0
1.67k stars 142 forks source link

Add the `DoubleWidth` type #197

Closed benrimmington closed 2 years ago

benrimmington commented 3 years ago

The DoubleWidth<Base> type has twice the bit width of its base type. It was proposed in SE-0104, but never shipped (due to compiler problems).

Start date End date File history
2017-01-05 2017-06-05 stdlib/public/core/Integers.swift.gyb
2017-06-06 2018-03-20 stdlib/public/core/DoubleWidth.swift.gyb
2018-03-23 2019-03-01 test/Prototypes/DoubleWidth.swift.gyb

The DoubleWidth.swift file is generated from the latest GYB template:

utils/gyb --line-directive="" test/Prototypes/DoubleWidth.swift.gyb
arch config compilation
x86_64 debug 1.2 seconds
x86_64 release 2.5 seconds
arm64 release 2.4 seconds

The DoubleWidthTests.swift file is a translation of the original tests (from StdlibUnittest to XCTest), excluding the expectCrashLater() calls.

arch config compilation
x86_64 debug 5.8 seconds
x86_64 release 9.7 seconds
arm64 release 9.6 seconds

There are also some new and updated tests — for string conversions and SR-6947.

arch config all / slowest tests execution
x86_64 debug all DoubleWidthTests 2.0 seconds
x86_64 debug …_Hexadecimal() 1.2 seconds
x86_64 debug …_LeftAndRightShifts() 0.7 seconds
x86_64 release all DoubleWidthTests 1.2 seconds
x86_64 release …_Hexadecimal() 0.7 seconds
x86_64 release …_LeftAndRightShifts() 0.4 seconds
benrimmington commented 3 years ago

I tried to test the DoubleWidth: _ExpressibleByBuiltinIntegerLiteral conformance:

+      swiftSettings: [.unsafeFlags(["-parse-stdlib"])]

+import Swift

-  public init(_builtinIntegerLiteral _x: Builtin.IntegerLiteral) {
+  public init(_builtinIntegerLiteral _x: Builtin.IntLiteral) {

-    if !Bool(_overflow) {
+    if !Bool(_builtinBooleanLiteral: _overflow) {

-  public init(integerLiteral x: Int) {
-    self.init(x)
-  }

There are two unrecognized builtins:

When arbitrary-precision literals were implemented, DoubleWidth: _ExpressibleByBuiltinIntegerLiteral was already within the #if false block:

https://github.com/apple/swift/pull/20208/files?file-filters%5B%5D=.gyb#diff-11720e84c8f7b8f4dfedfb52e97e620ec4d9249f662995d7fc67c6ce96c0438f

stephentyrone commented 2 years ago

Hi @benrimmington thanks. Just to set expectations, I'm booked up this week and on vacation next week, so it will probably be a little while before I get to take a look at this in any detail.

stephentyrone commented 2 years ago

Hi Ben, finally getting a chance to look at this, sorry for the delay.

Generally this seems fine (as expected, given that it's mostly borrowed from Swift's prototype). I think the only thing I'd like to tweak is a meta-point of not packaging a library for it yet (since it's still "experimental" and hence we don't really want to encourage use outside the project). However, I do have fairly immediate use cases in mind for testing some of the new IntegerUtilities API, so I think the best thing to do would be to move DoubleWidth into TestUtilities, where it will be available for use testing anything else.

If you have a specific use case that merits vending a library, let's discuss further.

benrimmington commented 2 years ago

move DoubleWidth into TestUtilities

Do you want me to do this, or would it be better if you (or Nate Cook, as the original implementer of DoubleWidth) added the files? TestUtilities would be a new module, I assume, unless you're referring to _TestSupport — but that seems to be only for real and complex testing?

stephentyrone commented 2 years ago

Sorry, yes, I mean _TestSupport. I'm happy to do this if you don't have time at the moment.

stephentyrone commented 2 years ago

Thanks! I'll finish reviewing this later today or tomorrow.

stephentyrone commented 2 years ago

@swift-ci please test

benrimmington commented 2 years ago

I moved DoubleWidthTests.swift into the IntegerUtilitiesTests target. (Alternatively, _TestSupport could have its own test target.) The CMake build might be broken at the moment, but I don't know how to test this.

stephentyrone commented 2 years ago

I'll take a look at the CMake build as part of my review.

benrimmington commented 2 years ago

IntegerUtilitiesTests now has a _TestSupport dependency (in Package.swift), shall I update CMakeLists.txt as well?

stephentyrone commented 2 years ago

@swift-ci test

xwu commented 2 years ago

Delighted to see this code in runnable form again—thanks @benrimmington!