re PR c/17946 (wanted: warning for "a && MASK" when "a & MASK" was probably intended)

2007-03-09  Dirk Mueller  <dmueller@suse.de>

       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

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6aa85152ee064714e0e20b41e4da02c0097a8b94..7bd6d741ea151b4548e7907580d7e537abb077e7 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2007-03-09  Dirk Mueller  <dmueller@suse.de>
+
+       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  <aoliva@redhat.com>
 
        * 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 0bcd462ddfc08466fcd3d5fccad0e946359f8c8c..6216d5e123a78316df315733854fa8e7c2328945 100644 (file)
--- 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 2eedf79d29d6e706e16301f62aae91fadbdab5c8..f47fa8e38150c9da75e5d6519e7181fd92f9c52f 100644 (file)
--- 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 8afc2c757ebf30295ddcb62436750c51c4976117..e45c5e0bf971f0c7f32a5dd46f71aaac51e3d5c2 100644 (file)
--- 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 938ea598d54431c0d0c2ec03f72159fe9d502790..bcf0fcd4ec82bed87c92d04cd561c56797c8e678 100644 (file)
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -118,6 +118,10 @@ Wlarger-than-
 Common RejectNegative Joined UInteger Warning
 -Wlarger-than-<number> Warn if an object is larger than <number> 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 38722840a8a73d9642d9bf846686c4eb0409f602..8d0ba18a79cd11dda644ed6bd21330ade7007995 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,7 @@
+2007-03-09  Dirk Mueller  <dmueller@suse.de>
+
+       * cp/call.c (build_new_op): Call warn_logical_operator.
+
 2007-03-08  Volker Reichelt  <reichelt@netcologne.de>
 
        PR c++/30852

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 3514e62324be44fc7e2bc76ac4bc09f29fe1aec6..fb4609edf7b6dc7a180a1c751b902ba4cb05c633 100644 (file)
--- 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 6ecde2e55630dc3fbd8b84b1b57a6650dc2907fa..987988441746a273926377b67a8c54d06fca5a9d 100644 (file)
--- 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 212bb2110349a20a75e45b3deb4e55e82d007ca5..75d1cff0fa5f2d91a02b426b30e06a55df131b4a 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2007-03-09  Dirk Mueller  <dmueller@suse.de>
+
+       PR c++/17946
+       * gcc.dg/Wlogical-op-1.c: New.
+       * g++.dg/warn/Wlogical-op-1.C: New.
+
 2007-03-09  Richard Guenther  <rguenther@suse.de>
 
        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 (file)
index 0000000..186f7ab
--- /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<typename Enum>
+class QFlags
+{
+public:
+    typedef void **Zero;
+    int i;
+    inline QFlags(Enum f) : i(f) {}
+
+    inline operator int() const
+    { return i;}
+
+};
+
+QFlags<testenum> 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<typename Enum>
+class QFlags
+{
+public:
+    typedef void **Zero;
+    int i;
+    inline QFlags(Enum f) : i(f) {}
+
+    inline operator int() const
+    { return i;}
+
+};
+
+QFlags<testenum> 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 (file)
index 0000000..154e8f4
--- /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();
+}