Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SE-0225] Implementation of isMultiple for BinaryInteger. #18689

Merged
merged 2 commits into from
Sep 4, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ extension MockBinaryInteger : BinaryInteger {
var trailingZeroBitCount: Int {
return _value.trailingZeroBitCount
}

func isMultiple(of other: MockBinaryInteger<T>) -> Bool {
return _value.isMultiple(of: other._value)
}

static func + (
lhs: MockBinaryInteger<T>, rhs: MockBinaryInteger<T>
Expand Down
25 changes: 25 additions & 0 deletions stdlib/public/core/Integers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,21 @@ public protocol BinaryInteger :
func quotientAndRemainder(dividingBy rhs: Self)
-> (quotient: Self, remainder: Self)

/// Returns true if this value is a multiple of `other`, and false otherwise.
///
/// For two integers a and b, a is a multiple of b if there exists a third
/// integer q such that a = q*b. For example, 6 is a multiple of 3, because
/// 6 = 2*3, and zero is a multiple of everything, because 0 = 0*x, for any
/// integer x.
///
/// Two edge cases are worth particular attention:
/// - `x.isMultiple(of: 0)` is `true` if `x` is zero and `false` otherwise.
/// - `T.min.isMultiple(of: -1)` is `true` for signed integer `T`, even
/// though the quotient `T.min / -1` is not representable in type `T`.
///
/// - Parameter other: the value to test.
func isMultiple(of other: Self) -> Bool

/// Returns `-1` if this value is negative and `1` if it's positive;
/// otherwise, `0`.
///
Expand Down Expand Up @@ -2755,6 +2770,16 @@ extension FixedWidthInteger {
lhs = _nonMaskingRightShift(lhs, shift)
}

@inlinable
public func isMultiple(of other: Self) -> Bool {
// Nothing but zero is a multiple of zero.
if other == 0 { return self == 0 }
// Special case to avoid overflow on .min / -1 for signed types.
if Self.isSigned && self == .min && other == -1 { return true }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be curious if performance changes if the two conditionals were replaced with a check of the magnitude, i.e. return other.magnitude <= 1 || self % other == 0...

Copy link
Contributor Author

@stephentyrone stephentyrone Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For concrete types, there should basically be no difference in the face of optimization. We should be able to teach the optimizer to have only a single test on the fast-path, but that's follow-on work after the API is in.

// Having handled those special cases, this is safe.
return self % other == 0
}

@inlinable // FIXME(sil-serialize-all)
@inline(__always)
public static func _nonMaskingRightShift(_ lhs: Self, _ rhs: Int) -> Self {
Expand Down
3 changes: 1 addition & 2 deletions test/Constraints/tuple-arguments-supported.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ func test8(_: ((Int, Int)) -> Int) {}
test8 { (_, _) -> Int in 2 }
test8 { (x, y) in x }

func isEven(_ x: Int) -> Bool { return x % 2 == 0 }
let items = Array(zip(0..<10, 0..<10))
_ = items.filter { (_, x) in isEven(x) }
_ = items.filter { (_, x) in x.isMultiple(of: 2) }
_ = items.filter { _ in true }

func toString(indexes: Int?...) -> String {
Expand Down
11 changes: 11 additions & 0 deletions test/Prototypes/BigInt.swift
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,11 @@ public struct _BigInt<Word: FixedWidthInteger & UnsignedInteger> :
let r = x._internalDivide(by: rhs)
return (x, r)
}

public func isMultiple(of other: _BigInt) -> Bool {
if other == 0 { return self == 0 }
return self % other == 0
}

public static func &=(lhs: inout _BigInt, rhs: _BigInt) {
var lhsTemp = lhs._dataAsTwosComplement()
Expand Down Expand Up @@ -1867,3 +1872,9 @@ BigIntBitTests.test("words") {
}

runAllTests()

BigIntTests.test("isMultiple") {
// Test that these do not crash.
expectTrue((0 as _BigInt<UInt>).isMultiple(of: 0))
expectFalse((1 as _BigInt<UInt>).isMultiple(of: 0))
}
18 changes: 15 additions & 3 deletions test/Prototypes/DoubleWidth.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ import StdlibUnittest
/// integer type. Nesting `DoubleWidth` instances, in particular, may result in
/// undesirable performance.
public struct DoubleWidth<Base : FixedWidthInteger>
where Base.Magnitude : UnsignedInteger,
Base.Words : Collection, Base.Magnitude.Words : Collection {
where Base.Words : Collection, Base.Magnitude.Words : Collection {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An unrelated PR a few weeks ago removed the need for the .Magnitude : UnsignedInteger conformance. Removing it to get rid of warning in tests.


public typealias High = Base
public typealias Low = Base.Magnitude
Expand Down Expand Up @@ -282,7 +281,7 @@ extension DoubleWidth.Words: Collection {

public func index(after i: Index) -> Index {
switch i._value {
case let .low(li) where Base.bitWidth < UInt.bitWidth:
case .low where Base.bitWidth < UInt.bitWidth:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is also just cleaning up some noise.

return Index(.high(_high.endIndex))
case let .low(li):
let next = _low.index(after: li)
Expand Down Expand Up @@ -1004,6 +1003,19 @@ dwTests.test("DivideMinByMinusOne") {
_ = f(Int1024.min)
}

dwTests.test("isMultiple") {
func isMultipleTest<T: FixedWidthInteger>(type: T.Type) {
expectTrue(T.min.isMultiple(of: 2))
expectFalse(T.max.isMultiple(of: 10))
// Test that these do not crash.
expectTrue((0 as T).isMultiple(of: 0))
expectFalse((1 as T).isMultiple(of: 0))
expectTrue(T.min.isMultiple(of: 0 &- 1))
}
isMultipleTest(type: Int128.self)
isMultipleTest(type: UInt128.self)
}

dwTests.test("MultiplyMinByMinusOne") {
func f(_ x: Int1024) -> Int1024 {
return x * -1
Expand Down
27 changes: 26 additions & 1 deletion test/stdlib/Integers.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ extension MockBinaryInteger : BinaryInteger {
return _value.trailingZeroBitCount
}

func isMultiple(of other: MockBinaryInteger<T>) -> Bool {
return _value.isMultiple(of: other._value)
}

static func + (
lhs: MockBinaryInteger<T>, rhs: MockBinaryInteger<T>
) -> MockBinaryInteger<T> {
Expand Down Expand Up @@ -560,7 +564,7 @@ tests.test("truncatingIfNeeded") {

tests.test("Parsing/LosslessStringConvertible") {
func _toArray<T: LosslessStringConvertible>(_ text: String) -> [T] {
return text.split(separator: " ").map { T(String($0)) }.flatMap { $0 }
return text.split(separator: " ").map { T(String($0)) }.compactMap { $0 }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor update to silence warning while we're here.

}

expectEqualSequence([1, 2, 3], _toArray("1 2 3") as [Int])
Expand Down Expand Up @@ -798,6 +802,27 @@ tests.test("DivideMinByMinusOne") {
_ = f(Int.min)
}

tests.test("isMultiple") {
func isMultipleTest<T: FixedWidthInteger>(type: T.Type) {
expectTrue(T.min.isMultiple(of: 2))
expectFalse(T.max.isMultiple(of: 10))
// Test that these do not crash.
expectTrue((0 as T).isMultiple(of: 0))
expectFalse((1 as T).isMultiple(of: 0))
expectTrue(T.min.isMultiple(of: 0 &- 1))
}
isMultipleTest(type: Int.self)
isMultipleTest(type: Int8.self)
isMultipleTest(type: Int16.self)
isMultipleTest(type: Int32.self)
isMultipleTest(type: Int64.self)
isMultipleTest(type: UInt.self)
isMultipleTest(type: UInt8.self)
isMultipleTest(type: UInt16.self)
isMultipleTest(type: UInt32.self)
isMultipleTest(type: UInt64.self)
}

tests.test("MultiplyMinByMinusOne") {
func f(_ x: Int) -> Int {
return x * -1
Expand Down