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[] ) {
#       int i = toInt(args[1]);
#       uint u = toUint(args[2]);
#       if (i < u)
#               return 1;
#       else 
#               return 0;
#}

...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[] ) {
 #       int i = toInt(args[1]);
 #       uint u = toUint(args[2]);
 #       if (i < u)
 #               return 1;
 #       else 
 #               return 0;
 #}

 ...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




------- Comment #2 from smjg iname.com  2009-02-22 11:45 -------
*** 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





------- Comment #3 from clugdbug yahoo.com.au  2009-02-23 02:28 -------
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





------- Comment #4 from ddparnell bigpond.com  2009-02-23 02:34 -------
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





------- Comment #5 from 2korden gmail.com  2009-02-23 07:40 -------
(In reply to comment #4)
 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





------- Comment #6 from jarrett.billingsley gmail.com  2009-02-23 07:53 -------
(In reply to comment #5)
 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




--- Comment #7 from Qian Xu <qian.xu funkwerk-itk.com>  2009-07-22 00:31:06 PDT
---
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





--- Comment #8 from Stewart Gordon <smjg iname.com>  2009-07-22 01:17:51 PDT ---
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





--- Comment #9 from Qian Xu <qian.xu funkwerk-itk.com>  2009-07-22 01:34:05 PDT
---
(In reply to comment #8)
 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




--- Comment #10 from Andrei Alexandrescu <andrei metalanguage.com>  2009-07-24
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




--- Comment #11 from Andrei Alexandrescu <andrei metalanguage.com>  2009-08-24
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




--- Comment #12 from Ellery Newcomer <ellery-newcomer utulsa.edu>  2009-09-03
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




--- Comment #13 from Don <clugdbug yahoo.com.au>  2009-09-03 08:39:48 PDT ---
(In reply to comment #12)
 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





--- Comment #14 from Ellery Newcomer <ellery-newcomer utulsa.edu>  2009-09-03
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



--- Comment #15 from Don <clugdbug yahoo.com.au> 2009-09-08 09:40:30 PDT ---
(In reply to comment #14)
 Okay, so what I have is it checks for 

 
 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



--- Comment #16 from Ellery Newcomer <ellery-newcomer utulsa.edu> 2009-09-08
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


--- Comment #17 from Witold Baryluk <baryluk smp.if.uj.edu.pl> 2009-11-18
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


--- Comment #18 from Andrei Alexandrescu <andrei metalanguage.com> 2010-11-26
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 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


--- Comment #19 from Éric Estièvenart <eric.estievenart free.fr> 2011-01-14
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