Given the recent (data->flags && ff_genericize) vs.
(data->flags & ff_genericize) typo, I've looked at warning in similar
cases.
We don't warn for cases like that at all, clang/clang++ has
-Wconstant-logical-operand warning enabled by default.
Their behavior is:
1) only warns for rhs of &&/|| (why?)
2) don't warn if rhs is bool
3) for C++ warn if rhs is constant or folds into constant,
for C warn if rhs is constant or folds into constant and
that constant is not 0 or 1
4) I think it doesn't warn if rhs comes from a macro
The following patch implements similar warning with similar wording,
just provides the value of the constant, but
1) warns for lhs and rhs
2) doesn't warn if either lhs or rhs is bool
3) doesn't warn if lhs or rhs is or folds to constant 0 or 1
(but does warn if it is constant 1 of enum type in an enum which
has enumerator other than just 0/1 (i.e. poor man's boolean))
4) doesn't care if it comes from a macro or not
I think 64 && x is similarly suspicious to x && 64 and both
are likely to be meant 64 & x or x & 64. I think having
&& 1 or && 0 is common even in C++, people don't always write
&& true or && false etc. and don't see why C++ would be different
in that from C, I think people sometimes write
if (1
#ifdef ABC
&& ABC
#endif
#ifdef DEF
&& DEF
#endif
&& 1)
and similar (or similarly with 0/true/false or ||). And the warning
is only enabled in -Wall, not by default.
2026-05-15 Jakub Jelinek <jakub@redhat.com>
PR c++/125081
gcc/
* doc/invoke.texi (Wconstant-logical-operand): Document.
gcc/c-family/
* c.opt (Wconstant-logical-operand): New option.
* c.opt.urls: Regenerate.
gcc/c/
* c-tree.h (parser_build_binary_op): Add ORIG_ARG1 argument.
* c-typeck.cc (parser_build_binary_op): Likewise. Emit
-Wconstant-logical-operand warnings.
* c-parser.cc (c_parser_binary_expression): Adjust
parser_build_binary_op caller, pass to it the original
stack[sp - 1].expr.value before c_objc_common_truthvalue_conversion.
gcc/cp/
* typeck.cc (cp_build_binary_op): Emit -Wconstant-logical-operand
warnings.
gcc/testsuite/
* c-c++-common/Wconstant-logical-operand-1.c: New test.
* c-c++-common/Wconstant-logical-operand-2.c: New test.
Reviewed-by: Jason Merrill <jason@redhat.com>
Reviewed-by: "Joseph S. Myers" <josmyers@redhat.com>
C++ ObjC++ Var(warn_conditionally_supported) Warning
Warn for conditionally-supported constructs.
+Wconstant-logical-operand
+C ObjC C++ ObjC++ Var(warn_constant_logical_operand) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn for constant operands of logical operators other than 0 or 1.
+
Wconversion
C ObjC C++ ObjC++ Var(warn_conversion) Warning
Warn for implicit type conversions that may change a value.
Wconditionally-supported
UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wconditionally-supported)
+Wconstant-logical-operand
+UrlSuffix(gcc/Warning-Options.html#index-Wconstant-logical-operand)
+
Wconversion
UrlSuffix(gcc/Warning-Options.html#index-Wconversion) LangUrlSuffix_Fortran(gfortran/Error-and-Warning-Options.html#index-Wconversion)
location_t loc;
/* The sizeof argument if expr.original_code == {PAREN_,}SIZEOF_EXPR. */
tree sizeof_arg;
+ /* The original expr.value before c_objc_common_truthvalue_conversion
+ for TRUTH_{AND,OR}*_EXPR lhs operands. */
+ tree orig_expr;
} stack[NUM_PRECS];
int sp;
/* Location of the binary operator. */
stack[sp - 1].expr = parser_build_binary_op (stack[sp].loc, \
stack[sp].op, \
stack[sp - 1].expr, \
- stack[sp].expr); \
+ stack[sp].expr, \
+ stack[sp - 1].orig_expr); \
+ stack[sp - 1].orig_expr = NULL_TREE; \
sp--; \
} while (0)
gcc_assert (!after || c_dialect_objc ());
stack[0].expr = c_parser_cast_expression (parser, after);
stack[0].prec = PREC_NONE;
stack[0].sizeof_arg = c_last_sizeof_arg;
+ stack[0].orig_expr = NULL_TREE;
sp = 0;
while (true)
{
stack[sp].expr
= convert_lvalue_to_rvalue (stack[sp].loc,
stack[sp].expr, true, true);
+ stack[sp].orig_expr = stack[sp].expr.value;
stack[sp].expr.value = c_objc_common_truthvalue_conversion
(stack[sp].loc, default_conversion (stack[sp].expr.value));
c_inhibit_evaluation_warnings += (stack[sp].expr.value
stack[sp].expr
= convert_lvalue_to_rvalue (stack[sp].loc,
stack[sp].expr, true, true);
+ stack[sp].orig_expr = stack[sp].expr.value;
stack[sp].expr.value = c_objc_common_truthvalue_conversion
(stack[sp].loc, default_conversion (stack[sp].expr.value));
c_inhibit_evaluation_warnings += (stack[sp].expr.value
stack[sp].prec = oprec;
stack[sp].op = ocode;
stack[sp].sizeof_arg = c_last_sizeof_arg;
+ stack[sp].orig_expr = NULL_TREE;
}
out:
while (sp > 0)
struct c_expr);
extern struct c_expr parser_build_binary_op (location_t,
enum tree_code, struct c_expr,
- struct c_expr);
+ struct c_expr, tree);
extern tree build_conditional_expr (location_t, tree, bool, tree, tree,
location_t, tree, tree, location_t);
extern tree build_compound_expr (location_t, tree, tree);
in the input. CODE, a tree_code, specifies the binary operator, and
ARG1 and ARG2 are the operands. In addition to constructing the
expression, we check for operands that were written with other binary
- operators in a way that is likely to confuse the user.
+ operators in a way that is likely to confuse the user. ORIG_ARG1 is
+ the original first operand for TRUTH_{AND,OR}IF_EXPR before it is
+ converted to truth value, otherwise NULL_TREE.
LOCATION is the location of the binary operator. */
struct c_expr
parser_build_binary_op (location_t location, enum tree_code code,
- struct c_expr arg1, struct c_expr arg2)
+ struct c_expr arg1, struct c_expr arg2, tree orig_arg1)
{
struct c_expr result;
result.m_decimal = 0;
warn_logical_operator (location, code, TREE_TYPE (result.value),
code1, arg1.value, code2, arg2.value);
+ if (warn_constant_logical_operand
+ && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
+ && INTEGRAL_NB_TYPE_P (type1)
+ && INTEGRAL_NB_TYPE_P (type2))
+ {
+ const char *name = code == TRUTH_ANDIF_EXPR ? "&&" : "||";
+ if (orig_arg1 == NULL_TREE)
+ orig_arg1 = arg1.value;
+ auto enum_other_than_0_1 = [] (tree type) {
+ if (TREE_CODE (type) != ENUMERAL_TYPE)
+ return false;
+ for (tree l = TYPE_VALUES (type); l; l = TREE_CHAIN (l))
+ {
+ tree v = DECL_INITIAL (TREE_VALUE (l));
+ if (!integer_zerop (v) && !integer_onep (v))
+ return true;
+ }
+ return false;
+ };
+ auto diagnose_constant_logical_operand = [=] (tree val, tree type) {
+ if (TREE_CODE (val) != INTEGER_CST || integer_zerop (val))
+ return false;
+ if (integer_onep (val) && !enum_other_than_0_1 (type))
+ return false;
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_replace (name + 1);
+ auto_diagnostic_group d;
+ if (warning_at (location, OPT_Wconstant_logical_operand,
+ "use of logical %qs with constant operand %qE",
+ name, val))
+ inform (&richloc, "use %qs for bitwise operation", name + 1);
+ return true;
+ };
+ if (!diagnose_constant_logical_operand (arg2.value, type2))
+ diagnose_constant_logical_operand (orig_arg1, type1);
+ }
+
if (warn_tautological_compare)
{
tree lhs = arg1.value;
return cp_build_binary_op (location, code, op0, op1, complain);
}
+ if (warn_constant_logical_operand
+ && (complain & tf_warning)
+ && (code0 == INTEGER_TYPE || code0 == ENUMERAL_TYPE)
+ && (code1 == INTEGER_TYPE || code1 == ENUMERAL_TYPE))
+ {
+ tree cop0 = fold_for_warn (op0), cop1 = fold_for_warn (op1);
+ const char *name
+ = ((code == TRUTH_ANDIF_EXPR || code == TRUTH_AND_EXPR)
+ ? "&&" : "||");
+ auto enum_other_than_0_1 = [] (tree type) {
+ if (TREE_CODE (type) != ENUMERAL_TYPE)
+ return false;
+ for (tree l = TYPE_VALUES (type); l; l = TREE_CHAIN (l))
+ {
+ tree v = DECL_INITIAL (TREE_VALUE (l));
+ if (!integer_zerop (v) && !integer_onep (v))
+ return true;
+ }
+ return false;
+ };
+ auto diagnose_constant_logical_operand = [=] (tree val, tree type) {
+ if (TREE_CODE (val) != INTEGER_CST || integer_zerop (val))
+ return false;
+ if (integer_onep (val) && !enum_other_than_0_1 (type))
+ return false;
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_replace (name + 1);
+ auto_diagnostic_group d;
+ if (warning_at (location, OPT_Wconstant_logical_operand,
+ "use of logical %qs with constant operand %qE",
+ name, val))
+ inform (&richloc, "use %qs for bitwise operation", name + 1);
+ return true;
+ };
+ if (!diagnose_constant_logical_operand (cop1, orig_type1))
+ diagnose_constant_logical_operand (cop0, orig_type0);
+ }
+
result_type = boolean_type_node;
break;
-Wchar-subscripts
-Wclobbered -Wcomment
-Wcompare-distinct-pointer-types
--Wno-complain-wrong-lang
+-Wno-complain-wrong-lang -Wconstant-logical-operand
-Wconversion -Wno-coverage-mismatch -Wno-cpp
-Wdangling-else -Wdangling-pointer -Wdangling-pointer=@var{n}
-Wdate-time
if (a < 0 && a < 0) @{ @dots{} @}
@end smallexample
+@opindex Wconstant-logical-operand
+@opindex Wno-constant-logical-operand
+@item -Wconstant-logical-operand
+Warn about another case of suspicious uses of logical operators in
+expressions, when neither operand of a logical operator is boolean
+and one of the operands is (or folds into) a constant other than 0 or 1,
+or enumerator with value 1 if the enumeral type contains enumerators with
+values other than 0 or 1.
+In such case the warning will suggest using corresponding bitwise operator.
+@smallexample
+extern int a;
+if (a && 64) @{ @dots{} @}
+@end smallexample
+If the warning is a false positive, one can clarify the code by using
+e.g. @code{a && (64 != 0)} instead.
+
@opindex Wlogical-not-parentheses
@opindex Wno-logical-not-parentheses
@item -Wlogical-not-parentheses
--- /dev/null
+/* PR c++/125081 */
+/* { dg-do compile } */
+/* { dg-options "-std=c23" { target c } } */
+/* { dg-additional-options "-Wconstant-logical-operand" } */
+
+void foo (int);
+enum A { B, C };
+enum D { E, F, G, H, I };
+
+void
+bar (int x, bool y)
+{
+ if (x && 64) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '64'" } */
+ foo (1); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (128 && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '128'" } */
+ foo (2); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x && 1)
+ foo (3);
+ if (1 && x)
+ foo (4);
+ if (x && 0)
+ foo (5);
+ if (0 && x)
+ foo (6);
+ if (x || 32) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '32'" } */
+ foo (7); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (256 || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '256'" } */
+ foo (8); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || 1)
+ foo (9);
+ if (1 || x)
+ foo (10);
+ if (x || 0)
+ foo (11);
+ if (0 || x)
+ foo (12);
+ if (y && 64)
+ foo (13);
+ if (128 && y)
+ foo (14);
+ if (y || 32)
+ foo (15);
+ if (256 || y)
+ foo (16);
+ if (x && B)
+ foo (17);
+ if (B && x)
+ foo (18);
+ if (x && C)
+ foo (19);
+ if (C && x)
+ foo (20);
+ if (x && E)
+ foo (21);
+ if (E && x)
+ foo (22);
+ if (x && F) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[1F]'" } */
+ foo (23); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (F && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[1F]'" } */
+ foo (24); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x && G) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[2G]'" } */
+ foo (25); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (G && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[2G]'" } */
+ foo (26); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || B)
+ foo (27);
+ if (B || x)
+ foo (28);
+ if (x || C)
+ foo (29);
+ if (C || x)
+ foo (30);
+ if (x || E)
+ foo (31);
+ if (E || x)
+ foo (32);
+ if (x || F) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[1F]'" } */
+ foo (33); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (F || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[1F]'" } */
+ foo (34); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || G) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[2G]'" } */
+ foo (35); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (G || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[2G]'" } */
+ foo (36); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+}
+
+void
+baz (int x, bool y)
+{
+ if (x && 63 + 1) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '64'" } */
+ foo (1); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (127 + 1 && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '128'" } */
+ foo (2); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || 31 + 1) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '32'" } */
+ foo (7); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (255 + 1 || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '256'" } */
+ foo (8); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (y && 63 + 1)
+ foo (13);
+ if (127 + 1 && y)
+ foo (14);
+ if (y || 31 + 1)
+ foo (15);
+ if (255 + 1 || y)
+ foo (16);
+}
--- /dev/null
+/* PR c++/125081 */
+/* { dg-do compile } */
+/* { dg-options "-std=c23" { target c } } */
+/* { dg-additional-options "-Wall" } */
+
+void foo (int);
+enum A { B, C };
+enum D { E, F, G, H, I };
+
+void
+bar (int x, bool y)
+{
+ if (x && 64) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '64'" } */
+ foo (1); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (128 && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '128'" } */
+ foo (2); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x && 1)
+ foo (3);
+ if (1 && x)
+ foo (4);
+ if (x && 0)
+ foo (5);
+ if (0 && x)
+ foo (6);
+ if (x || 32) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '32'" } */
+ foo (7); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (256 || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '256'" } */
+ foo (8); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || 1)
+ foo (9);
+ if (1 || x)
+ foo (10);
+ if (x || 0)
+ foo (11);
+ if (0 || x)
+ foo (12);
+ if (y && 64)
+ foo (13);
+ if (128 && y)
+ foo (14);
+ if (y || 32)
+ foo (15);
+ if (256 || y)
+ foo (16);
+ if (x && B)
+ foo (17);
+ if (B && x)
+ foo (18);
+ if (x && C)
+ foo (19);
+ if (C && x)
+ foo (20);
+ if (x && E)
+ foo (21);
+ if (E && x)
+ foo (22);
+ if (x && F) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[1F]'" } */
+ foo (23); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (F && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[1F]'" } */
+ foo (24); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x && G) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[2G]'" } */
+ foo (25); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ /* { dg-warning "enum constant in boolean context" "" { target c++ } .-2 } */
+ if (G && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '\[2G]'" } */
+ foo (26); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ /* { dg-warning "enum constant in boolean context" "" { target c++ } .-2 } */
+ if (x || B)
+ foo (27);
+ if (B || x)
+ foo (28);
+ if (x || C)
+ foo (29);
+ if (C || x)
+ foo (30);
+ if (x || E)
+ foo (31);
+ if (E || x)
+ foo (32);
+ if (x || F) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[1F]'" } */
+ foo (33); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (F || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[1F]'" } */
+ foo (34); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || G) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[2G]'" } */
+ foo (35); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ /* { dg-warning "enum constant in boolean context" "" { target c++ } .-2 } */
+ if (G || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '\[2G]'" } */
+ foo (36); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ /* { dg-warning "enum constant in boolean context" "" { target c++ } .-2 } */
+}
+
+void
+baz (int x, bool y)
+{
+ if (x && 63 + 1) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '64'" } */
+ foo (1); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (127 + 1 && x) /* { dg-warning "use of logical '\\\&\\\&' with constant operand '128'" } */
+ foo (2); /* { dg-message "note: use '\\\&' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (x || 31 + 1) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '32'" } */
+ foo (7); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (255 + 1 || x) /* { dg-warning "use of logical '\\\|\\\|' with constant operand '256'" } */
+ foo (8); /* { dg-message "note: use '\\\|' for bitwise operation" "" { target *-*-* } .-1 } */
+ if (y && 63 + 1)
+ foo (13);
+ if (127 + 1 && y)
+ foo (14);
+ if (y || 31 + 1)
+ foo (15);
+ if (255 + 1 || y)
+ foo (16);
+}
projects / thirdparty / gcc.git / commitdiff
