So I've had a pretty fun bug today when porting some code over to D. When I port C code to D, first I do a one-to-one translation and make sure everything works before I try to make use of any D features.

Here's a function snippet, try to figure out what's wrong with the code (don't worry about type definitions or external variables yet):

http://codepad.org/gYNxJ8a5

Ok that's quite a bit of code. Let's cut to the chase:

if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof != DRUM.sizeof))
{
    mmioClose(hmmio, 0);
    return szErrorCannotRead;
}

Here goes: The first argument to "mmioRead" is the already opened file handle, the second is the instance variable to write to (a struct variable of type DRUM), and the third argument is the number of bytes to read (an integer).

On success, the function will return the number of bytes read. We compare this to the size of the DRUM structure, and if the sizes are not equal then something has gone wrong. I've added a writeln() call inside the if body to verify that the if statement body is never executed.

Yet, I have no data in the "drum" variable after the call! What on earth did go wrong?

Let's take a closer look at the call:

if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof != DRUM.sizeof))

Look at that nasty parenthesis at the end, it's not in the right position at all! It should be:

if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof) != DRUM.sizeof)

So, what happened with the original call?

This expression: DRUM.sizeof != DRUM.sizeof
was evaluated and converted to this boolean: false
which was implicitly converted to an int: 0
which means mmioRead has read 0 bytes, and remember how this function
returns the number of bytes it has read? Well it returns 0.
0 in turn is implicitly converted to a bool, and the if body is not executed.

Btw, this is all compiling with all the warnings turned on. If I had at least a warning emitted from the compiler I could have spotted the bug sooner and not waste time due to silly (admittedly my) typos like these.

On Sun, 19 Jun 2011 23:13:06 +0300, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:

> So I've had a pretty fun bug today when porting some code over to D.
> When I port C code to D, first I do a one-to-one translation and make
> sure everything works before I try to make use of any D features.

How would you expect D to deal with types such as BOOL (which is a 32-bit type)?

-- 
Best regards,
 Vladimir                            mailto:vladimir@thecybershadow.net

On 6/19/11, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:
> On Sun, 19 Jun 2011 23:13:06 +0300, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
>
>> So I've had a pretty fun bug today when porting some code over to D. When I port C code to D, first I do a one-to-one translation and make sure everything works before I try to make use of any D features.
>
> How would you expect D to deal with types such as BOOL (which is a 32-bit
> type)?

I'm not sure what you're going for?

alias int BOOL;
enum : BOOL {
    FALSE = 0,
    TRUE  = 1
}

void main()
{
    bool value = getTrue();
}

BOOL getTrue()
{
    return TRUE;
}

Error: cannot implicitly convert expression (getTrue()) of type int to bool

My problem was implicit *D bool* conversion to D int types. Not the windows bool type which is an int.

On Mon, 20 Jun 2011 00:32:38 +0300, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:

> I'm not sure what you're going for?
>
> alias int BOOL;
> enum : BOOL {

And now you can't pass D boolean expressions to Win32 APIs without a cast or a "?:".

-- 
Best regards,
 Vladimir                            mailto:vladimir@thecybershadow.net

Andrej Mitrovic wrote:
> So I've had a pretty fun bug today when porting some code over to D. When I port C code to D, first I do a one-to-one translation and make sure everything works before I try to make use of any D features.
>
> Here's a function snippet, try to figure out what's wrong with the code (don't worry about type definitions or external variables yet):
>
> http://codepad.org/gYNxJ8a5
>
> Ok that's quite a bit of code. Let's cut to the chase:
>
> if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof != DRUM.sizeof))
> {
>     mmioClose(hmmio, 0);
>     return szErrorCannotRead;
> }
> [snip.]

Maybe DMD could warn on nonsense of the form x != x.


Timon

On 19/06/11 10:51 PM, Timon Gehr wrote:
> Andrej Mitrovic wrote:
>> So I've had a pretty fun bug today when porting some code over to D.
>> When I port C code to D, first I do a one-to-one translation and make
>> sure everything works before I try to make use of any D features.
>>
>> Here's a function snippet, try to figure out what's wrong with the
>> code (don't worry about type definitions or external variables yet):
>>
>> http://codepad.org/gYNxJ8a5
>>
>> Ok that's quite a bit of code. Let's cut to the chase:
>>
>> if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof != DRUM.sizeof))
>> {
>>      mmioClose(hmmio, 0);
>>      return szErrorCannotRead;
>> }
>> [snip.]
>
> Maybe DMD could warn on nonsense of the form x != x.
>
>
> Timon

That would help in this particular case, but not in general.

Peter Alexander wrote:
> On 19/06/11 10:51 PM, Timon Gehr wrote:
>> Andrej Mitrovic wrote:
>>> So I've had a pretty fun bug today when porting some code over to D. When I port C code to D, first I do a one-to-one translation and make sure everything works before I try to make use of any D features.
>>>
>>> Here's a function snippet, try to figure out what's wrong with the code (don't worry about type definitions or external variables yet):
>>>
>>> http://codepad.org/gYNxJ8a5
>>>
>>> Ok that's quite a bit of code. Let's cut to the chase:
>>>
>>> if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof != DRUM.sizeof))
>>> {
>>>      mmioClose(hmmio, 0);
>>>      return szErrorCannotRead;
>>> }
>>> [snip.]
>>
>> Maybe DMD could warn on nonsense of the form x != x.
>>
>>
>> Timon
>
> That would help in this particular case, but not in general.

I claim that in this particular case the x != x was more of a problem than the
implicit conversions.
Implicit conversions from boolean to int don't hurt in general.
But they are also not generally useful, just if you want to use arithmetics on bools:

bool b;
int i=b; // you seldom want to do this

int month;
int daysinmonth = month==1?28:30+((m&1)==(m>6)); // kinda neat, implicit
conversions in 2 places

A possible resolution could be to just disallow direct implicit conversion but allow bools in arithmetic expressions. That would be special casing though.


Timon

Sry should have been

int daysinmonth = month==1?28:30+((month&1)==(month>6));

obv.

Cheers,
-Timon

Timon Gehr:

> Maybe DMD could warn on nonsense of the form x != x.

Right. This thread is very good food for this enhancement request of mine: http://d.puremagic.com/issues/show_bug.cgi?id=5540

Vote for this enhancement :-)

----------------------

Peter Alexander:

> That would help in this particular case, but not in general.

Experience (there are even articles written on this, with statistics) shows this is a a common class of bug, and for a compiler it's easy to catch such bugs. So while not being very general as you want, it's general enough to justify a compiler test for such things. I hope D will catch similar things.

Bye,
bearophile

On 6/19/2011 2:24 PM, Vladimir Panteleev wrote:
> On Sun, 19 Jun 2011 23:13:06 +0300, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
>
>> So I've had a pretty fun bug today when porting some code over to D.
>> When I port C code to D, first I do a one-to-one translation and make
>> sure everything works before I try to make use of any D features.
>
> How would you expect D to deal with types such as BOOL (which is a 32-bit type)?
>
I agree, this is a ridiculous bug to have to spend so much time for.

How do you deal with BOOL? By emitting a **Warning** instead of an error, which is only turned on with an appropriate compiler option (say, higher verbosity).
Simple as that. :)

Mehrdad