Compound assignment operators shouldn't use templates

[konsumlamm]

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 *=.

[konsumlamm]

As a further point, += etc. in the standard library are also procs/funcs.

[konsumlamm]

I tried to implement this, but changing += breaks a test. This is possibly caused by nim-lang/Nim#19464.

[dlesnoff]

Could you tell us which test is broken by this change ?

[konsumlamm]

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.

[dlesnoff]

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.

[ringabout]

update: nim-lang/Nim#19464 is fixed

[dlesnoff]

@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).

[konsumlamm]

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 to funcs too) - this should only be a temporary solution imo
• reimplement compound assignments to not copy the first argument (and possibly reimplement + as result = a; result += b) - I think this is the best option in the long run