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

[BigInt tests] ❌🐰 Int properties #252

Open
wants to merge 3 commits into
base: biginteger
Choose a base branch
from

Conversation

LiarPrincess
Copy link

@LiarPrincess LiarPrincess commented Jan 18, 2023

Please read the #242 Using tests from “Violet - Python VM written in Swift” before.


🐰 Discussion

  • bitWidth - current implementation rounds up to nearest multiple of 64 (I assume that this is because UInt.bitWidth == 64 on my platform). In Violet I went with counting min required bit count to represent a given number.

    func test_trivial() {
      let zero = BigInt(0)
      XCTAssertEqual(zero.bitWidth, 1) //  0 is just 0
    
      let plus1 = BigInt(1)
      XCTAssertEqual(plus1.bitWidth, 2) // 1 needs '0' prefix -> '01'
    
      let minus1 = BigInt(-1)
      XCTAssertEqual(minus1.bitWidth, 1) // -1 is just 1
    }
  • trailingZeroBitCount - what should be the value for 0? Current implementation gives 64 (same reason as for bitWidth), Violet returns 1.

    // There is an edge case for '0':
    // - 'int' is finite, so they can return 'bitWidth'
    // - 'BigInt' is infinite, but we can't return this
    //
    // So, trailingZeroBitCount is equal to bitWidth, see:
    // https://developer.apple.com/documentation/swift/binaryinteger/trailingzerobitcount
    func test_zero() {
      let zero = BigInt(0)
      XCTAssertEqual(zero.trailingZeroBitCount, 1)
    }

Let me know what you think, I will fix the tests.

❌ Failures

func test_magnitude() {
  let big = BigInt(-9223372036854775808)
  let expect = UInt64(9223372036854775808)
  XCTAssertEqual(big.magnitude, BigInt(expect))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant