Thread overview
[Issue 8011] New: BigInt ++ and -- do wrong thing on negative numbers
May 02, 2012
Ary Borenszweig
May 02, 2012
Ary Borenszweig
May 03, 2012
Don
May 01, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8011

           Summary: BigInt ++ and -- do wrong thing on negative numbers
           Product: D
           Version: D2
          Platform: x86_64
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: c.m.brandenburg@gmail.com


--- Comment #0 from c.m.brandenburg@gmail.com 2012-05-01 11:33:55 PDT ---
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

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 01, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8011


c.m.brandenburg@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |2.059


--- Comment #1 from c.m.brandenburg@gmail.com 2012-05-01 11:37:37 PDT ---
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;
        }
    }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 02, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8011


Ary Borenszweig <ary@esperanto.org.ar> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ary@esperanto.org.ar


--- Comment #3 from Ary Borenszweig <ary@esperanto.org.ar> 2012-05-01 23:57:53 PDT ---
(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;
>         }
>     }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 02, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8011



--- Comment #4 from Ary Borenszweig <ary@esperanto.org.ar> 2012-05-01 23:58:55 PDT ---
"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;
        }
    }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 02, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8011



--- Comment #5 from github-bugzilla@puremagic.com 2012-05-02 16:46:08 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/bd72ae7d36e2fafe3a4c00dc9e4c568366cacb03 Fix issue 8011 BigInt ++ and -- do wrong thing on negative numbers

Patch by Ary Borenszweig

https://github.com/D-Programming-Language/phobos/commit/87dfaa82dbb8ff129b46037830ca7f9845ab6664 Merge pull request #564 from donc/bigint8011

Fix issue 8011 BigInt ++ and -- do wrong thing on negative numbers

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 03, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8011


Don <clugdbug@yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |clugdbug@yahoo.com.au
         Resolution|                            |FIXED


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------