Comment #0 by bearophile_hugs — 2011-02-07T12:33:29Z
This is a real bug in good C++ code:
if ((p_newHeader.frame_num != m_lastSlice.frame_num) ||
(p_newHeader.pic_parameter_set_id !=
p_newHeader.pic_parameter_set_id) ||
(p_newHeader.field_pic_flag != p_newHeader.field_pic_flag) ||
(p_newHeader.bottom_field_flag != m_lastSlice.bottom_field_flag)
){
return false;
}
The bug is of the kind:
if (x != x) {}
Two other similar bugs:
if (x == x) {}
auto y = x - x;
For the compiler it's easy to spot such three situations. They are correct code, yet experience shows that often such redundancies hide bugs. So I suggest to add to DMD warnings for such tree situations.
See also bug 3878
Comment #1 by bearophile_hugs — 2011-02-07T15:05:52Z
Another case, in good C++ code (probably caused by not complete copy & paste & modify):
return Contains(Sphere(lss.mP0, lss.mRadius)) &&
Contains(Sphere(lss.mP0, lss.mRadius));
That is of this kind:
x && x
Comment #2 by bearophile_hugs — 2011-04-07T15:08:11Z
Comment #4 by bearophile_hugs — 2011-06-19T16:44:30Z
(In reply to comment #3)
> See a very nice example of bug of this kind, that's easy to find for a compiler
> or lint:
>
> http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=139021
The bug in Andrej Mitrovic code was:
if (mmioRead(hmmio, cast(LPSTR)&drum, DRUM.sizeof != DRUM.sizeof))
{
mmioClose(hmmio, 0);
return szErrorCannotRead;
}
Comment #5 by bearophile_hugs — 2011-06-29T13:24:48Z
Comment #6 by bearophile_hugs — 2011-06-29T16:56:41Z
A class of bug-hiding redundancy:
if (x == 10)
foo1();
else if (x == 20)
foo2();
else if (x == 10) // ***
foo3();
Another class, two assignments to the same variable in a row:
x = foo();
x = bar();
While this is OK:
x = 1;
x = x + 1;
x = foo(x);
Another class (the two branches are equal):
if (x)
foo();
else
foo();
Comment #7 by lt.infiltrator — 2015-12-09T12:17:38Z
I think that you're being a little too zealous. For instance, foo() in your last example, and any function in general, may not necessarily be pure (in the mathematical sense) so they may produce different results and/or side-effects.
Comment #8 by dmitry.olsh — 2018-05-22T09:45:01Z
I think indeed for expressions that are trivially true (or false) there must be an error in the context of || and &&. x == x and x != x is a popular mistake.
Except for true/false literals as they are often used for debugging and disabling parts of program explicitly. It's a balancing act but a lot of vlaue to be had.
On the other hand I'd really just give this to dscanner or simillar lint. I tried dscanner right now but it doesn't seem to pick it up.
Comment #9 by razvan.nitu1305 — 2022-08-15T14:09:37Z
Walter is against adding warnings, so that path is a dead end.
These checks should be done by a linter that uses dmd as a library.