www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - This seems like what could be a common cause of bugs

reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
This is just an observation, not a question or anything.

void main()
{
    enum width = 100;
    double step = 1 / width;

    writeln(step);  // 0
}

I've just had this bug in my code. I forgot to make either width or 1
a floating-point type. IOW, I didn't do this:

void main()
{
    enum width = 100.0;
    double step = 1 / width;   // or .1

    writeln(step);  // now 0.01
}

This seems like a very easy mistake to make. But I know the compiler
is probably powerless to do anything about it. It has an expression
resulting in an int at the RHS, and it can easily cast this to a
double since there's no obvious loss of precision when casting
int->double.

Where's those lint tools for D.. :/
Jul 08 2011
next sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Fri, 08 Jul 2011 18:45:58 -0400, Andrej Mitrovic  
<andrej.mitrovich gmail.com> wrote:


 This is just an observation, not a question or anything.

 void main()
 {
     enum width = 100;
     double step = 1 / width;

     writeln(step);  // 0
 }

 I've just had this bug in my code. I forgot to make either width or 1
 a floating-point type. IOW, I didn't do this:

 void main()
 {
     enum width = 100.0;
     double step = 1 / width;   // or .1

     writeln(step);  // now 0.01
 }

 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.


BTW, this is definitely not something the compiler can determine.  You may  
actually *want* to have integer division then convert to a double.

So a lint tool would be perfect for this -- it cannot be an error, and  
warnings don't exist in D.

-Steve
Jul 08 2011
parent reply "Nick Sabalausky" <a a.a> writes:
"Steven Schveighoffer" <schveiguy yahoo.com> wrote in message 
news:op.vybq4xkkeav7ka localhost.localdomain...

 On Fri, 08 Jul 2011 18:45:58 -0400, Andrej Mitrovic 
 <andrej.mitrovich gmail.com> wrote:


 This is just an observation, not a question or anything.

 void main()
 {
     enum width = 100;
     double step = 1 / width;

     writeln(step);  // 0
 }

 I've just had this bug in my code. I forgot to make either width or 1
 a floating-point type. IOW, I didn't do this:

 void main()
 {
     enum width = 100.0;
     double step = 1 / width;   // or .1

     writeln(step);  // now 0.01
 }

 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.


 BTW, this is definitely not something the compiler can determine.  You may 
 actually *want* to have integer division then convert to a double.

 So a lint tool would be perfect for this -- it cannot be an error, and 
 warnings don't exist in D.


Yea they do: dmd -wi
Jul 09 2011
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sat, 09 Jul 2011 13:28:25 -0400, Nick Sabalausky <a a.a> wrote:


 "Steven Schveighoffer" <schveiguy yahoo.com> wrote in message
 news:op.vybq4xkkeav7ka localhost.localdomain...

 On Fri, 08 Jul 2011 18:45:58 -0400, Andrej Mitrovic
 <andrej.mitrovich gmail.com> wrote:


 This is just an observation, not a question or anything.

 void main()
 {
     enum width = 100;
     double step = 1 / width;

     writeln(step);  // 0
 }

 I've just had this bug in my code. I forgot to make either width or 1
 a floating-point type. IOW, I didn't do this:

 void main()
 {
     enum width = 100.0;
     double step = 1 / width;   // or .1

     writeln(step);  // now 0.01
 }

 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.


 BTW, this is definitely not something the compiler can determine.  You  
 may
 actually *want* to have integer division then convert to a double.

 So a lint tool would be perfect for this -- it cannot be an error, and
 warnings don't exist in D.


 Yea they do: dmd -wi


Hm... I didn't know about that switch, I thought warnings were enabled  
with -w and that made them errors.

I suppose you could stick this in there.

-Steve
Jul 11 2011
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 11 July 2011 08:17:50 Steven Schveighoffer wrote:

 On Sat, 09 Jul 2011 13:28:25 -0400, Nick Sabalausky <a a.a> wrote:

 "Steven Schveighoffer" <schveiguy yahoo.com> wrote in message
 news:op.vybq4xkkeav7ka localhost.localdomain...
 

 On Fri, 08 Jul 2011 18:45:58 -0400, Andrej Mitrovic
 
 <andrej.mitrovich gmail.com> wrote:

 This is just an observation, not a question or anything.
 
 void main()
 {
 
     enum width = 100;
     double step = 1 / width;
     
     writeln(step);  // 0
 
 }
 
 I've just had this bug in my code. I forgot to make either width or
 1
 a floating-point type. IOW, I didn't do this:
 
 void main()
 {
 
     enum width = 100.0;
     double step = 1 / width;   // or .1
     
     writeln(step);  // now 0.01
 
 }
 
 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.


 
 BTW, this is definitely not something the compiler can determine.  You
 may
 actually *want* to have integer division then convert to a double.
 
 So a lint tool would be perfect for this -- it cannot be an error, and
 warnings don't exist in D.


 
 Yea they do: dmd -wi


 
 Hm... I didn't know about that switch, I thought warnings were enabled
 with -w and that made them errors.
 
 I suppose you could stick this in there.


By default, warnings are completely disabled. -w enables them and makes the 
errors. So, it's more like telling it to add more errors than to enable 
warnings. -wi enables them, but it leaves them as warnings. They're then 
"informational" (basically, -wi makes warnings work like they do with pretty 
much every other compiler on the planet). IIRC, people kept bugging Walter 
about how warnings work with dmd, so he added -wi.

- Jonathan M Davis
Jul 11 2011
parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 11 Jul 2011 11:45:47 -0400, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:


 On Monday 11 July 2011 08:17:50 Steven Schveighoffer wrote:



 Hm... I didn't know about that switch, I thought warnings were enabled
 with -w and that made them errors.

 I suppose you could stick this in there.


 By default, warnings are completely disabled. -w enables them and makes  
 the
 errors. So, it's more like telling it to add more errors than to enable
 warnings. -wi enables them, but it leaves them as warnings. They're then
 "informational" (basically, -wi makes warnings work like they do with  
 pretty
 much every other compiler on the planet). IIRC, people kept bugging  
 Walter
 about how warnings work with dmd, so he added -wi.


eh... then I retract my statement.  I thought the set of -wi warnings  
could be different than the set of -w errors.

This is not always an error, I don't think you should need a cast for  
things like this.

-Steve
Jul 11 2011
prev sibling parent reply "Nick Sabalausky" <a a.a> writes:
"Steven Schveighoffer" <schveiguy yahoo.com> wrote in message 
news:op.vyge30ogeav7ka localhost.localdomain...

 On Sat, 09 Jul 2011 13:28:25 -0400, Nick Sabalausky <a a.a> wrote:


 "Steven Schveighoffer" <schveiguy yahoo.com> wrote in message
 news:op.vybq4xkkeav7ka localhost.localdomain...

 On Fri, 08 Jul 2011 18:45:58 -0400, Andrej Mitrovic
 <andrej.mitrovich gmail.com> wrote:


 This is just an observation, not a question or anything.

 void main()
 {
     enum width = 100;
     double step = 1 / width;

     writeln(step);  // 0
 }

 I've just had this bug in my code. I forgot to make either width or 1
 a floating-point type. IOW, I didn't do this:

 void main()
 {
     enum width = 100.0;
     double step = 1 / width;   // or .1

     writeln(step);  // now 0.01
 }

 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.


 BTW, this is definitely not something the compiler can determine.  You 
 may
 actually *want* to have integer division then convert to a double.

 So a lint tool would be perfect for this -- it cannot be an error, and
 warnings don't exist in D.


 Yea they do: dmd -wi


 Hm... I didn't know about that switch, I thought warnings were enabled 
 with -w and that made them errors.

 I suppose you could stick this in there.


-wi: Enable warnings
-w: Enable warnings and treat them as errors

Didn't used to have -wi, but I bugged the hell out of Walter about it. ;)

The --help screen describes -w and -wi differently than above, but the above 
is more accurate.
Jul 11 2011
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 11 Jul 2011 16:46:07 -0400, Nick Sabalausky <a a.a> wrote:


 "Steven Schveighoffer" <schveiguy yahoo.com> wrote in message



 Hm... I didn't know about that switch, I thought warnings were enabled
 with -w and that made them errors.

 I suppose you could stick this in there.


 -wi: Enable warnings
 -w: Enable warnings and treat them as errors

 Didn't used to have -wi, but I bugged the hell out of Walter about it. ;)

 The --help screen describes -w and -wi differently than above, but the  
 above
 is more accurate.


Yes, but this is getting into territory where the false positive rate  
might get high.  I don't think it should be an error under -w.  So that  
means -wi and -w would cover different sets of warnings.

-Steve
Jul 12 2011
parent reply Kagamin <spam here.lot> writes:
Steven Schveighoffer Wrote:


 Yes, but this is getting into territory where the false positive rate  
 might get high.


I bet it will be difficult to find a real-world example of this false positive.
Jul 12 2011
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Tue, 12 Jul 2011 11:07:57 -0400, Kagamin <spam here.lot> wrote:


 Steven Schveighoffer Wrote:


 Yes, but this is getting into territory where the false positive rate
 might get high.


 I bet it will be difficult to find a real-world example of this false  
 positive.


It depends on how the warning is triggered.

Does this fail?

double x = 1;

Does this:

double x = 1 / 2;

How about this:

void foo(int x, int y)
in
{
    assert(y != 0);
    assert(x % y == 0);
}
body
{
   double z = x / y;
}

The reason for declaring something a double is not always because you want  
the thing you are assigning it to be a double expression.  For instance,  
sometimes I do something like this:

int z = x / y;

And then realize, all the places where I use z, I actually want to be  
doubles, so I simply change it to:

double z = x / y;

To avoid frequent casting.

-Steve
Jul 12 2011
parent Kagamin <spam here.lot> writes:
Steven Schveighoffer Wrote:


 On Tue, 12 Jul 2011 11:07:57 -0400, Kagamin <spam here.lot> wrote:
 

 Steven Schveighoffer Wrote:


 Yes, but this is getting into territory where the false positive rate
 might get high.


 I bet it will be difficult to find a real-world example of this false  
 positive.


 
 It depends on how the warning is triggered.
 
 Does this fail?
 
 void foo(int x, int y)


Well, I said "real-world example". You're talking about frequency, right? If
the frequency is zero, it's no problem.
Jul 13 2011
prev sibling next sibling parent Don <nospam nospam.com> writes:
Andrej Mitrovic wrote:

 This is just an observation, not a question or anything.
 
 void main()
 {
     enum width = 100;
     double step = 1 / width;
 
     writeln(step);  // 0
 }
 
 I've just had this bug in my code. I forgot to make either width or 1
 a floating-point type. IOW, I didn't do this:
 
 void main()
 {
     enum width = 100.0;
     double step = 1 / width;   // or .1
 
     writeln(step);  // now 0.01
 }
 
 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.
 
 Where's those lint tools for D.. :/


It is indeed a common bug. I made a proposal for fixing it, which was 
accepted, but I still haven't got around to making the patch. It's a 
simple addition to range propagation: integer operations need to record 
if there is a potentially non-zero fractional part in the result.
Non-zero fractional parts are created by /, %, and ^^.
Certain operations (bitwise operations, casts) force the fractional part 
to zero, enabling it to be convertable to float.
Jul 22 2011
prev sibling parent reply "Marco Leise" <Marco.Leise gmx.de> writes:
Am 09.07.2011, 00:45 Uhr, schrieb Andrej Mitrovic  
<andrej.mitrovich gmail.com>:


 This is just an observation, not a question or anything.

 void main()
 {
     enum width = 100;
     double step = 1 / width;

     writeln(step);  // 0
 }

 I've just had this bug in my code. I forgot to make either width or 1
 a floating-point type. IOW, I didn't do this:

 void main()
 {
     enum width = 100.0;
     double step = 1 / width;   // or .1

     writeln(step);  // now 0.01
 }

 This seems like a very easy mistake to make. But I know the compiler
 is probably powerless to do anything about it. It has an expression
 resulting in an int at the RHS, and it can easily cast this to a
 double since there's no obvious loss of precision when casting
 int->double.

 Where's those lint tools for D.. :/


That's why Pascal uses another set of operators for integer divisions,  
namely 'div' and 'mod', so you can never get into that situation. The  
above code would have worked and in case step was an integer, the compiler  
would have complained about not using 'div'. I doubt that we will see  
these in D - at the end of the day code that is both valid C and valid D  
must do the same thing, but I never had problems with 'div' and 'mod' and  
it seems like a good solution.
Aug 14 2011
parent bearophile <bearophileHUGS lycos.com> writes:
Marco Leise:


 That's why Pascal uses another set of operators for integer divisions,  
 namely 'div' and 'mod', so you can never get into that situation. The  
 above code would have worked and in case step was an integer, the compiler  
 would have complained about not using 'div'. I doubt that we will see  
 these in D - at the end of the day code that is both valid C and valid D  
 must do the same thing, but I never had problems with 'div' and 'mod' and  
 it seems like a good solution.


On this I like the Pascal design better. I think in D2 you can't change how
division works. Python2 has done this during its evolution to Python3, but D
has to keep some compatibility with C.

But adding an integer division operator to D and later discouraging (with a
warning too) the usage of the normal division operator for integer numbers is a
possibility (later if you want to port C code to D you see many deprecation
integer division warnings. A hypothetical c2d tool based on DMD source is later
able to fix your C code ported to D).

Bye,
bearophile
Aug 15 2011