www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Error on negating unsigned types

reply Johan Engelen <j j.nl> writes:
The Weka folks would like to see a compile error on negating 
unsigned types:
```
uint total = 0;
void add(int x) { total += x; }
ubyte popCount() { return 5; }

add(popCount());
add(-popCount());
writeln(total); // <-- what does this print? (behavior is 
different from C)
```

After adding the error, a few places in Phobos error out.
For example:
``` (std.format.formatIntegral)
     T arg = val;

     immutable negative = (base == 10 && arg < 0);
     if (negative)
     {
         arg = -arg;
     }
```

``` (std.conv.toTextRange)
     T value;

     bool negative = value < 0;
     Unqual!(Unsigned!T) v = negative ? -value : value;
```

So, adding the error may be nice, but it would make generic code 
a little more verbose.
Ideas? People OK with that?

cheers,
   Johan
Jul 11 2017
next sibling parent reply Johan Engelen <j j.nl> writes:
On Tuesday, 11 July 2017 at 19:46:00 UTC, Johan Engelen wrote:

 The Weka folks would like to see a compile error on negating 
 unsigned types:


Also this nice hackery would need a workaround:
```
    if ((y&(-y))==y)
```
Jul 11 2017
next sibling parent reply Johan Engelen <j j.nl> writes:
On Tuesday, 11 July 2017 at 19:57:06 UTC, Johan Engelen wrote:

 On Tuesday, 11 July 2017 at 19:46:00 UTC, Johan Engelen wrote:

 The Weka folks would like to see a compile error on negating 
 unsigned types:


 Also this nice hackery would need a workaround:
 ```
    if ((y&(-y))==y)
 ```


http://www.exploringbinary.com/ten-ways-to-check-if-an-integer-is-a-power-of-two-in-c/

"The two’s complement of x is computed with ~x + 1, which inverts 
the bits of x and adds 1 (~x + 1 is equivalent to -x, but 
negation is technically illegal for an unsigned integer)."
Jul 11 2017
parent reply Johan Engelen <j j.nl> writes:
On Tuesday, 11 July 2017 at 20:02:07 UTC, Johan Engelen wrote:

 On Tuesday, 11 July 2017 at 19:57:06 UTC, Johan Engelen wrote:

 On Tuesday, 11 July 2017 at 19:46:00 UTC, Johan Engelen wrote:

 The Weka folks would like to see a compile error on negating 
 unsigned types:


 Also this nice hackery would need a workaround:
 ```
    if ((y&(-y))==y)
 ```


 http://www.exploringbinary.com/ten-ways-to-check-if-an-integer-is-a-power-of-two-in-c/

 "The two’s complement of x is computed with ~x + 1, which 
 inverts the bits of x and adds 1 (~x + 1 is equivalent to -x, 
 but negation is technically illegal for an unsigned integer)."


Codegen is the same for both: https://godbolt.org/g/JiHiEe
So Phobos's `if ((y&(-y))==y)` can be rewritten to `if 
((y&(~y+1))==y)` without consequence.
Jul 11 2017
parent Guillaume Chatelet <chatelet.guillaume gmail.com> writes:
On Tuesday, 11 July 2017 at 20:05:43 UTC, Johan Engelen wrote:

 On Tuesday, 11 July 2017 at 20:02:07 UTC, Johan Engelen wrote:

 On Tuesday, 11 July 2017 at 19:57:06 UTC, Johan Engelen wrote:

 On Tuesday, 11 July 2017 at 19:46:00 UTC, Johan Engelen wrote:

 [...]


 Also this nice hackery would need a workaround:
 ```
    if ((y&(-y))==y)
 ```


 http://www.exploringbinary.com/ten-ways-to-check-if-an-integer-is-a-power-of-two-in-c/

 "The two’s complement of x is computed with ~x + 1, which 
 inverts the bits of x and adds 1 (~x + 1 is equivalent to -x, 
 but negation is technically illegal for an unsigned integer)."


 Codegen is the same for both: https://godbolt.org/g/JiHiEe
 So Phobos's `if ((y&(-y))==y)` can be rewritten to `if 
 ((y&(~y+1))==y)` without consequence.


Interestingly returning a bool saves one register and a XOR.
Jul 11 2017
prev sibling parent Daniel Kozak via Digitalmars-d <digitalmars-d puremagic.com> writes:
I belive this is a bug or it should be fixed and do same as C does.

import std.stdio;

void main()
{
    uint total = 0;
    byte popCount() { return 5; }

    writeln(total + popCount() - popCount());
}

works as expected

On Tue, Jul 11, 2017 at 9:57 PM, Johan Engelen via Digitalmars-d <
digitalmars-d puremagic.com> wrote:


 On Tuesday, 11 July 2017 at 19:46:00 UTC, Johan Engelen wrote:


 The Weka folks would like to see a compile error on negating unsigned
 types:


 Also this nice hackery would need a workaround:
 ```
    if ((y&(-y))==y)
 ```
Jul 11 2017
prev sibling next sibling parent Eyal Lotem <eyal.lotem gmail.com> writes:
On Tuesday, 11 July 2017 at 19:46:00 UTC, Johan Engelen wrote:

 The Weka folks would like to see a compile error on negating 
 unsigned types:


The error would be great!

I've filed a bug yesterday: 
https://issues.dlang.org/show_bug.cgi?id=17633

In it I asked to change the result type, but an error works just 
as well.

Unsigned negation bit us with costly bugs multiple times so far.
Jul 11 2017
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/11/2017 12:46 PM, Johan Engelen wrote:

 uint total = 0;
 void add(int x) { total += x; }
 ubyte popCount() { return 5; }
 
 add(popCount());
 add(-popCount());
 writeln(total); // <-- what does this print? (behavior is different from C)


The behavior should be the same.

https://issues.dlang.org/show_bug.cgi?id=17637
Jul 11 2017
parent ketmar <ketmar ketmar.no-ip.org> writes:
Walter Bright wrote:


 On 7/11/2017 12:46 PM, Johan Engelen wrote:

 uint total = 0;
 void add(int x) { total += x; }
 ubyte popCount() { return 5; }
 add(popCount());
 add(-popCount());
 writeln(total); // <-- what does this print? (behavior is different from 
 C)


 The behavior should be the same.

 https://issues.dlang.org/show_bug.cgi?id=17637


so, you want to change promotion rules? just to clarify the things, how 
*exactly* you want new rules to work, and what will happen with `byte a; a 
= -a;`, and why?
Jul 11 2017
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 07/11/2017 03:46 PM, Johan Engelen wrote:

 So, adding the error may be nice, but it would make generic code a 
 little more verbose.
 Ideas? People OK with that?


A compelling argument is that the rule exposed two bugs in Phobos. -- Andrei
Jul 12 2017
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/12/17 11:00 AM, Andrei Alexandrescu wrote:

 On 07/11/2017 03:46 PM, Johan Engelen wrote:

 So, adding the error may be nice, but it would make generic code a 
 little more verbose.
 Ideas? People OK with that?


 
 A compelling argument is that the rule exposed two bugs in Phobos. -- 
 Andrei
 


Those aren't bugs, they are false negatives (pun?).

``` (std.format.formatIntegral)
     T arg = val;

     immutable negative = (base == 10 && arg < 0);
     if (negative) // will always be false for unsigned types
     {
         arg = -arg;
     }
```

``` (std.conv.toTextRange)
     T value;

     bool negative = value < 0; // also will always be false
     Unqual!(Unsigned!T) v = negative ? -value : value;
```

Not sure if the compiler can figure that out. Which means this may cause 
a bunch of nuisance errors.

-Steve
Jul 12 2017
parent reply Johan Engelen <j j.nl> writes:
On Wednesday, 12 July 2017 at 20:12:03 UTC, Steven Schveighoffer 
wrote:

...
 Which means this may cause a bunch of nuisance errors.


It's a trade-off between nuisance in some cases (the Phobos ones 
can be solved with val = abs(val), or with static if), and 
possibly catching bugs in other cases.

We can compare this with negation of a bool and subtracting a 
bool:
```
bool b;
auto x = 1 - b; // allowed
auto y = -b; // not allowed
```
Jul 12 2017
parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/12/17 5:24 PM, Johan Engelen wrote:

 On Wednesday, 12 July 2017 at 20:12:03 UTC, Steven Schveighoffer wrote:

 ...
 Which means this may cause a bunch of nuisance errors.


 
 It's a trade-off between nuisance in some cases (the Phobos ones can be 
 solved with val = abs(val), or with static if), and possibly catching 
 bugs in other cases.
 
 We can compare this with negation of a bool and subtracting a bool:
 ```
 bool b;
 auto x = 1 - b; // allowed
 auto y = -b; // not allowed
 ```
 


My only point is with Andrei's assertion that it "exposed two bugs in 
Phobos".

In terms of the trade-off, it looks like this is just a straight bug 
that should be fixed. If the behavior goes back to being like C, is it 
still so bad that it needs an error?

My testing:

import std.stdio;
void main()
{
     ubyte x = 5;
     int y = -x;
     writeln(y);
     uint t = 0;
     t += x;
     writeln(t);
     t += y;
     writeln(t);
}

outputs:
251
5
256

In C:

#include <stdio.h>
int main()
{
     unsigned char x = 5;
     int y = -x;
     printf("%d\n", y);
     unsigned int t = 0;
     t += x;
     printf("%u\n", t);
     t += y;
     printf("%u\n", t);
     return 0;
}

outputs:
-5
5
0

The C behavior seems fine to me.

-Steve
Jul 13 2017
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 7/11/2017 12:46 PM, Johan Engelen wrote:

 So, adding the error may be nice, but it would make generic code a little more 
 verbose.


The particular issue you were having appears to be a bug in the compiler (I 
already filed it as a bug report). Being a bug, we need more evidence that 
adding an error for -u is compelling.

There really isn't a comprehensive solution for mistakes using unsigned. Some 
languages deal with the issue by not having an unsigned type at all (Java). At 
some level, it's necessary to just be aware of different integer sizes,
integral 
promotion rules, what happens with overflows, and sign.
Jul 12 2017