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

--- Comment #55 from github-bugzilla@puremagic.com ---
Commit pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/8890678458e775e93ade1de307a417b06032ee0f Merge pull request #799 from lionello/bug259

Fixed some signed/unsigned warnings in druntime

--

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

Marc Schütz <schuetzm@gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schuetzm@gmx.net

--

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

--- Comment #56 from Andrei Alexandrescu <andrei@erdani.com> ---
What's left to do about this?

--

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

--- Comment #57 from Lionello Lunesu <lio+bugzilla@lunesu.com> ---
(In reply to Andrei Alexandrescu from comment #56)
> What's left to do about this?

I need to finish some of the static code analysis that I have been adding so we get less false positives. It's the false positives that will turn Walter (and others) off and prevent merging it in.

--

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

Dominikus Dittes Scherkl <dominikus@scherkl.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dominikus@scherkl.de

--- Comment #58 from Dominikus Dittes Scherkl <dominikus@scherkl.de> ---
The problem is still there, and the behaviour is completely inconsistent, so braking any code isn't a problem I think because I cannot imagine that anybody really relies on the strange behaviour:

unittest
{
    byte   a = -3;
    ubyte  b =  2;
    short  c = -3;
    ushort d =  2;
    int    e = -3;
    uint   f =  2;
    long   g = -3;
    ulong  h =  2;

    assert(a < b);
    assert(c < d);
    assert(e < f); // fails!!
    assert(g < h); // fails!!
    assert(a < h); // fails!!
    assert(b > g);
    assert(d > e);
}

So why don't we change to something that simply always works?

int opCmp(T, U)(const(T) a, const(U) b) pure @safe @nogc nothrow
   if(is(Unqual!T == Unqual!U) || isFloatingPoint!T || isFloatingPoint!U)
{
   // Should be buildin. Naive implementation:
   return a <= b ? a != b ? -1 : 0 : 1;
}

/// Returns negative value if a < b, 0 if they are equal or positive value if a
> b.
/// This will always yield a correct result, no matter which integral types are
compared.
/// It uses one extra comparison operation if and only if
/// one type is signed and the other unsigned but has bigger max.
int opCmp(T, U)(const(T) a, const(U) b) pure @safe @nogc nothrow
   if(isIntegral!T && isIntegral!U && !is(Unqual!T == Unqual!U))
{
   static if(T.sizeof == U.sizeof) alias C = Unsigned!T;
   else alias C = CommonType!(T, U); // this will be the larger type

   static if(isSigned!T && isUnsigned!U && T.sizeof <= U.sizeof)
   {
      return (a < 0) ? -1 : opCmp(cast(Unsigned!C)a, cast(C)b);
   }
   else static if(isUnsigned!T && isSigned!U && T.sizeof >= U.sizeof)
   {
      return (b < 0) ? 1 : opCmp(cast(C)a, cast(Unsigned!C)b);
   }
   else
   {
      // both signed or both unsigned or the unsigned type is smaller
      // and can therefore be safely cast to the signed type
      return opCmp(cast(C)a, cast(C)b);
   }
}

--

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

--- Comment #59 from Lionello Lunesu <lio+bugzilla@lunesu.com> ---
It's currently using the C integer promotion rules, which are consistent (they're rules after all) but far from simple.

--

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

--- Comment #60 from Dominikus Dittes Scherkl <dominikus@scherkl.de> ---
(In reply to Lionello Lunesu from comment #59)
> It's currently using the C integer promotion rules, which are consistent (they're rules after all) but far from simple.

Ah, ok. I see why a<b and c<d work - they are all promoted to int.
But never the less: who would be affected by changing the behaviour for int and
long? Is really anybody relying on that? And sure that was not a bug from the
beginning?
I don't think that we really should keep bugs just to be consistend with C.
--> this is a case where a compiler warning is good: "Comparing signed with
unsigned values. This works in D but may be a performance issue and behaves
different from C. Is this intended?" (of course works only after the fix :-)

--

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

--- Comment #61 from Stewart Gordon <smjg@iname.com> ---
(In reply to Dominikus Dittes Scherkl from comment #58)
> The problem is still there, and the behaviour is completely inconsistent, so braking any code isn't a problem I think because I cannot imagine that anybody really relies on the strange behaviour:

Exactly, because it won't break any code.  It will cause code that is already broken to correctly error.

--

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

naptime <naptimeentertainment@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |naptimeentertainment@gmail.
                   |                            |com

--

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

Andrei Alexandrescu <andrei@erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|2.059                       |---
            Version|D1 & D2                     |D2

--