[commons-numbers] 08/18: NUMBERS-120: Don't calculate more bits than necessary in toFloatingPointBits(int, int)

[er...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

erans pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit 167f004f167c5177c7944e4c1f5e9d72dcbd1843
Author: Schamschi <he...@gmx.at>
AuthorDate: Thu Jun 27 14:50:01 2019 +0200

    NUMBERS-120: Don't calculate more bits than necessary in toFloatingPointBits(int, int)
---
 .../commons/numbers/fraction/BigFraction.java      | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
index 3a1a63d..c5410b0 100644
--- a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
+++ b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/BigFraction.java
@@ -712,23 +712,29 @@ public class BigFraction extends Number implements Comparable<BigFraction>, Seri
          * significant bits of the fraction's value using integer division. To
          * guarantee that the quotient of the division has at least
          * (significandLength + 2) bits, the bit length of the dividend must
-         * exceed that of the divisor by at least that amount. If the numerator
-         * has powers of 2 beyond this limit, they can be removed as well.
+         * exceed that of the divisor by at least that amount.
+         *
+         * If the denominator has prime factors other than 2, i.e. if the
+         * divisor was not reduced to 1, an excess of exactly
+         * (significandLength + 2) bits is sufficient, because the knowledge
+         * that the fractional part of the precise quotient's binary
+         * representation does not terminate is enough information to resolve
+         * cases where the most significant (significandLength + 2) bits alone
+         * are not conclusive.
+         *
+         * Otherwise, the quotient must be calculated exactly and the bit length
+         * of the numerator can only be reduced as long as no precision is lost
+         * in the process (meaning it can have powers of 2 removed, like the
+         * denominator).
          */
         int numRightShift = positiveNumerator.bitLength() - divisor.bitLength() - (significandLength + 2);
-        if (numRightShift > 0) {
+        if (numRightShift > 0 && divisor.equals(BigInteger.ONE)) {
             numRightShift = Math.min(numRightShift, positiveNumerator.getLowestSetBit());
         }
         BigInteger dividend = positiveNumerator.shiftRight(numRightShift);
 
         BigInteger quotient = dividend.divide(divisor);
 
-        /*
-         * If the denominator was a power of two, then the divisor was reduced
-         * to 1, meaning the quotient was calculated exactly. Otherwise, the
-         * fractional part of the precise quotient's binary representation does
-         * not terminate.
-         */
         int quotRightShift = quotient.bitLength() - (significandLength + 1);
         long significand = roundAndRightShift(
                 quotient,