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.isValidInt gives an incorrect result for large numbers #4540

Closed
scabug opened this issue May 3, 2011 · 8 comments
Closed

BigInt.isValidInt gives an incorrect result for large numbers #4540

scabug opened this issue May 3, 2011 · 8 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented May 3, 2011

=== What steps will reproduce the problem (please be specific and use wikiformatting)? ===

scala> BigInt("10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").isValidInt

=== What is the expected behavior? ===

res1: Boolean = false

=== What do you see instead? ===

res1: Boolean = true

=== Additional information ===

From what I understood, the problematic code is in ScalaNumericConversions.scala:

def isValidInt = isWhole && (toInt == toLong)

which won't work for numbers larger than 64 bits.

=== What versions of the following are you using? ===

  • Scala: 2.8.1.r0-b20110222191246
  • Java: 1.6.0_18
  • Operating system: GNU/Linux
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented May 3, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4540?orig=1
Reporter: svasey

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented May 4, 2011

@dragos said:
Paul, since it looks like you worked on these methods, can you please have a look?

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented May 5, 2011

@paulp said:
(In r24891) Makes BigInt's isValidThing methods make some kind of sense.
I wish I hadn't written so much code for the numerical classes
which languishes in git tributaries. Closes #4540, no review.

@scabug scabug closed this May 18, 2011
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Apr 3, 2012

@nadezhin said:
There are still other failures of ScalaNumericConversions.isValidXXX functions.

def testDoubleIsValidByte() {
    assert((0.0).isValidByte)
}
def testLongIsValidByte() {
  assert(!(1L << 32).isValidByte)
}

The expected behaviour - these tests should pass.
They fail in the master.

Preparing a pull request...

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Apr 8, 2012

@nadezhin said:
Pull request #363
scala/scala#363
fixes RichDouble.isValid[Byte|Short|Char|Int].

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 29, 2017

Stephane Bersier (stephane bersier) said:
This bug or a similar one still seems alive in ScalaNumericConversions. For example:

/** Returns true iff this has a zero fractional part, and is within the
* range of [[scala.Int]] MinValue and MaxValue; otherwise returns false.
*/
def isValidInt = isWhole && (toLong == toInt)

This fails any time n % 2^64 == n % 2^32 and |n| > 2^64. For example 2^65 + 1.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 29, 2017

@nadezhin said:
IsValidInt is overriden in BigInt:
override def isValidInt = this >= Int.MinValue && this <= Int.MaxValue .

I am not sure how this bug can happen.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Apr 1, 2017

Stephane Bersier (stephane bersier) said:
It can cause a fault if someone extends ScalaNumericConversions! It should be left abstract if everyone is supposed to override it.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.