I have the following code for implementing a new warning (PR16302). It
works as intended but I feel there is some duplication with the code
in fold_range_test (fold_const.c). However, fold_range_test cannot
handle arguments that contain C_MAYBE_CONST_EXPR, hence the explicit
testing I added below.
Any suggestions?
Is this sufficient or is there any way to do this "properly" ?
/* Warn about uses of logical || / && operator in a context where it
is likely that the bitwise equivalent was intended by the
programmer. We have seen an expression in which CODE is a binary
- operator used to combine expressions OP_LEFT and OP_RIGHT, which
- before folding had CODE_LEFT and CODE_RIGHT. */
-
+ operator used to combine expressions OP_LEFT and OP_RIGHT, which
before folding
+ had CODE_LEFT and CODE_RIGHT, into an expression of type TYPE. */
void
-warn_logical_operator (location_t location, enum tree_code code,
+warn_logical_operator (location_t location, enum tree_code code, tree type,
enum tree_code code_left, tree op_left,
enum tree_code ARG_UNUSED (code_right), tree op_right)
{
+ int or_op = (code == TRUTH_ORIF_EXPR || code == TRUTH_OR_EXPR);
+ int in0_p, in1_p, in_p;
+ tree low0, low1, low, high0, high1, high, lhs, rhs, tem;
+ bool strict_overflow_p = false;
+
if (code != TRUTH_ANDIF_EXPR
&& code != TRUTH_AND_EXPR
&& code != TRUTH_ORIF_EXPR
&& code != TRUTH_OR_EXPR)
return;
@@ -1740,17 +1744,65 @@ warn_logical_operator (location_t locati
&& !TREE_NO_WARNING (op_left)
&& TREE_CODE (op_right) == INTEGER_CST
&& !integer_zerop (op_right)
&& !integer_onep (op_right))
{
- if (code == TRUTH_ORIF_EXPR || code == TRUTH_OR_EXPR)
+ if (or_op)
warning_at (location, OPT_Wlogical_op, "logical %<or%>"
" applied to non-boolean constant");
else
warning_at (location, OPT_Wlogical_op, "logical %<and%>"
" applied to non-boolean constant");
TREE_NO_WARNING (op_left) = true;
+ return;
+ }
+
+ /* We do not warn for constants because they are typical of macro
+ expansions that test for features. */
+ if (CONSTANT_CLASS_P (op_left) || CONSTANT_CLASS_P (op_right))
+ return;
+
+ /* This warning only makes sense with logical operands. */
+ if (!(truth_value_p (TREE_CODE (op_left))
+ || INTEGRAL_TYPE_P (TREE_TYPE (op_left)))
+ || !(truth_value_p (TREE_CODE (op_right))
+ || INTEGRAL_TYPE_P (TREE_TYPE (op_right))))
+ return;
+
+ lhs = make_range (op_left, &in0_p, &low0, &high0, &strict_overflow_p);
+ rhs = make_range (op_right, &in1_p, &low1, &high1, &strict_overflow_p);
+
+ if (lhs && TREE_CODE (lhs) == C_MAYBE_CONST_EXPR)
+ lhs = C_MAYBE_CONST_EXPR_EXPR (lhs);
+
+ if (rhs && TREE_CODE (rhs) == C_MAYBE_CONST_EXPR)
+ rhs = C_MAYBE_CONST_EXPR_EXPR (rhs);
+
+ /* If this is an OR operation, invert both sides; we will invert
+ again at the end. */
+ if (or_op)
+ in0_p = !in0_p, in1_p = !in1_p;
+
+ /* If both expressions are the same, if we can merge the ranges, and we
+ can build the range test, return it or it inverted. If one of the
+ ranges is always true or always false, consider it to be the same
+ expression as the other. */
+ if ((lhs == 0 || rhs == 0 || 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 (type,
+ lhs != 0 ? lhs
+ : rhs != 0 ? rhs : integer_zero_node,
+ in_p, low, high)))
+ {
+ if (TREE_CODE (tem) != INTEGER_CST)
+ return;
+
+ warning_at (location, OPT_Wlogical_op,
+ or_op
+ ? "logical %<or%> of collectively exhaustive tests
is always true"
+ : "logical %<and%> of mutually exclusive tests is
always false");
}
}
