Debugging typos

Posted on:March 06 2008

I really want to bang my head against something when I spent an hour debugging a problem again and found it to be a simple typo. For example, I found the problem in the following code (not originally C++ but translated to C it would look like this). It simply should count the numbers of bits set in a value:

value = some value where we want to count the number of bits set in it.
testBit = 0x1;
bitsSet = 0;
for (int i=0; i<16; ++i)
{
  if (value & testBit)
    bitsSet += 1;
  testBit << 1;
}

See the problem? Yep, it's the last line. Instead of 'testBit << 1;', it should read 'testBit = testBit << 1;'. Argh. Too bad the compiler didn't write a warning, because that line doesn't actually make sense at all, in any case.

Comments:

I prefer to always use braces around my conditional and iteration statements. For me it's easier to read, even though it takes a bit more space.

The compiler may optimize it to the same thing, but why didn't you use the pre-increment operator on bitsSet like you did with the loop iterator i?

I actually got this as a question during a Google interview: What is the fastest way to count the number of bits set in, for example, an int?

Michael

Quote
2008-03-06 19:22:00

Maybe increasing the warning level of your compiler would have helped ;)

Sylence

Quote
2008-03-06 19:25:00

And the utter irony is that a C compiler, with the right options at least, could warn you about that

With a file bug.c
======================================
int main()
{
int value = 10;
int testBit = 0x1;
int bitsSet = 0;
for (int i=0; i<16; ++i)
{
if (value & testBit)
bitsSet += 1;
testBit << 1;
}
return 0;
}
========================================

the compiler output I get is
james@Electron:~/temp$ gcc -Wall -pedantic -std=c99 bug.c
bug.c: In function ‘main’:
bug.c:10: warning: statement with no effect
james@Electron:~/temp$

Electron

Quote
2008-03-06 19:27:00

Hmm, your blog seems to want to truncate my comment. Regardless, if I compile what you have as C code, my compiler output is

james@Electron:~/temp$ gcc -Wall -pedantic -std=c99 bug.c
bug.c: In function ‘main’:
bug.c:10: warning: statement with no effect

Electron

Quote
2008-03-06 19:31:00

>>>Argh. Too bad the compiler didn't write a warning

Before you complain about the compiler try to not disable any warnings with pragmas and/or keep your compiler warning level too low:

GCC -Wall produces:
test.cpp:14: warning: statement has no effect

MSVC /W3 (warning level 3) produces:
test.cpp(14) : warning C4552: '<<' : operator has no effect; expected operator with side-effect

Also if you install VistaSDK you get MSVC compiler that has static analysis option (/analyze).

It's so typical for game developers to disable all warnings with pragmas, lower the warning level to 1, don't treat warnings as errors (msvc /WX or gcc -Werror), and then whine how C++ causes too many bugs. ;)

BKLA

Quote
2008-03-06 20:32:00

MSVC /W3 (warning level 3) produces:
test.cpp(14) : warning C4552: '<<' : operator has no effect; expected operator with side-effect

Also if you install VistaSDK you get MSVC compiler that has static analysis option (/analyze).

BKLA

Quote
2008-03-06 20:33:00

Petty coding mistakes are those which take the most times to debug. I always end thinking: What, it was just that ?

Hervé

Quote
2008-03-07 01:02:00

I sense your lack of faith disturbing! Looks like you made some between-the-lines comment about... ¿Java?

Uhm.

Alejo

Quote
2008-03-07 01:59:00

// or
testBit<<=1;

Joilnen

Quote
2008-03-07 15:49:00

yes, the original language the problem occurred was ActionScript, and that compiler won't report problems like this :)

niko

Quote
2008-03-07 15:56:00

// or
testBit<<=1;

Joilnen

Quote
2008-03-07 16:10:00

There's a fundamentally different way to count the number of set bits, without using branches. It goes something like this:

value = (value & 0x5555) + ((value & 0xAAAA) >> 1);
value = (value & 0x3333) + ((value & 0xCCCC) >> 2);
value = (value & 0x0F0F) + ((value & 0xF0F0) >> 4);
value = (value & 0x00FF) + ((value & 0xFF00) >> 8);

Depending on the system details (branch overhead etc.), this can be significantly faster. And it looks way cooler, too ;)

Quote
2008-03-16 13:35:00