On 09/16/2015 09:59 AM, Marek Polacek wrote:
This patch implements a new warning, -Wduplicated-cond. It warns for
code such as
if (x)
// ...
else if (x)
// ...
As usual, I like this improvement. I think it's worth extending
to conditional expressions (e.g., x ? f() : x ? g() : h() should
be diagnosed as well).
The patch currently issues a false positive for the test case
below. I suspect the chain might need to be cleared after each
condition that involves a side-effect.
int foo (int a)
{
if (a) return 1; else if (++a) return 2; else if (a) return 3;
return 0;
}
The patch doesn't diagnose some more involved cases like the one
below:
if (i < 0) return 1; else if (!(i > 0 || i == 0)) return 2;
even though it does diagnose some other such cases, including:
if (i < 0) return 1; else if (!(i >= 0)) return 2;
and even:
int foo (int a, int b, int c) {
if (a + c == b) return 1; else if (a + c - b == 0) return 2;
return 0;
}
I'm guessing this is due to operand_equal_p returning true for
some pairs of equivalent expressions and false for others. Would
making this work be feasible?
You probably didn't intend to diagnose the final else clause in
the following case but I wonder if it should be (as an extension
of the original idea):
if (i) return 1; else if (!i) return 2; else return 0;
(In fact, I wonder if it might be worth diagnosing even the
'if (!i)' part since the condition is the inverse of the one
in the if and thus redundant).
Is diagnosing things like 'if (a) if (a);' or 'if (a); else {
if (a); }' not feasible or too involved, or did you choose not
to because you expect it might generate too many warnings? (If
the latter, perhaps it could be disabled by default and enabled
by -Wduplicated-cond=2).
Martin
