fix: high() and low() when number of bits is not a power of 2 (#172)

Functions high() and low() were not clearing the most
significant bits of the last limb, leading to values
that were outside of the range of the integer.
Additionally, for signed integers an an incorrect mask
was used to set/clear the most-significant bit.

* chore: cleanup function signatures

Addresses review comments by @moigagoo

Author: markspanbroek

stint/intops.nim

@@ -17,32 +17,40 @@ import
 
 export StInt
 
-const
-  signMask = 1.Word shl (WordBitWidth - 1)
-  clearSignMask = not signMask
-
 # Signedness
 # --------------------------------------------------------
 {.push raises: [], inline, noinit, gcsafe.}
 
+func signMask(T: typedesc[StInt]): Word {.compileTime.} =
+  let position = T.bits - (T.bits.wordsRequired-1) * WordBitWidth - 1
+  Word(1) shl position
+
+func clearSignMask(T: typedesc[StInt]): Word {.compileTime.} =
+  not signMask(T)
+
 func sign*(a: StInt): int =
   ## get the sign of `a`
   ## either -1, 0, or 1
+  const mask = signMask(typeof(a))
   if a.impl.isZero: return 0
-  if a.limbs[^1] < signMask: 1
+  if a.limbs[^1] < mask: 1
   else: -1
 
 func isNegative*(a: StInt): bool =
-  a.limbs[^1] >= signMask
+  const mask = signMask(typeof(a))
+  a.limbs[^1] >= mask
 
 func isPositive*(a: StInt): bool =
-  a.limbs[^1] < signMask
+  const mask = signMask(typeof(a))
+  a.limbs[^1] < mask
 
 func clearMSB(a: var StInt) =
-  a.limbs[^1] = a.limbs[^1] and clearSignMask
+  const mask = clearSignMask(typeof(a))
+  a.limbs[^1] = a.limbs[^1] and mask
 
 func setMSB(a: var StInt) =
-  a.limbs[^1] = a.limbs[^1] or signMask
+  const mask = signMask(typeof(a))
+  a.limbs[^1] = a.limbs[^1] or mask
 
 func negate*(a: var StInt) =
   ## two complement negation
@@ -94,6 +102,7 @@ func high*[bits](_: typedesc[StInt[bits]]): StInt[bits] =
   # so we only have to unset the most significant bit.
   for i in 0 ..< result.limbs.len:
     result[i] = high(Word)
+  result.clearExtraBitsOverMSB()
   result.clearMSB
 
 func low*[bits](_: typedesc[StInt[bits]]): StInt[bits] =

stint/private/datatypes.nim

@@ -84,7 +84,7 @@ template len*(a: StUint): int =
 
 {.push raises: [], inline, noinit, gcsafe.}
 
-template clearExtraBitsOverMSB*(a: var StUint) =
+template clearExtraBitsOverMSB*(a: var StUint | StInt) =
   ## A Stuint is stored in an array of 32 of 64-bit word
   ## If we do bit manipulation at the word level,
   ## for example a 8-bit stuint stored in a 64-bit word

stint/uintops.nim

@@ -49,6 +49,7 @@ func one*[bits: static[int]](T: typedesc[StUint[bits]]): T {.inline.} =
 func high*[bits](_: typedesc[StUint[bits]]): StUint[bits] {.inline.} =
   for i in 0 ..< result.limbs.len:
     result[i] = high(Word)
+  result.clearExtraBitsOverMSB()
 
 func low*[bits](_: typedesc[StUint[bits]]): StUint[bits] {.inline.} =
   result.setZero

tests/test_bugfix.nim

@@ -18,3 +18,16 @@ suite "various bugfix":
 
     expect(AssertionDefect):
       discard "0bcdef12345".parse(UInt128, 10)
+
+  test "high and low when number of bits is not a power of 2":
+    check StUint[24].high.stuint(128) == 2.u128.pow(24) - 1
+    check StUint[40].high.stuint(128) == 2.u128.pow(40) - 1
+    check StUint[96].high.stuint(128) == 2.u128.pow(96) - 1
+
+    check StInt[24].high.stint(128) == 2.i128.pow(23) - 1.i128
+    check StInt[40].high.stint(128) == 2.i128.pow(39) - 1.i128
+    check StInt[96].high.stint(128) == 2.i128.pow(95) - 1.i128
+
+    check StInt[24].low.stint(128) == - 2.i128.pow(23)
+    check StInt[40].low.stint(128) == - 2.i128.pow(39)
+    check StInt[96].low.stint(128) == - 2.i128.pow(95)