Bug 8011 – BigInt ++ and -- do wrong thing on negative numbers
Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P2
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2012-05-01T11:33:00Z
Last change time
2015-06-09T04:41:31Z
Assigned to
nobody
Creator
c.m.brandenburg
Comments
Comment #0 by c.m.brandenburg — 2012-05-01T11:33:55Z
The ++ and -- operators for BigInt do the wrong thing when operating on a negative number. Specifically, ++ decrements and -- increments—opposite of what it should be.
As an example, take the following program.
import std.bigint;
import std.stdio;
void main() {
BigInt x;
x = 1;
writeln(++x);
writeln(++x);
x = -3;
writeln(++x);
writeln(++x);
x = 1;
writeln(--x);
writeln(--x);
writeln(--x);
writeln(--x);
}
It outputs the following:
2
3
-4
-5
0
-1
0
-1
It _should_ output the following:
2
3
-2
-1
0
-1
-2
-3
Comment #1 by c.m.brandenburg — 2012-05-01T11:37:37Z
The root cause is in bigint.d, in the opUnary() function. BigInt uses an underlying BigUint and maps the ++ and -- operators directly through to the BigUint as addition and subtraction, respectively, disregarding the BigInt's sign. However, this is wrong. Signed ++ and -- must be passed through as subtraction and addition, respectively.
I modified the function by commenting out the two errant lines and replacing them each with a correct one. This is a proof-of-concept fix for the bug.
// non-const unary operations
BigInt opUnary(string op)() if (op=="++" || op=="--")
{
static if (op=="++")
{
//data = BigUint.addOrSubInt(data, 1UL, false, sign);
data = BigUint.addOrSubInt(data, 1UL, sign ? true : false, sign);
return this;
}
else static if (op=="--")
{
//data = BigUint.addOrSubInt(data, 1UL, true, sign);
data = BigUint.addOrSubInt(data, 1UL, sign ? false : true, sign);
return this;
}
}
Comment #2 by lovelydear — 2012-05-01T14:13:57Z
As it seems that you propose fixes, and that nobody seems to be around to have a look at them, maybe you could do a pull request ?
Thx anyway for your efforts, they didn't go unnoticed.
Comment #3 by ary — 2012-05-01T23:57:53Z
(In reply to comment #1)
> The root cause is in bigint.d, in the opUnary() function. BigInt uses an
> underlying BigUint and maps the ++ and -- operators directly through to the
> BigUint as addition and subtraction, respectively, disregarding the BigInt's
> sign. However, this is wrong. Signed ++ and -- must be passed through as
> subtraction and addition, respectively.
>
> I modified the function by commenting out the two errant lines and replacing
> them each with a correct one. This is a proof-of-concept fix for the bug.
>
> // non-const unary operations
> BigInt opUnary(string op)() if (op=="++" || op=="--")
> {
> static if (op=="++")
> {
> //data = BigUint.addOrSubInt(data, 1UL, false, sign);
> data = BigUint.addOrSubInt(data, 1UL, sign ? true : false, sign);
> return this;
> }
> else static if (op=="--")
> {
> //data = BigUint.addOrSubInt(data, 1UL, true, sign);
> data = BigUint.addOrSubInt(data, 1UL, sign ? false : true, sign);
> return this;
> }
> }
Comment #4 by ary — 2012-05-01T23:58:55Z
"sign" is already a boolean, so it's simpler to do:
// non-const unary operations
BigInt opUnary(string op)() if (op=="++" || op=="--")
{
static if (op=="++")
{
data = BigUint.addOrSubInt(data, 1UL, sign, sign);
return this;
}
else static if (op=="--")
{
data = BigUint.addOrSubInt(data, 1UL, !sign, sign);
return this;
}
}
Comment #5 by github-bugzilla — 2012-05-02T16:46:08Z