Merge pull request #33 from pietroppeter/fix-empty-limbs

Fix empty limbs

Author: def-

bigints.nimble

@@ -1,6 +1,6 @@
 # Package
 
-version     = "0.4.3"
+version     = "0.4.4"
 author      = "Dennis Felsing"
 description = "Arbitrary-precision integers implemented in pure Nim"
 license     = "MIT"

readme.md

@@ -29,7 +29,6 @@ For examples of usage see the [examples](examples) folder.
 ## Current limitations and possible enhancements
 
 * cannot multiply a number with itself (x *= x). An issue like this exist also for addition (#27) and possibly other operations
-* an uninitialized `BigInt` might raise error when performing some operations (e.g. #26)
 * not expected to work on 32 bit
 * some common bitwise operations (`and`, `or`, `xor`, `not`) are not implemented
 * operations between `BigInt` and standard integer types besides `int32` are not implemented
@@ -121,25 +120,47 @@ template initBigInt(val: uint): BigInt
 proc initBigInt(val: BigInt): BigInt
 ```
 
+## **const** zero
+
+
+```nim
+zero = ([0'u], {})
+```
+
+## **const** one
+
+
+```nim
+one = ([1'u], {})
+```
+
 ## **proc** cmp
 
+Returns:
+ * a value less than zero, if <tt class="docutils literal"><span class="pre">a &lt; b</span></tt>
+ * a value greater than zero, if <tt class="docutils literal"><span class="pre">a > b</span></tt>
+ * zero, if <tt class="docutils literal"><span class="pre">a == b</span></tt>
 
 ```nim
 proc cmp(a, b: BigInt): int64
 ```
 
 ## **proc** cmp
 
+Returns:
+ * a value less than zero, if <tt class="docutils literal"><span class="pre">a &lt; b</span></tt>
+ * a value greater than zero, if <tt class="docutils literal"><span class="pre">a > b</span></tt>
+ * zero, if <tt class="docutils literal"><span class="pre">a == b</span></tt>
 
 ```nim
-proc cmp(a: int32; b: BigInt): int64
+proc cmp(a: BigInt; b: int32): int64
 ```
 
 ## **proc** cmp
 
 
 ```nim
-proc cmp(a: BigInt; b: int32): int64
+proc cmp(a: int32; b: BigInt): int64
 ```
 
 ## **proc** `<`

src/bigints.nim

@@ -66,19 +66,26 @@ else:
 proc initBigInt*(val: BigInt): BigInt =
   result = val
 
-const zero = initBigInt(0)
-const one = initBigInt(1)
+const zero* = initBigInt(0)
+const one* = initBigInt(1)
+
+proc isZero(a: BigInt): bool {.inline.} =
+  for i in countdown(a.limbs.high, 0):
+    if a.limbs[i] != 0'u32:
+      return false
+  return true
 
 proc unsignedCmp(a: BigInt, b: int32): int64 =
+  # here a and b have same sign a none of them is zero.
+  # in particular we have that a.limbs.len >= 1
   result = int64(a.limbs.len) - 1
 
   if result != 0:
     return
 
   result = int64(a.limbs[0]) - int64(b)
 
-proc unsignedCmp(a: int32, b: BigInt): int64 =
-  -unsignedCmp(b, a)
+proc unsignedCmp(a: int32, b: BigInt): int64 = -unsignedCmp(b, a)
 
 proc unsignedCmp(a, b: BigInt): int64 =
   result = int64(a.limbs.len) - int64(b.limbs.len)
@@ -93,40 +100,52 @@ proc unsignedCmp(a, b: BigInt): int64 =
       return
 
 proc cmp*(a, b: BigInt): int64 =
-  if Negative in a.flags and a.limbs != @[0'u32]:
-    if Negative in b.flags and b.limbs != @[0'u32]:
-      return unsignedCmp(b, a)
-    else:
-      return -1
-  else:
-    if Negative in b.flags:
+  ## Returns:
+  ## * a value less than zero, if `a < b`
+  ## * a value greater than zero, if `a > b`
+  ## * zero, if `a == b`
+  if a.isZero:
+    if b.isZero:
+      return 0
+    elif Negative in b.flags: # b.isNegative
       return 1
-    else:
-      return unsignedCmp(a, b)
-
-proc cmp*(a: int32, b: BigInt): int64 =
-  if a < 0:
-    if Negative in b.flags and b.limbs != @[0'u32]:
-      return unsignedCmp(b, a)
     else:
       return -1
-  else:
-    if Negative in b.flags:
+  elif Negative in a.flags: # a.isNegative
+    if b.isZero or Negative notin b.flags: # b >= 0
+      return -1
+    else:
+      return unsignedCmp(b, a) 
+  else: # a > 0
+    if b.isZero or Negative in b.flags: # b <= 0
       return 1
     else:
       return unsignedCmp(a, b)
 
 proc cmp*(a: BigInt, b: int32): int64 =
-  if Negative in a.flags and a.limbs != @[0'u32]:
+  ## Returns:
+  ## * a value less than zero, if `a < b`
+  ## * a value greater than zero, if `a > b`
+  ## * zero, if `a == b`
+  if a.isZero:
     if b < 0:
-      return unsignedCmp(b, a)
+      return 1
+    elif b == 0:
+      return 0
     else:
       return -1
-  else:
+  elif Negative in a.flags:  # a.isNegative
     if b < 0:
+      return unsignedCmp(b, a)
+    else:
+      return -1
+  else: # a > 0
+    if b <= 0:
       return 1
     else:
-      return unsignedCmp(a, b)
+      return unsignedCmp(b, a)
+
+proc cmp*(a: int32, b: BigInt): int64 = -cmp(b, a)
 
 proc `<` *(a, b: BigInt): bool = cmp(a, b) < 0
 proc `<` *(a: BigInt, b: int32): bool = cmp(a, b) < 0
@@ -285,7 +304,9 @@ proc unsignedSubtraction(a: var BigInt, b, c: BigInt) =
       negate(a)
 
 proc additionInt(a: var BigInt, b: BigInt, c: int32) =
-  if Negative in b.flags:
+  if b.isZero:
+    a = c.initBigInt
+  elif Negative in b.flags:
     if c < 0:
       unsignedAdditionInt(a, b, c)
       a.flags.incl(Negative)
@@ -333,7 +354,9 @@ template optAddInt*{x = y + z}(x,y: BigInt, z: int32) = additionInt(x, y, z)
 template optAdd*{x = y + z}(x,y,z: BigInt) = addition(x, y, z)
 
 proc subtractionInt(a: var BigInt, b: BigInt, c: int32) =
-  if Negative in b.flags:
+  if b.isZero:
+    a = (-c).initBigInt
+  elif Negative in b.flags:
     if c < 0:
       # TODO: is this right?
       unsignedSubtractionInt(a, b, c)
@@ -390,6 +413,8 @@ template unsignedMultiplicationInt(a: BigInt, b: BigInt, c: int32, bl) =
   normalize(a)
 
 template unsignedMultiplication(a: BigInt, b, c: BigInt, bl, cl) =
+  # always called with bl >= cl
+
   for i in 0 ..< bl:
     tmp += uint64(b.limbs[i]) * uint64(c.limbs[0])
     a.limbs[i] = uint32(tmp and uint32.high)
@@ -445,21 +470,20 @@ proc multiplicationInt(a: var BigInt, b: BigInt, c: int32) =
 
 # This doesn't work when a = b
 proc multiplication(a: var BigInt, b, c: BigInt) =
+  if b.isZero or c.isZero:
+    a = zero
+    return
   let
     bl = b.limbs.len
     cl = c.limbs.len
   var tmp: uint64
 
   a.limbs.setXLen(bl + cl)
-
   if cl > bl:
     unsignedMultiplication(a, c, b, cl, bl)
   else:
     unsignedMultiplication(a, b, c, bl, cl)
 
-  if a.limbs == @[0'u32]:
-    return
-
   if Negative in b.flags:
     if Negative in c.flags:
       a.flags.excl(Negative)
@@ -872,6 +896,8 @@ proc pow*(base: int32|BigInt, exp: int32|BigInt): BigInt =
     base *= tmp
 
 proc toString*(a: BigInt, base: range[2..36] = 10): string =
+  if a.isZero:
+    return "0"
   if base in multiples:
     return toStringMultipleTwo(a, base)
 

tests/tester.nim

@@ -109,3 +109,122 @@ test "validate digits in string parsing (https://github.com/def-/nim-bigints/iss
     discard initBigInt("2", 2)
   expect ValueError:
     discard initBigInt("z", 35)
+
+test "empty limbs when uninitialized (https://github.com/def-/nim-bigints/issues/26)":
+  # reported issue has an example about multiplication and it is due to a call to a.limbs[0] for an uninitialized a: BigInt
+  # besides multiplication, one could look at appearances of [0] in source to find possible failures
+  # failures bound to reaching a line with [0] are fatal
+  # besides appearances of [0], also logic implemented through a.limbs.len might (and indeed does) show error
+  # logic around sign might also play a role
+  var
+    zeroEmpty: BigInt # should be treated as zero, same with -zeroEmpty
+  let
+    zeroInt32: int32 = 0
+    oneInt32: int32 = 1
+    bigOne: BigInt = initBigInt(@[0.uint32, 1])
+  
+  # unsignedCmp(a: BigInt, b: int32) has [0]; used by public cmp and <
+  # never reached in the following cases (no fatal), since it cannot be reached (see comment in code)
+  # still, errors can be found in comparing zero to zero 
+  check zeroEmpty < oneInt32 # ok
+  check zeroEmpty > -oneInt32 # ok
+  check -zeroEmpty < oneInt32 # ok
+  check -zeroEmpty > -oneInt32 # ok
+  check not(zeroEmpty < zeroInt32) # error: fixed
+  check not(zeroEmpty > zeroInt32) # ok
+  check not(-zeroEmpty < zeroInt32) # error: fixed
+  check not(-zeroEmpty > zeroInt32) # ok
+  check zeroEmpty == zeroInt32 # error: fixed
+  check -zeroEmpty == zeroInt32 # error: fixed
+  
+  # this came up in the above testing and can be though as secondary effect of unitialization (fix in negate?)
+  check $zero == "0" # ok
+  check $zeroEmpty == "0" # ok
+  check $(-zeroEmpty) == "0" # error: fixed
+
+  # unsignedCmp(a, b: BigInt) has no [0] but it has logic with limbs.len
+  check zeroEmpty < one # ok
+  check zeroEmpty > -one # ok
+  check -zeroEmpty < one # ok
+  check -zeroEmpty > -one # ok
+  check not (zeroEmpty < zeroInt32) # error: fixed
+  check not (zeroEmpty > zeroInt32) # ok
+  check not (zeroEmpty < zero) # error: fixed
+  check not (zeroEmpty > zero) # ok
+  check zeroEmpty == zero # error: fixed
+  check -zeroEmpty == zero # error: fixed
+
+  # proc unsignedAdditionInt(a: var BigInt, b: BigInt, c: int32)
+  check zeroEmpty + 1.int32 == one # fixed: fatal[IndexError] in bigints.nim(181) unsignedAdditionInt
+  check -zeroEmpty + 1.int32 == one # fixed: fatal[IndexError] in bigints.nim(245) unsignedSubtractionInt
+  check zeroEmpty + (-1).int32 == -one # fixed: fatal[IndexError] in bigints.nim(245) unsignedSubtractionInt
+  check -zeroEmpty + (-1).int32 == -one # fixed: fatal[IndexError] in bigints.nim(181) unsignedAdditionInt
+
+  # proc unsignedAddition(a: var BigInt, b, c: BigInt)
+  check zeroEmpty + one == one # ok
+  check one + zeroEmpty == one # ok
+  check -zeroEmpty + one == one # ok
+  check one + -zeroEmpty == one # ok
+  check zeroEmpty + zeroEmpty == zero # ok
+  check -zeroEmpty + zeroEmpty == zero # ok
+  check -zeroEmpty + -zeroEmpty == zero # ok
+  check zeroEmpty + -zeroEmpty == zero # ok
+  check bigOne + zeroEmpty == bigOne # ok
+  check bigOne + -zeroEmpty == bigOne # ok
+  check zeroEmpty + bigOne == bigOne # ok
+  check -zeroEmpty + bigOne == bigOne # ok
+  check -bigOne + zeroEmpty == -bigOne # ok
+  check -bigOne + -zeroEmpty == -bigOne # ok
+  check zeroEmpty + -bigOne == -bigOne # ok
+  check -zeroEmpty + -bigOne == -bigOne # ok
+
+  # proc unsignedSubtractionInt(a: var BigInt, b: BigInt, c: int32)
+  check zeroEmpty - 1.int32 == -one # fixed: fatal[IndexError] in bigints.nim(245) unsignedSubtractionInt
+  check -zeroEmpty - 1.int32 == -one # fixed: fatal[IndexError] in bigints.nim(181) unsignedAdditionInt
+  check zeroEmpty - (-1).int32 == one # fixed: fatal[IndexError] in bigints.nim(181) unsignedAdditionInt
+  check -zeroEmpty - (-1).int32 == one # fixed: fatal[IndexError] in bigints.nim(245) unsignedSubtractionInt
+
+  # proc unsignedSubtraction(a: var BigInt, b, c: BigInt)
+  check zeroEmpty - one == -one # ok
+  check one - zeroEmpty == one # ok
+  check -zeroEmpty - one == -one # ok
+  check one - -zeroEmpty == one # ok
+  check zeroEmpty - zeroEmpty == zero # ok
+  check -zeroEmpty - zeroEmpty == zero # ok
+  check -zeroEmpty - -zeroEmpty == zero # ok
+  check zeroEmpty - -zeroEmpty == zero # ok
+  check bigOne - zeroEmpty == bigOne # ok
+  check bigOne - -zeroEmpty == bigOne # ok
+  check zeroEmpty - bigOne == -bigOne # ok
+  check -zeroEmpty - bigOne == -bigOne # ok
+  check -bigOne - zeroEmpty == -bigOne # ok
+  check -bigOne - -zeroEmpty == -bigOne # ok
+  check zeroEmpty - -bigOne == bigOne # ok
+  check -zeroEmpty - -bigOne == bigOne # ok
+
+  # multiplication
+  check zeroEmpty * 1.int32 == zero
+  check -zeroEmpty * 1.int32 == zero
+  check zeroEmpty * one == zero
+  check -zeroEmpty * one == zero
+  check one * zeroEmpty == zero
+  check one * -zeroEmpty == zero
+
+  # https://github.com/def-/nim-bigints/issues/26
+  block:
+    var
+      a: BigInt
+      b: BigInt = 12.initBigInt
+
+    check a*b == 0
+  
+  # division does not have issues, but let's add some checks
+  check zeroEmpty div one == zero
+  check -zeroEmpty div one == zero
+  check zeroEmpty mod one == zero
+  check -zeroEmpty mod one == zero
+
+  check zeroEmpty div 1.int32 == zero
+  check -zeroEmpty div 1.int32 == zero
+  check zeroEmpty mod 1.int32 == zero
+  check -zeroEmpty mod 1.int32 == zero