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

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.

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

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);

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)

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.

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

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:

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

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

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);
+}

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();
+}