www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 259] New: Comparing signed to unsigned does not generate an error

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259

           Summary: Comparing signed to unsigned does not generate an error
           Product: D
           Version: 0.163
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Keywords: spec
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: lio lunesu.com


From the book of Bright,
http://www.digitalmars.com/d/expression.html#RelExpression

"It is an error to have one operand be signed and the other unsigned for a <,
<=, > or >= expression. Use casts to make both operands signed or both operands
unsigned."

...yet...

#import std.conv;
#int main( char[] args[] ) {








...compiled with "dmd", "dmd -debug" or "dmd -release" does not give an error
message, neither at compile time, nor at run time.


--
Jul 20 2006
next sibling parent Thomas Kuehne <thomas-dloop kuehne.cn> writes:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

d-bugmail puremagic.com schrieb am 2006-07-20:

 http://d.puremagic.com/issues/show_bug.cgi?id=259



 From the book of Bright,
 http://www.digitalmars.com/d/expression.html#RelExpression

 "It is an error to have one operand be signed and the other unsigned for a <,
<=, > or >= expression. Use casts to make both operands signed or both operands
 unsigned."

 ...yet...

 #import std.conv;
 #int main( char[] args[] ) {








 ...compiled with "dmd", "dmd -debug" or "dmd -release" does not give an error
 message, neither at compile time, nor at run time.


Added to DStress as
http://dstress.kuehne.cn/nocompile/o/opCmp_08_A.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_B.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_C.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_D.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_E.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_F.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_G.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_H.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_I.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_J.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_K.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_L.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_M.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_N.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_O.d
http://dstress.kuehne.cn/nocompile/o/opCmp_08_P.d

Thomas


-----BEGIN PGP SIGNATURE-----

iD8DBQFE8DkSLK5blCcjpWoRAvd4AJ9n86OSH7vfqYKSxBQHFupEzG9/OgCfasr1
EMOV+he335iGTfu8g/Ff45g=
=v3rD
-----END PGP SIGNATURE-----
Aug 26 2006
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


smjg iname.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |o.dathe gmx.de





*** Bug 2205 has been marked as a duplicate of this bug. ***


--
Feb 22 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






Note that it's critical than any fix for this bug should not make code like the
following fail to compile:

uint a = 5;
if (a > 2) { ... }

Otherwise the cure would be worse than the disease <g>.


--
Feb 23 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






Even more critical, any fix for this bug should not make code like the
following fail to compile:

uint a = 5;
if (a > -2) { ... }


--
Feb 23 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259








 Even more critical, any fix for this bug should not make code like the
 following fail to compile:
 
 uint a = 5;
 if (a > -2) { ... }
 


Why not? This is obviously a bug!


--
Feb 23 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259








 uint a = 5;
 if (a > -2) { ... }
 


 
 Why not? This is obviously a bug!


Agreed.  I can't tell you how many times I've done something stupid like:

for(uint i = something; i >= 0; i--)
{ /* yay, infinite loop! */ }

A nontrivial condition that always results in an infinite loop should never be
accepted.


--
Feb 23 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Qian Xu <qian.xu funkwerk-itk.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |qian.xu funkwerk-itk.com





---
Is there any official response to this issue? It was reported at 2006, but the
status is still "NEW"

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 22 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






As I look, bug 2006 doesn't seem to have anything to do with this one at all.

But if you find a duplicate of a bug, then mark it as one!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 22 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






---


 As I look, bug 2006 doesn't seem to have anything to do with this one at all.
 
 But if you find a duplicate of a bug, then mark it as one!


sorry, i mean it was reported in year 2006.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 22 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |andrei metalanguage.com
         AssignedTo|nobody puremagic.com        |andrei metalanguage.com





18:08:49 PDT ---
This needs to be fixed. I'm taking ownership (though Walter will do all the
work) in order to not forget. I'll add a unittest to TDPL too. Until then let's
vote this up.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 24 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|andrei metalanguage.com     |bugzilla digitalmars.com
           Severity|normal                      |major





22:17:42 PDT ---
Reassigning this to Walter. Walter, any chance you could please bump the
priority on fixing this one. I am bumping priority to major, it's an important
bug that is getting rather old.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 24 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Ellery Newcomer <ellery-newcomer utulsa.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ellery-newcomer utulsa.edu





08:33:41 PDT ---
This morning I've been tinkering with the DMD source, and I've made an edit to
expression.c that appears to fix this issue. How do I submit?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 03 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au







 This morning I've been tinkering with the DMD source, and I've made an edit to
 expression.c that appears to fix this issue. How do I submit?


Either create a patch using SVN, and attach it, or just post the lines you
changed (Walter doesn't actually use patch files, as far as I can tell). Search
for bugs with keyword 'patch' for examples.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 03 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






13:51:16 PDT ---
Okay, so what I have is it checks for 

signed cmp unsigned 

or vice versa in CmpExp::Semantic just before typeCombine gets called, which
works, but then stuff like

1 < 1u

doesn't. So the idea is 

signed cmp unsigned 

or vice versa is okay if the signed arg is a literal and its value is
nonnegative. 

This should work fine if sizeof(signed arg) =< sizeof(unsigned arg) because the
value of the signed arg is within the range of the unsigned arg, and
typeCombine should be able to expand the type of the signed arg to that of the
unsigned arg or whatever.
It should work if sizeof(signed arg) > sizeof(unsigned arg) because the value
of the unsigned arg is within the range of the signed arg, and typeCombine
should be able to expand the type of the unsigned arg to that of the signed
arg.

I don't know, maybe this should be happening in typeCombine. Insert the
following in expression.c CmpExp::semantic before the line

typeCombine(sc);


    if ( e1->type->isintegral() && e2->type->isintegral()){
        if(e1->type->isunsigned() ^ e2->type->isunsigned()){
            if(!e1->type->isunsigned() &&
                dynamic_cast<IntegerExp*>(e1) &&
                ((sinteger_t) e1->toInteger()) >= 0) goto JustKidding;
            if(!e2->type->isunsigned() &&
                dynamic_cast<IntegerExp*>(e2) &&
                ((sinteger_t) e2->toInteger()) >= 0) goto JustKidding;
            error("comparing signed and unsigned integers");
        }
        JustKidding:;
    }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 03 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






 Okay, so what I have is it checks for 


[snip]

 
 signed cmp unsigned 
 
 or vice versa is okay if the signed arg is a literal and its value is
 nonnegative. 


In D2, it's possible to a lot better than this. With the new D2 rules for
implicit conversion between integral types (range-based), it should be enough
to require that one of the types must implicitly convert to the other. This
doesn't quite work yet, since range-based addition (for example) is not yet
implemented.
This will mean that stuff like:
byte b;
uint u;
b = -67;
if (u < b + 0x100) ...
is OK because although b + 0x100 is signed, it can never be less than 1, so it
can safely be converted to unsigned.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 08 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




10:43:00 PDT ---
Cool. That sounds like a much better solution than the patch I posted.

Concerning my patch, I just realized that comparison with unsigned and zero
generally doesn't make sense either, except maybe

{unsigned} > 0

so now I have

    if ( e1->type->isintegral() && e2->type->isintegral()){
        if(e1->type->isunsigned() ^ e2->type->isunsigned()){
            if(!e1->type->isunsigned() && dynamic_cast<IntegerExp*>(e1)){
                sinteger_t v1 = ((sinteger_t) e1->toInteger());
                if(v1 > 0) goto JustKidding;
                // 0 < uns or 0 !>= uns okay
                else if(v1 == 0 && (op == TOKlt || op == TOKul))
                    goto JustKidding;
            }else if(dynamic_cast<IntegerExp*>(e2)){
                sinteger_t v2 = ((sinteger_t) e2->toInteger());
                if(v2 > 0) goto JustKidding;
                // uns > 0 or uns !<= 0 okay
                else if(v2 == 0 && (op == TOKgt || op == TOKug))
                    goto JustKidding;
            }
            error("comparing signed and unsigned integers");
        }
        JustKidding:;
    }

in case anyone plans to commit it

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 08 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Witold Baryluk <baryluk smp.if.uj.edu.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |baryluk smp.if.uj.edu.pl



13:13:14 PST ---
Hi,

I today found remarkable bug:

int k = -1;
uint n = 7;
assert(k < n);


Code above should execute without proble, but dmd it computing false as value
of (k<n) which absolutly nonsensical. casting n to int, resolves problem

int k = -1;
uint n = 7;
assert(k < cast(T)n);

How it can be so long time not resolved?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 18 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |critical



10:43:45 PST ---
Escalating severity of this dangerous issue. Has 11 votes, too.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 26 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Éric Estièvenart <eric.estievenart free.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |eric.estievenart free.fr



11:55:24 PST ---
Indeed this is rather critical; I hit it in a simple precondition like:
void insert( Object o, int ndx = -1 ) // -1 means last
  in { assert( ndx >= -1 && ndx <= somearray.length ); }

forcing to add a cast everywhere would be very painful...

The following code should compile without error and pass:
void main()
{
  uint ulen = 0;
  int ndx = -1;
  assert( -1 <= ulen );
  assert( ndx <= ulen );
}

If it generates warnings is another issue...

// This one should compile too
void func2()
{
  uint u;
  // should not compile (or at least warn): always yields to true
  static assert( !is( typeof( u >= 0 ) ) );
  static assert( !is( typeof( u >= -1 ) ) );
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 14 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Christian Kamm <kamm-removethis incasoftware.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kamm-removethis incasoftwar
                   |                            |e.de



2011-10-10 08:58:59 PDT ---
See
https://github.com/D-Programming-Language/dmd/pull/119
https://github.com/D-Programming-Language/dmd/pull/444
for two possible fixes.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 10 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs eml.cc



See:

https://github.com/D-Programming-Language/dmd/commit/4536fd5e3102fcea168660e9c4d2a2c80d50e7f5

But druntime and Phobos will probably need some changes.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 24 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259







 But druntime and Phobos will probably need some changes.


Keeping this as a warning in the current compiler, and turn it into an
(deprecated) error in the next DMD version, will probably allow to update
druntime and Phobos.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 24 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




I think Walter is willing to accept that patch if someone improves the patch to
remove some of its false positives.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 25 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




It seems wrong to me that:

if (-1 < 2u) {...}
fails to compile. Both are in the range -int.max .. int.max, so they can safely
be compared using signed comparison.

The problems with mixed sign are when one operand can be in the range
-int.max..0 and the other can be in the range int.max+1u..uint.max. If at least
one of the operands is in the polysemous range 0..int.max, there should be no
problem.

But, the patch as it stands allows a polysemous-ranged int to be compared with
a uint, but does not allow a polysemous-range uint to be compared with an int.
At present the spec doesn't give any reason for unsigned to be treated from
signed.

The problem is the silly C promotion rule that converts both operands to
unsigned (instead, it should generate an error). In any existing C code that
works, it works because the signed operand is in the polysemous range. Of
course there could be code which relies on (-1<2u) being false. But surely such
code is broken.

Do we really need to retain backwards compatibility with broken C code?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 25 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






Thank you Don.


 Of course there could be code which relies on (-1<2u) being false.
 But surely such code is broken.


I don't like this is in D.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 25 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Jesse Phillips <Jesse.K.Phillips+D gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Jesse.K.Phillips+D gmail.co
                   |                            |m
   Target Milestone|---                         |2.059


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 19 2012
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Denis Shelomovskij <verylonglogin.reg gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |verylonglogin.reg gmail.com



2013-03-05 10:44:52 MSK ---
Issue state:


https://github.com/D-Programming-Language/dmd/pull/444
was merged, then #ifdef'd out as it was "too disruptive" in
https://github.com/D-Programming-Language/dmd/commit/52d8c150ecfbe2cad6672f50094a6ff1230e72e3

https://github.com/D-Programming-Language/dmd/pull/1611

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Mar 04 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |WONTFIX



01:27:07 PDT ---
I think we're going to have to give up on this one.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




2013-04-07 20:42:55 MSD ---


 I think we're going to have to give up on this one.


Not being able to at least highlight signed to unsigned comparisons in a
program is a major lack of functionality. IMHO this is why enhancement Issue
9811 matters as it is the place where we can forget about "disruptiveness" and
"is it always and error" questions.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




15:03:49 PDT ---


 I think we're going to have to give up on this one.


This is a bit abrupt. What was the decision process? I just shared an anecdote
about a few major bugs at work caused by this exact behavior.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




15:49:14 PDT ---


 This is a bit abrupt. What was the decision process? I just shared an anecdote
 about a few major bugs at work caused by this exact behavior.


It's been 7 years of discussion now without an answer that works. I don't think
it's abrupt!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259








 This is a bit abrupt. What was the decision process? I just shared an anecdote
 about a few major bugs at work caused by this exact behavior.


 
 It's been 7 years of discussion now without an answer that works. I don't think
 it's abrupt!


How does pull request 119 not work?

And why haven't you written a fix yourself in these 7 years - or even made a
single comment on this bug until now?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




16:50:41 PDT ---


 How does pull request 119 not work?


Follow the links & comments 
https://github.com/D-Programming-Language/dmd/pull/119

If you want to discuss that further, please post there.


 And why haven't you written a fix yourself in these 7 years - or even made a
 single comment on this bug until now?


I've discussed it extensively on the n.g. This isn't the only place it's come
up.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Andrei Alexandrescu <andrei erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|WONTFIX                     |



17:24:25 PDT ---
Reopening. The entire discussion makes it obvious this needs a fix. It's
critical and has 20 votes, probably more than probably any other bug. Closing
this without any explanation is straight the opposite of listening to the
community. Thanks.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




17:50:06 PDT ---
I don't mind if it is reopened. It's just that it's sat at the top of the
"critical" bug list forever with no movement towards a solution. I'd like to
get this resolved one way or another.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




18:38:23 PDT ---
Great. Here's a solution Walter and I just discussed:

Consider a comparison a < b, a <= b, a > b, or a >= b, in which a and b are
integral types of different signedness. Without loss of generality, let's
consider a is signed and b is unsigned and the comparison is a < b. Then we
have the following cases:

1. If a.sizeof > b.sizeof, then a < b is lowered into a < cast(typeof(a)) b.
Then signed comparison proceeds normally. This is a classic value-based
conversion dating from the C days, and we do it in D as well.

2. Otherwise, if a is determined through Value Range Propagation to be greater
than or equal to zero, then a < b is lowered into cast(U) a < b, where U is the
unsigned variant of typeof(a). Then unsigned comparison proceeds normally.

3. Otherwise, the comparison is in error. The error message may recommend using
the std.traits.unsigned function, which executes a size-informed cast.

Walter, if you agree with this resolution please mark this as "preapproved".

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




18:57:27 PDT ---
Preapproved FTW! Who wants to implement this?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Andrei Alexandrescu <andrei erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|bugzilla digitalmars.com    |nobody puremagic.com



19:05:56 PDT ---
Resetting asignee to default. If anyone wants to work on this, please assign it
to yourself!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259







 3. Otherwise, the comparison is in error. The error message may recommend using
 the std.traits.unsigned function, which executes a size-informed cast.


This is the source of unsigned:


auto unsigned(T)(T x) if (isIntegral!T)
{
         static if (is(Unqual!T == byte )) return cast(ubyte ) x;
    else static if (is(Unqual!T == short)) return cast(ushort) x;
    else static if (is(Unqual!T == int  )) return cast(uint  ) x;
    else static if (is(Unqual!T == long )) return cast(ulong ) x;
    else
    {
        static assert(T.min == 0, "Bug in either unsigned or isIntegral");
        return cast(Unqual!T) x;
    }
}


Is it better to use template constraints there?
And isn't it better for unsigned to accept only unsigned Ts?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




07:21:23 PDT ---
Thanks Lionello for taking this over. I thought of one more case this morning
so let me insert in the food chain:

(Recall a is signed and b is unsigned.)

==============

1. If a.sizeof > b.sizeof, then a < b is lowered into a < cast(typeof(a)) b.
Then signed comparison proceeds normally. This is a classic value-based
conversion dating from the C days, and we do it in D as well.

2. Otherwise, if a is determined through Value Range Propagation to be greater
than or equal to zero, then a < b is lowered into cast(U) a < b, where U is the
unsigned variant of typeof(a). Then unsigned comparison proceeds normally.

3. (NEW) Otherwise, if b is determined through Value Range Propagation to be
less than or equal to typeof(b).max / 2, then a < b is lowered into a < cast(S)
b, where S is the signed variant of typeof(b). Then signed comparison proceeds
normally.

4. Otherwise, the comparison is in error. The error message may recommend using
the std.traits.unsigned function, which executes a size-informed cast.

==============

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 08 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




Sounds good but ... given that people have been trying for 7 years without
success to implement the basic rule from the spec, how many more years can we
realistically expect it to be before this new scheme being suggested is in the
compiler?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 08 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




16:14:12 PDT ---


 Sounds good but ... given that people have been trying for 7 years without
 success to implement the basic rule from the spec, how many more years can we
 realistically expect it to be before this new scheme being suggested is in the
 compiler?


Now that we have a preapproved solution, a solid VRP implementation, and a much
expanded contribution base, I can only assume it'll take days. Lionello?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 08 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




03:02:20 PDT ---
Yes, I'll have this done this week.

For point 1: how to cope with the fact that I can safely cast an uint to long,
but can't cast an int to an ulong (without issues)? 

Surely cast(int)-1 < cast(ulong)1 should evaluate to true? But according to
point 1 it would be rewritten as cast(ulong)-1 < cast(ulong)1 which is
0xFFFFFFFFFFFFFFFF < 0x1 and evaluate to false.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 09 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259






 Yes, I'll have this done this week.
 For point 1: how to cope with the fact that I can safely cast an uint to long,
 but can't cast an int to an ulong (without issues)? 
 Surely cast(int)-1 < cast(ulong)1 should evaluate to true? But according to
 point 1 it would be rewritten as cast(ulong)-1 < cast(ulong)1 which is
 0xFFFFFFFFFFFFFFFF < 0x1 and evaluate to false.


No.  Point 1 is the case where a.sizeof > b.sizeof.  This isn't the case here.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 09 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




08:37:50 PDT ---
Thanks Lionello for working on this! It will make D noticeably better at
bread-and-butter work. Regarding your question - what Stewart said. Let me know
if you hit any snag.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 09 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




09:20:22 PDT ---
uint b;
if (b > -2) { ... }

The integral expression -2 doesn't have any size per se, but whole size thing
shouldn't even apply here, since the two expressions have ranges that are
completely disjoint. Points 2 and 3 won't catch this case either and would
cause an error. We could consider all integrals 'long' and apply point 1. Or,
test for disjoint ranges and replace the whole comparison with a constant? (I
have a feeling this optimization might already be happening in a later pass?)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




09:45:57 PDT ---


 uint b;
 if (b > -2) { ... }
 
 The integral expression -2 doesn't have any size per se, but whole size thing
 shouldn't even apply here, since the two expressions have ranges that are
 completely disjoint. Points 2 and 3 won't catch this case either and would
 cause an error. We could consider all integrals 'long' and apply point 1. Or,
 test for disjoint ranges and replace the whole comparison with a constant? (I
 have a feeling this optimization might already be happening in a later pass?)


I'd say we need to make this an error. If we don't (i.e. make the comparison
always true), then we'd silently change behavior.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




19:24:53 PDT ---
https://github.com/D-Programming-Language/dmd/pull/1889

There are many things silently(!) breaking, though, as some templates are not
being chosen because of internal comparison errors.

For example, FormatSpec.width and FormatSpec.precision are ints but often
compared against array lengths (for padding and such). TBH, using negative
width and precision to mean "argument index" is a hack and we'd be better off
changing them to uint and use a flag for the "argument index" case.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 11 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




19:42:37 PDT ---
// For the record: my test cases. Will add/fix existing unittests as well.
import std.traits;
int i;
uint ui;
long l;
ulong ul;
// 0. same-signed-ness
static assert(__traits(compiles, ui>ul));
static assert(__traits(compiles, ul>ui));
static assert(__traits(compiles, i>l));
static assert(__traits(compiles, l>i));
static assert(__traits(compiles, 1>2));
static assert(!(1>2));
static assert(__traits(compiles, 2>1));
static assert(2>1);
// 1. sizeof(signed) > sizeof(unsigned)
static assert(__traits(compiles, l>ui));
static assert(__traits(compiles, ui>l));
static assert(__traits(compiles, -1L>2));
static assert(!(-1L>2));
static assert(__traits(compiles, 2>-1L));
static assert(2>-1L);
// 2. signed.min >= 0
static assert(__traits(compiles, ui>cast(int)2));
static assert(__traits(compiles, cast(int)2>ui));
static assert(__traits(compiles, ul>cast(int)2));
static assert(__traits(compiles, cast(int)2>ul));
// 3. unsigned.max < typeof(unsigned.max/2)
static assert(__traits(compiles, i>cast(uint)2));
static assert(__traits(compiles, cast(uint)2>i));
static assert(__traits(compiles, cast(int)-1>cast(uint)3));
static assert(__traits(compiles, cast(uint)3>cast(int)-1));
static assert(__traits(compiles, -1>2UL));
static assert(!(-1>2UL));
static assert(__traits(compiles, 2UL>-1));
static assert(2UL>-1);
// error
static assert(!__traits(compiles, ul>-2));
static assert(!__traits(compiles, -2>ul));
static assert(!__traits(compiles, i>ul));
static assert(!__traits(compiles, ul>i));
static assert(!__traits(compiles, l>ul));
static assert(!__traits(compiles, ul>l));
static assert(!__traits(compiles, i>ui));
static assert(!__traits(compiles, ui>i));

void main(){}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 11 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




22:01:28 PDT ---
We should probably consider making this a warning or deprecation, instead of an
error. The silent failures within templates make it very hard to fix code.

Warnings and errors during template instantiations are suppressed and the final
"errors instantiating template" doesn't say why (not even with -v; this is
probably worth a filing a separate bug for.) 

I'm having to add a "printf" to expression.c to see those occurrences,
\util\dmd2\src\phobos\std\format.d(1216)
\util\dmd2\src\phobos\std\format.d(1224)
\util\dmd2\src\phobos\std\format.d(1395)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 12 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




14:20:12 PDT ---
I've discovered one additional case,

1b. If both types can be cast to the bigger signed type, the cast is safe

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 03 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259


Nick Treleaven <ntrel-public yahoo.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ntrel-public yahoo.co.uk



03:11:43 PDT ---


 https://github.com/D-Programming-Language/dmd/pull/1889


Update: that was split into two, see:
https://github.com/D-Programming-Language/dmd/pull/1913

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 20 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




12:12:57 PDT ---
Also submitted fixes to druntime:
https://github.com/D-Programming-Language/druntime/pull/601

(Note: check the pull requests for the latest info)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 07 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=259




Commit pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/439d079dce0079cbacaedd91648d85d2c7d660c0


Bug259: fixed signed-unsigned comparisons in druntime

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