Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 76 additions & 3 deletions src/bigints.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
##
## The bitwise operations behave as if negative numbers were represented in 2's complement.

import std/[algorithm, bitops, math, options]
import std/[algorithm, bitops, endians, math, options]

type
BigInt* = object
Expand All @@ -11,7 +11,7 @@ type
# * if `a` is non-zero: `a.limbs[a.limbs.high] != 0`
# * if `a` is zero: `a.limbs.len <= 1`
limbs: seq[uint32]
isNegative: bool
isNegative*: bool


# forward declarations
Expand Down Expand Up @@ -78,7 +78,7 @@ const
zero = initBigInt(0)
one = initBigInt(1)

func isZero(a: BigInt): bool {.inline.} =
func isZero*(a: BigInt): bool {.inline.} =
a.limbs.len == 0 or (a.limbs.len == 1 and a.limbs[0] == 0)

func abs*(a: BigInt): BigInt =
Expand Down Expand Up @@ -1309,3 +1309,76 @@ func powmod*(base, exponent, modulus: BigInt): BigInt =
result = (result * basePow) mod modulus
basePow = (basePow * basePow) mod modulus
exponent = exponent shr 1

proc toBytes*(a: BigInt; endianness = system.cpuEndian): seq[byte] =
## Convert a `BigInt` to a byte-sequence.
## The byte-sequence is the absolute (positive) value of `a`, it *does not* contain a sign-bit.
runnableExamples:
let n = initBigInt("18591708106338011146")
let buf = n.toBytes(bigEndian)
doAssert buf == @[0x1'u8,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0xA]
Comment on lines +1316 to +1319
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
runnableExamples:
let n = initBigInt("18591708106338011146")
let buf = n.toBytes(bigEndian)
doAssert buf == @[0x1'u8,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0xA]
runnableExamples:
let n = initBigInt("18591708106338011146")
let buf = n.toBytes(bigEndian)
doAssert buf == @[0x1'u8,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0xA]

if not a.isZero:
result = newSeq[byte](a.limbs.len shl 2)
var i: int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var i: int
var i = 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nim initializes integers to zero.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but imo it's clearer to explicitly initialize them, rather than relying on implicit initialization.

case endianness
of bigEndian:
for s in [24, 16, 8, 0]:
result[i] = uint8(a.limbs[a.limbs.high] shr s)
if result[0] != 0x00: inc(i)
Comment on lines +1325 to +1327
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for s in [24, 16, 8, 0]:
result[i] = uint8(a.limbs[a.limbs.high] shr s)
if result[0] != 0x00: inc(i)
# convert a.limbs[a.limbs.high]
let highLimb = a.limbs[a.limbs.high]
for s in [24, 16, 8, 0]:
result[i] = uint8(highLimb shr s)
if result[0] != 0x00: inc(i)

for l in countdown(a.limbs.high.pred, a.limbs.low):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for l in countdown(a.limbs.high.pred, a.limbs.low):
for l in countdown(a.limbs.high.pred, 0):

low on a seq is always 0. You also used 0 below, so that's consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A low on an array is always 0, a low on an empty seq can be -1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already checked that a is not zero (so a.limbs is not empty), but I don't really care, we can keep it that way.

bigEndian32(addr result[i], unsafeAddr a.limbs[l])
inc(i, 4)
result.setLen(i)
of littleEndian:
for l in 0..a.limbs.high:
littleEndian32(addr result[i], unsafeAddr a.limbs[l])
inc(i, 4)
while result[pred i] == 0x00:
dec i
result.setLen(i)
Comment on lines +1331 to +1338
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result.setLen(i)
of littleEndian:
for l in 0..a.limbs.high:
littleEndian32(addr result[i], unsafeAddr a.limbs[l])
inc(i, 4)
while result[pred i] == 0x00:
dec i
result.setLen(i)
of littleEndian:
for l in 0..a.limbs.high:
littleEndian32(addr result[i], unsafeAddr a.limbs[l])
inc(i, 4)
while result[pred i] == 0x00:
dec i
result.setLen(i)


proc fromBytes*(result: var BigInt; buf: openarray[uint8]; endianness = system.cpuEndian) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
proc fromBytes*(result: var BigInt; buf: openarray[uint8]; endianness = system.cpuEndian) =
proc fromBytes*(buffer: openarray[uint8]; endianness = system.cpuEndian): BigInt =

This would be more ergonomic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the var variant for already allocated big integers. We generally read from memory at initialisation though, so why not.

## Convert a byte-sequence to `BigInt` value.
## The input `buf` is only interpreted as a natural (positive) number.
runnableExamples:
var n: BigInt
n.fromBytes([0x1'u8,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0xA], bigEndian)
n = -n
doAssert n == initBigInt("-18591708106338011146")
Comment on lines +1341 to +1347
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## Convert a byte-sequence to `BigInt` value.
## The input `buf` is only interpreted as a natural (positive) number.
runnableExamples:
var n: BigInt
n.fromBytes([0x1'u8,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0xA], bigEndian)
n = -n
doAssert n == initBigInt("-18591708106338011146")
## Convert a byte-sequence to a `BigInt` value.
## The input `buffer` is interpreted as a non-negative number.
runnableExamples:
var n = -fromBytes([0x1'u8,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0xA], bigEndian)
doAssert n == initBigInt("-18591708106338011146")

result.limbs.setLen((buf.len + 3) shr 2)
case endianness
of bigEndian:
var
li = result.limbs.high
bi = buf.low
block:
var
limb: uint32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
limb: uint32
limb = 0'u32

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nim initializes uint32 to zero.

j = 4 - (buf.len and 3)
while j < 4:
limb = (limb shl 8) or buf[bi].uint32
inc(bi)
inc(j)
Comment on lines +1355 to +1361
Copy link
Contributor

@konsumlamm konsumlamm Jul 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var
limb: uint32
j = 4 - (buf.len and 3)
while j < 4:
limb = (limb shl 8) or buf[bi].uint32
inc(bi)
inc(j)
var limb = 0'u32
for _ in 1..(buf.len and 0b11):
limb = (limb shl 8) or buf[bi].uint32
inc(bi)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(buf.len and 3) should be (buf.len and 0b11).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, edited.

if bi > 0:
result.limbs[li] = limb
dec(li)
while li >= 0:
bigEndian32(addr result.limbs[li], unsafeAddr buf[bi])
inc(bi, 4)
dec(li)
of littleEndian:
var
li = result.limbs.low
bi = buf.low
while (bi + 4) < buf.len:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
while (bi + 4) < buf.len:
while (bi + 3) < buf.len:

littleEndian32 uses buf[bi], buf[bi + 1], buf[bi + 2] and buf[bi + 3], not buf[bi + 4].

littleEndian32(addr result.limbs[li], unsafeAddr buf[bi])
inc(bi, 4)
inc(li)
if bi < buf.len:
var
limb: uint32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
limb: uint32
limb = 0'u32

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nim initializes uint32 to zero.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit is better than implicit. For integers, I may understand since we implicitly rely on the litteral 0 being an integer.

ji = buf.high
while ji >= bi:
limb = (limb shl 8) or buf[ji]
dec(ji)
result.limbs[li] = limb
File renamed without changes.
18 changes: 17 additions & 1 deletion tests/tbigints.nim
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ proc main() =
when (NimMajor, NimMinor) >= (1, 5):
block: # literals
# workaround
include tliterals
include ./literals
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why rename this specific file and not the others? Since we overwrite the default nimble test task, we shouldn't need the t prefix anymore. Either way, this is unrelated to the rest of the PR, so please make a new PR for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok then.


block: # zero
# see https://github.com/nim-lang/bigints/issues/26
Expand Down Expand Up @@ -880,6 +880,22 @@ proc main() =
doAssert pred(a, 3) == initBigInt(4)
doAssert succ(a, 3) == initBigInt(10)

when nimvm: discard
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this work in a static context?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Endianness.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean std/endians is not available? I see.

else:
let n = initBigInt("18591708106338011146")
block:
let buf = n.toBytes(bigEndian)
doAssert buf == @[ 0x1'u8, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0xA]
var res: Bigint
res.fromBytes(buf, bigEndian)
doAssert res == n
block:
let buf = n.toBytes(littleEndian)
doAssert buf == @[ 0xA'u8, 0x8, 0x7, 0x6, 0x5, 0x4, 0x3, 0x2, 0x1 ]
var res: Bigint
res.fromBytes(buf, littleEndian)
doAssert res == n


static: main()
main()
9 changes: 9 additions & 0 deletions tests/trandom.nim
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,12 @@ block: # check uniformity
doAssert(trials/nbuckets*0.5 < float(x))
doAssert(float(x) < trials/nbuckets*1.5)

block: # check serialization roundtrip
const
trials = 1024
var a, b: Bigint
for x in 0..trials:
a = rand(0.initBigInt..pow(2.initBigInt, x))
for endian in [bigEndian, littleEndian]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for endian in [bigEndian, littleEndian]:
for endian in [bigEndian, littleEndian]:

bigEndian and littleEndian aren't in scope here. I wonder why the CI didn't run...

b.fromBytes(a.toBytes(endian), endian)
doAssert a == b