This patch improves -Wlogical-op so that it also warns about cases such as
P && P or P || P. I made use of what merge_ranges computes: if we have equal
operands with the same ranges, warn -- that seems to work well.
(-Wlogical-op still isn't enabled neither by -Wall nor by -Wextra.)
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-04-21 Marek Polacek <pola...@redhat.com>
PR c/63357
* c-common.c (warn_logical_operator): Warn if the operands have the
same expressions.
* doc/invoke.texi: Update description of -Wlogical-op.
* c-c++-common/Wlogical-op-1.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 7fe7fa6..6eecc73 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1772,22 +1772,35 @@ warn_logical_operator (location_t location, enum
tree_code code, tree type,
return;
/* If both expressions have the same operand, if we can merge the
- ranges, and if the range test is always false, then warn. */
+ ranges, ... */
if (operand_equal_p (lhs, rhs, 0)
&& merge_ranges (&in_p, &low, &high, in0_p, low0, high0,
- in1_p, low1, high1)
- && 0 != (tem = build_range_check (UNKNOWN_LOCATION,
- type, lhs, in_p, low, high))
- && integer_zerop (tem))
+ in1_p, low1, high1))
{
- if (or_op)
- warning_at (location, OPT_Wlogical_op,
- "logical %<or%> "
- "of collectively exhaustive tests is always true");
- else
- warning_at (location, OPT_Wlogical_op,
- "logical %<and%> "
- "of mutually exclusive tests is always false");
+ tem = build_range_check (UNKNOWN_LOCATION, type, lhs, in_p, low, high);
+ /* ... and if the range test is always false, then warn. */
+ if (tem && integer_zerop (tem))
+ {
+ if (or_op)
+ warning_at (location, OPT_Wlogical_op,
+ "logical %<or%> of collectively exhaustive tests is "
+ "always true");
+ else
+ warning_at (location, OPT_Wlogical_op,
+ "logical %<and%> of mutually exclusive tests is "
+ "always false");
+ }
+ /* Or warn if the operands have exactly the same range, e.g.
+ A > 0 && A > 0. */
+ else if (low0 == low1 && high0 == high1)
+ {
+ if (or_op)
+ warning_at (location, OPT_Wlogical_op,
+ "logical %<or%> of equal expressions");
+ else
+ warning_at (location, OPT_Wlogical_op,
+ "logical %<and%> of equal expressions");
+ }
}
}
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index c20dd4d..8ce233b 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -4936,7 +4936,12 @@ programmer intended to use @code{strcmp}. This warning
is enabled by
@opindex Wno-logical-op
Warn about suspicious uses of logical operators in expressions.
This includes using logical operators in contexts where a
-bit-wise operator is likely to be expected.
+bit-wise operator is likely to be expected. Also warns when
+the operands of a logical operator are the same:
+@smallexample
+extern int a;
+if (a < 0 && a < 0) @{ @dots{} @}
+@end smallexample
@item -Wlogical-not-parentheses
@opindex Wlogical-not-parentheses
diff --git gcc/testsuite/c-c++-common/Wlogical-op-1.c
gcc/testsuite/c-c++-common/Wlogical-op-1.c
index e69de29..33d4f38 100644
--- gcc/testsuite/c-c++-common/Wlogical-op-1.c
+++ gcc/testsuite/c-c++-common/Wlogical-op-1.c
@@ -0,0 +1,109 @@
+/* PR c/63357 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-op" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+# define true 1
+# define false 0
+#endif
+
+extern int bar (void);
+extern int *p;
+struct R { int a, b; } S;
+
+void
+andfn (int a, int b)
+{
+ if (a && a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (!a && !a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (!!a && !!a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (a > 0 && a > 0) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (a < 0 && a < 0) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (a == 0 && a == 0) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (a <= 0 && a <= 0) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (a >= 0 && a >= 0) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (a == 0 && !(a != 0)) {} /* { dg-warning "logical .and. of equal
expressions" } */
+
+ if (a && a && a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if ((a + 1) && (a + 1)) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if ((10 * a) && (a * 10)) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (!!a && a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+
+ if (*p && *p) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (p[0] && p[0]) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if (S.a && S.a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if ((bool) a && (bool) a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+ if ((unsigned) a && a) {} /* { dg-warning "logical .and. of equal
expressions" } */
+
+ /* Stay quiet here. */
+ if (a && b) {}
+ if (!a && !b) {}
+ if (!!a && !!b) {}
+ if (a > 0 && b > 0) {}
+ if (a < 0 && b < 0) {}
+ if (a == 0 && b == 0) {}
+ if (a <= 0 && b <= 0) {}
+ if (a >= 0 && b >= 0) {}
+
+ if (a > 0 && a > 1) {}
+ if (a > -2 && a > 1) {}
+ if (a && (short) a) {}
+ if ((char) a && a) {}
+ if (++a && a) {}
+ if (++a && ++a) {}
+ if (a && --a) {}
+ if (a && a / 2) {}
+ if (bar () && bar ()) {}
+ if (p && *p) {}
+ if (p[0] && p[1]) {}
+ if (S.a && S.b) {}
+}
+
+void
+orfn (int a, int b)
+{
+ if (a || a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (!a || !a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (!!a || !!a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (a > 0 || a > 0) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (a < 0 || a < 0) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (a == 0 || a == 0) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (a <= 0 || a <= 0) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (a >= 0 || a >= 0) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (a == 0 || !(a != 0)) {} /* { dg-warning "logical .or. of equal
expressions" } */
+
+ if (a || a || a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if ((a + 1) || (a + 1)) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if ((10 * a) || (a * 10)) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (!!a || a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+
+ if (*p || *p) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (p[0] || p[0]) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if (S.a || S.a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if ((bool) a || (bool) a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+ if ((unsigned) a || a) {} /* { dg-warning "logical .or. of equal
expressions" } */
+
+ /* Stay quiet here. */
+ if (a || b) {}
+ if (!a || !b) {}
+ if (!!a || !!b) {}
+ if (a > 0 || b > 0) {}
+ if (a < 0 || b < 0) {}
+ if (a == 0 || b == 0) {}
+ if (a <= 0 || b <= 0) {}
+ if (a >= 0 || b >= 0) {}
+
+ if (a > 0 || a > 1) {}
+ if (a > -2 || a > 1) {}
+ if (a || (short) a) {}
+ if ((char) a || a) {}
+ if (++a || a) {}
+ if (++a || ++a) {}
+ if (a || --a) {}
+ if (a || a / 2) {}
+ if (bar () || bar ()) {}
+ if (p || *p) {}
+ if (p[0] || p[1]) {}
+ if (S.a || S.b) {}
+}
Marek
