From 63a0874077b3325c2b3cc47401d8bfe916d8f128 Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Fri, 9 Mar 2007 16:16:35 +0000 Subject: [PATCH] re PR c/17946 (wanted: warning for "a && MASK" when "a & MASK" was probably intended) 2007-03-09 Dirk Mueller PR c++/17946 * doc/invoke.texi (-Wlogical-op): Document. * common.opt (-Wlogical-op): New. * c-common.h (warn_logical_operator): Declare. * c-common.c (warn_logical_operator): Define. * c-typeck.c (parser_build_binary_op): Call warn_logical_operator. * cp/call.c (build_new_op): Call warn_logical_operator. * testsuite/gcc.dg/Wlogical-op-1.c: New. * testsuite/g++.dg/warn/Wlogical-op-1.C: New. From-SVN: r122751 --- gcc/ChangeLog | 10 +++ gcc/c-common.c | 38 +++++++++ gcc/c-common.h | 1 + gcc/c-typeck.c | 3 + gcc/common.opt | 4 + gcc/cp/ChangeLog | 4 + gcc/cp/call.c | 21 ++++- gcc/doc/invoke.texi | 13 +++- gcc/testsuite/ChangeLog | 6 ++ gcc/testsuite/g++.dg/warn/Wlogical-op-1.C | 92 ++++++++++++++++++++++ gcc/testsuite/gcc.dg/Wlogical-op-1.c | 94 +++++++++++++++++++++++ 11 files changed, 281 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wlogical-op-1.C create mode 100644 gcc/testsuite/gcc.dg/Wlogical-op-1.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 6aa85152ee0..7bd6d741ea1 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,13 @@ +2007-03-09 Dirk Mueller + + PR c++/17946 + * doc/invoke.texi (-Wlogical-op): Document. + * common.opt (-Wlogical-op): New. + * c-common.h (warn_logical_operator): Declare. + * c-common.c (warn_logical_operator): Define. + * c-typeck.c (parser_build_binary_op): Call + warn_logical_operator. + 2007-03-09 Alexandre Oliva * rtl.h (gen_rtx_ASM_INPUT): Use "" instead of NULL file name. diff --git a/gcc/c-common.c b/gcc/c-common.c index 0bcd462ddfc..6216d5e123a 100644 --- a/gcc/c-common.c +++ b/gcc/c-common.c @@ -978,6 +978,44 @@ overflow_warning (tree value) } } + +/* Warn about use of a logical || / && operator being used in a + context where it is likely that the bitwise equivalent was intended + by the programmer. CODE is the TREE_CODE of the operator, ARG1 + and ARG2 the arguments. */ + +void +warn_logical_operator (enum tree_code code, tree arg1, tree + arg2) +{ + switch (code) + { + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_AND_EXPR: + if (!TREE_NO_WARNING (arg1) + && INTEGRAL_TYPE_P (TREE_TYPE (arg1)) + && !CONSTANT_CLASS_P (arg1) + && TREE_CODE (arg2) == INTEGER_CST + && !integer_zerop (arg2) + && !integer_onep (arg2)) + { + warning (OPT_Wlogical_op, + "logical %<%s%> with non-zero constant " + "will always evaluate as true", + ((code == TRUTH_ANDIF_EXPR) + || (code == TRUTH_AND_EXPR)) ? "&&" : "||"); + TREE_NO_WARNING (arg1) = true; + } + + break; + default: + break; + } +} + + /* Print a warning about casts that might indicate violation of strict aliasing rules if -Wstrict-aliasing is used and strict aliasing mode is in effect. OTYPE is the original diff --git a/gcc/c-common.h b/gcc/c-common.h index 2eedf79d29d..f47fa8e3815 100644 --- a/gcc/c-common.h +++ b/gcc/c-common.h @@ -673,6 +673,7 @@ extern void strict_aliasing_warning (tree, tree, tree); extern void empty_body_warning (tree, tree); extern tree convert_and_check (tree, tree); extern void overflow_warning (tree); +extern void warn_logical_operator (enum tree_code, tree, tree); extern void check_main_parameter_types (tree decl); extern bool c_determine_visibility (tree); extern bool same_scalar_type_ignoring_signedness (tree, tree); diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c index 8afc2c757eb..e45c5e0bf97 100644 --- a/gcc/c-typeck.c +++ b/gcc/c-typeck.c @@ -2640,6 +2640,9 @@ parser_build_binary_op (enum tree_code code, struct c_expr arg1, if (warn_parentheses) warn_about_parentheses (code, code1, code2); + if (code1 != tcc_comparison) + warn_logical_operator (code, arg1.value, arg2.value); + /* Warn about comparisons against string literals, with the exception of testing for equality or inequality of a string literal with NULL. */ if (code == EQ_EXPR || code == NE_EXPR) diff --git a/gcc/common.opt b/gcc/common.opt index 938ea598d54..bcf0fcd4ec8 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -118,6 +118,10 @@ Wlarger-than- Common RejectNegative Joined UInteger Warning -Wlarger-than- Warn if an object is larger than bytes +Wlogical-op +Common Warning Var(warn_logical_op) +Warn when a logical operator is suspicously always evaluating to true or false + Wunsafe-loop-optimizations Common Var(warn_unsafe_loop_optimizations) Warning Warn if the loop cannot be optimized due to nontrivial assumptions. diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 38722840a8a..8d0ba18a79c 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,7 @@ +2007-03-09 Dirk Mueller + + * cp/call.c (build_new_op): Call warn_logical_operator. + 2007-03-08 Volker Reichelt PR c++/30852 diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 3514e62324b..fb4609edf7b 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -3694,6 +3694,7 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3, void *p; bool strict_p; bool any_viable_p; + bool expl_eq_arg1 = false; if (error_operand_p (arg1) || error_operand_p (arg2) @@ -3723,6 +3724,12 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3, case CALL_EXPR: return build_object_call (arg1, arg2); + case TRUTH_ORIF_EXPR: + case TRUTH_ANDIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + if (COMPARISON_CLASS_P (arg1)) + expl_eq_arg1 = true; default: break; } @@ -3930,6 +3937,12 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3, conv = conv->u.next; arg3 = convert_like (conv, arg3); } + + if (!expl_eq_arg1) + { + warn_logical_operator (code, arg1, arg2); + expl_eq_arg1 = true; + } } } @@ -3950,6 +3963,12 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3, case INDIRECT_REF: return build_indirect_ref (arg1, "unary *"); + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + if (!expl_eq_arg1) + warn_logical_operator (code, arg1, arg2); case PLUS_EXPR: case MINUS_EXPR: case MULT_EXPR: @@ -3968,8 +3987,6 @@ build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3, case BIT_AND_EXPR: case BIT_IOR_EXPR: case BIT_XOR_EXPR: - case TRUTH_ANDIF_EXPR: - case TRUTH_ORIF_EXPR: return cp_build_binary_op (code, arg1, arg2); case UNARY_PLUS_EXPR: diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 6ecde2e5563..98798844174 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -236,9 +236,9 @@ Objective-C and Objective-C++ Dialects}. -Wformat-security -Wformat-y2k @gol -Wimplicit -Wimplicit-function-declaration -Wimplicit-int @gol -Wimport -Wno-import -Winit-self -Winline @gol --Wno-int-to-pointer-cast @gol --Wno-invalid-offsetof -Winvalid-pch @gol --Wlarger-than-@var{len} -Wunsafe-loop-optimizations -Wlong-long @gol +-Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol +-Winvalid-pch -Wlarger-than-@var{len} -Wunsafe-loop-optimizations @gol +-Wlogical-op -Wlong-long @gol -Wmain -Wmissing-braces -Wmissing-field-initializers @gol -Wmissing-format-attribute -Wmissing-include-dirs @gol -Wmissing-noreturn @gol @@ -3420,6 +3420,13 @@ behavior and are not portable in C, so they usually indicate that the programmer intended to use @code{strcmp}. This warning is enabled by @option{-Wall}. +@item -Wlogical-op +@opindex Wlogical-op +@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. + @item -Waggregate-return @opindex Waggregate-return Warn if any functions that return structures or unions are defined or diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 212bb211034..75d1cff0fa5 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2007-03-09 Dirk Mueller + + PR c++/17946 + * gcc.dg/Wlogical-op-1.c: New. + * g++.dg/warn/Wlogical-op-1.C: New. + 2007-03-09 Richard Guenther PR tree-optimization/30904 diff --git a/gcc/testsuite/g++.dg/warn/Wlogical-op-1.C b/gcc/testsuite/g++.dg/warn/Wlogical-op-1.C new file mode 100644 index 00000000000..186f7ab0e7c --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wlogical-op-1.C @@ -0,0 +1,92 @@ +// { dg-do compile} +// { dg-options "-Wlogical-op" } + +enum { a, b }; + +enum testenum { t1, t2}; + +extern int c; +extern bool bool_a, bool_b; + +template +class QFlags +{ +public: + typedef void **Zero; + int i; + inline QFlags(Enum f) : i(f) {} + + inline operator int() const + { return i;} + +}; + +QFlags f(t2); +extern void do_something(int); + +extern testenum testa(); + +void foo() +{ + if ( f && b ) // { dg-warning "always evaluate as" } + do_something(1); + if ( c && b ) // { dg-warning "always evaluate as" } + do_something(2); + + if ( b && c == a ) // { dg-bogus "always evaluate as" } + do_something(101); + if ( 1 && c ) + do_something(102); // { dg-bogus "always evaluate as" } + if ( t2 && b ) // { dg-bogus "always evaluate as" } + do_something(103); + if ( true && c == a ) // { dg-bogus "always evaluate as" } + do_something(104); + if ( b && true ) // { dg-bogus "always evaluate as" } + do_something(105); +} +// { dg-do compile} +// { dg-options "-Winvariant-expr" } + +enum { a, b }; + +enum testenum { t1, t2}; + +extern int c; +extern bool bool_a, bool_b; + +template +class QFlags +{ +public: + typedef void **Zero; + int i; + inline QFlags(Enum f) : i(f) {} + + inline operator int() const + { return i;} + +}; + +QFlags f(t2); +extern void do_something(int); + +extern testenum testa(); + +void foo() +{ + if ( f && b ) // { dg-warning "always evaluate as" } + do_something(1); + if ( c && b ) // { dg-warning "always evaluate as" } + do_something(2); + + if ( b && c == a ) // { dg-bogus "always evaluate as" } + do_something(101); + if ( 1 && c ) + do_something(102); // { dg-bogus "always evaluate as" } + if ( t2 && b ) // { dg-bogus "always evaluate as" } + do_something(103); + if ( true && c == a ) // { dg-bogus "always evaluate as" } + do_something(104); + if ( b && true ) // { dg-bogus "always evaluate as" } + do_something(105); +} diff --git a/gcc/testsuite/gcc.dg/Wlogical-op-1.c b/gcc/testsuite/gcc.dg/Wlogical-op-1.c new file mode 100644 index 00000000000..154e8f493f5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wlogical-op-1.c @@ -0,0 +1,94 @@ +/* + { dg-do compile} + { dg-options "-Wlogical-op" } +*/ + +enum { a, ba, b }; + +enum testenum { t1, t2}; + +extern int c; +extern char bool_a, bool_b; + +extern int testa(); + +void foo() +{ + if ( testa() && b ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( c && b ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( c && 0x42 ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( c && 0x42 ) /* { dg-warning "always evaluate as" } */ + (void) testa(); + + if ( c && 0x80 >>6) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + + if ( b && c == a ) /* { dg-bogus "always evaluate as" } */ + (void)testa(); + + if ( 1 && c ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( t2 && b ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( 0 && c == a ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( b && 1 ) /* { dg-warning "always evaluate as" } */ + (void)testa(); +} +/* + { dg-do compile} + { dg-options "-Winvariant-expr" } +*/ + +enum { a, ba, b }; + +enum testenum { t1, t2}; + +extern int c; +extern char bool_a, bool_b; + +extern int testa(); + +void foo() +{ + if ( testa() && b ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( c && b ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( c && 0x42 ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( c && 0x42 ) /* { dg-warning "always evaluate as" } */ + (void) testa(); + + if ( c && 0x80 >>6) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + + if ( b && c == a ) /* { dg-bogus "always evaluate as" } */ + (void)testa(); + + if ( 1 && c ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( t2 && b ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( 0 && c == a ) /* { dg-warning "always evaluate as" } */ + (void)testa(); + + if ( b && 1 ) /* { dg-warning "always evaluate as" } */ + (void)testa(); +} -- 2.30.2