bigints
bigints copied to clipboard
nim-lang
Compound assignment operators shouldn't use `template`s
Currently, e.g. += is defined as
template `+=`*(a: var BigInt, b: BigInt) =
a = a + b
This causes a to be evaluated twice, which can produce unexpected results, when it's an expression that can cause side effects. For example
proc lol(x: var BigInt): var BigInt =
echo "hello ", x
x
var a = 42.initBigInt
lol(a) += 1.initBigInt
prints hello 42 twice, instead of just once.
The solution would be to just make it a func instead. The same applies to -= and *=.
As a further point, += etc. in the standard library are also procs/funcs.
I tried to implement this, but changing += breaks a test. This is possibly caused by https://github.com/nim-lang/Nim/issues/19464.
Could you tell us which test is broken by this change ?
The first "off by one in division" test case in tests/tbugs.nim. You can also check this for yourself by simply making += a func.
This is indeed strange, it is off by one like in issue #5 (but one greater than expected result), and only for the static call. Otherwise I didn't encounter any other bugs. We might have a look at the changes of the PR #6 that should have fixed issue #5.
@konsumlamm Isn't this issue now just a matter of changing template to proc with a :%s/template ′(\c)=′/proc ′$1=′/g (Vim approximative sed regexp to change all template of compound assigments into procs).
That fixes the issue for 1.6, but not for 1.4. So there are several possibilities for how to proceed:
- drop 1.4 support
- accept that compound assignment operators are buggy for 1.4 (either by keeping them as
templates or by changing them tofuncs too) - this should only be a temporary solution imo - reimplement compound assignments to not copy the first argument (and possibly reimplement
+asresult = a; result += b) - I think this is the best option in the long run