www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 8011] New: BigInt ++ and -- do wrong thing on negative numbers

reply d-bugmail puremagic.com writes:
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
next sibling parent d-bugmail puremagic.com writes:
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 01 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
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 01 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
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 01 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
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 02 2012
prev sibling parent d-bugmail puremagic.com writes:
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: -------
May 02 2012