* C/C++ PATCH to implement -Wbool-operation (PR c/77490)
@ 2016-09-15 15:44 Marek Polacek
2016-09-15 22:50 ` Eric Gallager
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-09-15 15:44 UTC (permalink / raw)
To: GCC Patches, Jason Merrill, Joseph Myers
Now that the C++ FE boolean in/decrement changes are in, I can move forwards
with this patch which implements a new warning named -Wbool-operation (better
names?) which warns about nonsensical code such as ~bool, ~(i == 10), or
bool-- (in C).
It could also warn for other operations such as * or / or <<, but I wasn't
sure about those, so this isn't included in this patch.
Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?
2016-09-15 Marek Polacek <polacek@redhat.com>
PR c/77490
* c.opt (Wbool-operation): New.
* c-typeck.c (build_unary_op): Warn about bit not on expressions that
have boolean value. Warn about ++/-- on booleans.
* typeck.c (cp_build_unary_op): Warn about bit not on expressions that
have boolean value.
* doc/invoke.texi: Document -Wbool-operation.
* c-c++-common/Wbool-operation-1.c: New test.
* gcc.dg/Wall.c: Use -Wno-bool-operation.
* gcc.dg/Wbool-operation-1.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index c55c7c3..fb6f2d1 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -315,6 +315,10 @@ Wbool-compare
C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about boolean expression compared with an integer value different from true/false.
+Wbool-operation
+C ObjC C++ ObjC++ Var(warn_bool_op) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about certain operations on boolean expressions.
+
Wframe-address
C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 4dec397..c455d22 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4196,6 +4196,22 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
|| (typecode == VECTOR_TYPE
&& !VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg))))
{
+ tree e = arg;
+
+ /* Warn if the expression has boolean value. */
+ while (TREE_CODE (e) == COMPOUND_EXPR)
+ e = TREE_OPERAND (e, 1);
+
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ || truth_value_p (TREE_CODE (e)))
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on a boolean expression"))
+ {
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_insert_before (location, "!");
+ inform_at_rich_loc (&richloc, "did you mean to use logical "
+ "not?");
+ }
if (!noconvert)
arg = default_conversion (arg);
}
@@ -4306,6 +4322,16 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
"decrement of enumeration value is invalid in C++");
}
+ if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
+ {
+ if (code == PREINCREMENT_EXPR || code == POSTINCREMENT_EXPR)
+ warning_at (location, OPT_Wbool_operation,
+ "increment of a boolean expression");
+ else
+ warning_at (location, OPT_Wbool_operation,
+ "decrement of a boolean expression");
+ }
+
/* Ensure the argument is fully folded inside any SAVE_EXPR. */
arg = c_fully_fold (arg, false, NULL);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index c53a85a..9fc1a4b 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
arg, true)))
errstring = _("wrong type argument to bit-complement");
else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
- arg = cp_perform_integral_promotions (arg, complain);
+ {
+ /* Warn if the expression has boolean value. */
+ location_t location = EXPR_LOC_OR_LOC (arg, input_location);
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ || truth_value_p (TREE_CODE (arg)))
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on a boolean expression"))
+ inform (location, "did you mean to use logical not (%<!%>)?");
+ arg = cp_perform_integral_promotions (arg, complain);
+ }
break;
case ABS_EXPR:
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 8eb5eff..976e137 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -256,8 +256,8 @@ Objective-C and Objective-C++ Dialects}.
-pedantic-errors @gol
-w -Wextra -Wall -Waddress -Waggregate-return @gol
-Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
--Wc90-c99-compat -Wc99-c11-compat @gol
+-Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
-Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
-Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
-Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
@@ -3654,6 +3654,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
@gccoptlist{-Waddress @gol
-Warray-bounds=1 @r{(only with} @option{-O2}@r{)} @gol
-Wbool-compare @gol
+-Wbool-operation @gol
-Wc++11-compat -Wc++14-compat@gol
-Wchar-subscripts @gol
-Wcomment @gol
@@ -4761,6 +4762,16 @@ if ((n > 1) == 2) @{ @dots{} @}
@end smallexample
This warning is enabled by @option{-Wall}.
+@item -Wbool-operation
+@opindex Wno-bool-operation
+@opindex Wbool-operation
+Warn about suspicious operations on expressions of a boolean type. For
+instance, bitwise negation of a boolean is very likely a bug in the program.
+For C, this warning also warns about incrementing or decrementing a boolean,
+which rarely makes sense.
+
+This warning is enabled by @option{-Wall}.
+
@item -Wduplicated-cond
@opindex Wno-duplicated-cond
@opindex Wduplicated-cond
diff --git gcc/testsuite/c-c++-common/Wbool-operation-1.c gcc/testsuite/c-c++-common/Wbool-operation-1.c
index e69de29..287bd03 100644
--- gcc/testsuite/c-c++-common/Wbool-operation-1.c
+++ gcc/testsuite/c-c++-common/Wbool-operation-1.c
@@ -0,0 +1,37 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+typedef volatile bool T;
+typedef int __attribute__ ((vector_size (4 * sizeof (int)))) v4si;
+extern bool foo (void);
+
+int
+fn (bool b, bool b2, T b3, int n, v4si v)
+{
+ int r = 0;
+
+ r += ~b; /* { dg-warning "on a boolean expression" } */
+ r += n + ~b; /* { dg-warning "on a boolean expression" } */
+ r += ~(n == 1); /* { dg-warning "on a boolean expression" } */
+ r += ~(n || 1); /* { dg-warning "on a boolean expression" } */
+ r += ~b == 1; /* { dg-warning "on a boolean expression" } */
+ r += ~(++n, n == 1); /* { dg-warning "on a boolean expression" } */
+ r += ~(++n, n > 1); /* { dg-warning "on a boolean expression" } */
+ r += ~(++n, n && 1); /* { dg-warning "on a boolean expression" } */
+ r += (++n, ~b); /* { dg-warning "on a boolean expression" } */
+ r += (++n, n + ~b); /* { dg-warning "on a boolean expression" } */
+ r += ~b3; /* { dg-warning "on a boolean expression" } */
+ r += ~foo (); /* { dg-warning "on a boolean expression" } */
+ r += ~(bool) !1; /* { dg-warning "on a boolean expression" } */
+
+ v = ~v;
+ r += ~(int) b;
+ r += -b;
+
+ return r;
+}
diff --git gcc/testsuite/gcc.dg/Wall.c gcc/testsuite/gcc.dg/Wall.c
index 8984847..6c73533 100644
--- gcc/testsuite/gcc.dg/Wall.c
+++ gcc/testsuite/gcc.dg/Wall.c
@@ -1,7 +1,7 @@
/* PR 30437: Test -Wall
Don't change this without changing Wno-all.c as well. */
/* { dg-do compile } */
-/* { dg-options "-Wall" } */
+/* { dg-options "-Wall -Wno-bool-operation" } */
void foo(int a)
{
diff --git gcc/testsuite/gcc.dg/Wbool-operation-1.c gcc/testsuite/gcc.dg/Wbool-operation-1.c
index e69de29..b24e763 100644
--- gcc/testsuite/gcc.dg/Wbool-operation-1.c
+++ gcc/testsuite/gcc.dg/Wbool-operation-1.c
@@ -0,0 +1,16 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int
+fn (_Bool b)
+{
+ int r = 0;
+
+ r += b++; /* { dg-warning "increment of a boolean expression" } */
+ r += ++b; /* { dg-warning "increment of a boolean expression" } */
+ r += b--; /* { dg-warning "decrement of a boolean expression" } */
+ r += --b; /* { dg-warning "decrement of a boolean expression" } */
+
+ return r;
+}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-15 15:44 C/C++ PATCH to implement -Wbool-operation (PR c/77490) Marek Polacek
@ 2016-09-15 22:50 ` Eric Gallager
2016-09-16 9:03 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Eric Gallager @ 2016-09-15 22:50 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Jason Merrill, Joseph Myers
Hi, I have some comments/questions in-line:
On 9/15/16, Marek Polacek <polacek@redhat.com> wrote:
> Now that the C++ FE boolean in/decrement changes are in, I can move
> forwards with this patch which implements a new warning named -Wbool-operation
> (better names?) which warns about nonsensical code such as ~bool, ~(i == 10), or
> bool-- (in C).
>
> It could also warn for other operations such as * or / or <<, but I wasn't
> sure about those, so this isn't included in this patch.
>
> Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?
>
> 2016-09-15 Marek Polacek <polacek@redhat.com>
>
> PR c/77490
> * c.opt (Wbool-operation): New.
>
> * c-typeck.c (build_unary_op): Warn about bit not on expressions that
> have boolean value. Warn about ++/-- on booleans.
>
> * typeck.c (cp_build_unary_op): Warn about bit not on expressions that
> have boolean value.
>
> * doc/invoke.texi: Document -Wbool-operation.
>
> * c-c++-common/Wbool-operation-1.c: New test.
> * gcc.dg/Wall.c: Use -Wno-bool-operation.
> * gcc.dg/Wbool-operation-1.c: New test.
>
> diff --git gcc/c-family/c.opt gcc/c-family/c.opt
> index c55c7c3..fb6f2d1 100644
> --- gcc/c-family/c.opt
> +++ gcc/c-family/c.opt
> @@ -315,6 +315,10 @@ Wbool-compare
> C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++
> ObjC++,Wall)
> Warn about boolean expression compared with an integer value different from
> true/false.
>
> +Wbool-operation
> +C ObjC C++ ObjC++ Var(warn_bool_op) Warning LangEnabledBy(C ObjC C++
> ObjC++,Wall)
> +Warn about certain operations on boolean expressions.
> +
> Wframe-address
> C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++
> ObjC++,Wall)
> Warn when __builtin_frame_address or __builtin_return_address is used
> unsafely.
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index 4dec397..c455d22 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -4196,6 +4196,22 @@ build_unary_op (location_t location, enum tree_code
> code, tree xarg,
> || (typecode == VECTOR_TYPE
> && !VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg))))
> {
> + tree e = arg;
> +
> + /* Warn if the expression has boolean value. */
> + while (TREE_CODE (e) == COMPOUND_EXPR)
> + e = TREE_OPERAND (e, 1);
> +
> + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> + || truth_value_p (TREE_CODE (e)))
> + && warning_at (location, OPT_Wbool_operation,
> + "%<~%> on a boolean expression"))
> + {
> + gcc_rich_location richloc (location);
> + richloc.add_fixit_insert_before (location, "!");
> + inform_at_rich_loc (&richloc, "did you mean to use logical "
> + "not?");
> + }
> if (!noconvert)
> arg = default_conversion (arg);
> }
> @@ -4306,6 +4322,16 @@ build_unary_op (location_t location, enum tree_code
> code, tree xarg,
> "decrement of enumeration value is invalid in C++");
> }
>
> + if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
> + {
> + if (code == PREINCREMENT_EXPR || code == POSTINCREMENT_EXPR)
> + warning_at (location, OPT_Wbool_operation,
> + "increment of a boolean expression");
> + else
> + warning_at (location, OPT_Wbool_operation,
> + "decrement of a boolean expression");
> + }
> +
> /* Ensure the argument is fully folded inside any SAVE_EXPR. */
> arg = c_fully_fold (arg, false, NULL);
>
> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
> index c53a85a..9fc1a4b 100644
> --- gcc/cp/typeck.c
> +++ gcc/cp/typeck.c
> @@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg,
> bool noconvert,
> arg, true)))
> errstring = _("wrong type argument to bit-complement");
> else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> - arg = cp_perform_integral_promotions (arg, complain);
> + {
> + /* Warn if the expression has boolean value. */
> + location_t location = EXPR_LOC_OR_LOC (arg, input_location);
> + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> + || truth_value_p (TREE_CODE (arg)))
> + && warning_at (location, OPT_Wbool_operation,
> + "%<~%> on a boolean expression"))
> + inform (location, "did you mean to use logical not (%<!%>)?");
> + arg = cp_perform_integral_promotions (arg, complain);
> + }
> break;
>
> case ABS_EXPR:
> diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> index 8eb5eff..976e137 100644
> --- gcc/doc/invoke.texi
> +++ gcc/doc/invoke.texi
> @@ -256,8 +256,8 @@ Objective-C and Objective-C++ Dialects}.
> -pedantic-errors @gol
> -w -Wextra -Wall -Waddress -Waggregate-return @gol
> -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n}
> @gol
> --Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
> --Wc90-c99-compat -Wc99-c11-compat @gol
> +-Wno-attributes -Wbool-compare -Wbool-operation @gol
> +-Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
> -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
> -Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
> -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
> @@ -3654,6 +3654,7 @@ Options} and @ref{Objective-C and Objective-C++
> Dialect Options}.
> @gccoptlist{-Waddress @gol
> -Warray-bounds=1 @r{(only with} @option{-O2}@r{)} @gol
> -Wbool-compare @gol
> +-Wbool-operation @gol
> -Wc++11-compat -Wc++14-compat@gol
> -Wchar-subscripts @gol
> -Wcomment @gol
> @@ -4761,6 +4762,16 @@ if ((n > 1) == 2) @{ @dots{} @}
> @end smallexample
> This warning is enabled by @option{-Wall}.
>
> +@item -Wbool-operation
> +@opindex Wno-bool-operation
> +@opindex Wbool-operation
> +Warn about suspicious operations on expressions of a boolean type. For
> +instance, bitwise negation of a boolean is very likely a bug in the program.
> +For C, this warning also warns about incrementing or decrementing a boolean,
> +which rarely makes sense.
> +
I'd also mention that C++ now warns about incrementing and
decrementing anyways, even without this option.
> +This warning is enabled by @option{-Wall}.
> +
> @item -Wduplicated-cond
> @opindex Wno-duplicated-cond
> @opindex Wduplicated-cond
> diff --git gcc/testsuite/c-c++-common/Wbool-operation-1.c
> gcc/testsuite/c-c++-common/Wbool-operation-1.c
> index e69de29..287bd03 100644
> --- gcc/testsuite/c-c++-common/Wbool-operation-1.c
> +++ gcc/testsuite/c-c++-common/Wbool-operation-1.c
> @@ -0,0 +1,37 @@
> +/* PR c/77490 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall" } */
> +
> +#ifndef __cplusplus
> +# define bool _Bool
> +#endif
> +
> +typedef volatile bool T;
> +typedef int __attribute__ ((vector_size (4 * sizeof (int)))) v4si;
> +extern bool foo (void);
> +
> +int
> +fn (bool b, bool b2, T b3, int n, v4si v)
> +{
> + int r = 0;
> +
> + r += ~b; /* { dg-warning "on a boolean expression" } */
> + r += n + ~b; /* { dg-warning "on a boolean expression" } */
> + r += ~(n == 1); /* { dg-warning "on a boolean expression" } */
> + r += ~(n || 1); /* { dg-warning "on a boolean expression" } */
> + r += ~b == 1; /* { dg-warning "on a boolean expression" } */
> + r += ~(++n, n == 1); /* { dg-warning "on a boolean expression" } */
> + r += ~(++n, n > 1); /* { dg-warning "on a boolean expression" } */
> + r += ~(++n, n && 1); /* { dg-warning "on a boolean expression" } */
> + r += (++n, ~b); /* { dg-warning "on a boolean expression" } */
> + r += (++n, n + ~b); /* { dg-warning "on a boolean expression" } */
> + r += ~b3; /* { dg-warning "on a boolean expression" } */
> + r += ~foo (); /* { dg-warning "on a boolean expression" } */
> + r += ~(bool) !1; /* { dg-warning "on a boolean expression" } */
> +
> + v = ~v;
> + r += ~(int) b;
> + r += -b;
> +
> + return r;
> +}
> diff --git gcc/testsuite/gcc.dg/Wall.c gcc/testsuite/gcc.dg/Wall.c
> index 8984847..6c73533 100644
> --- gcc/testsuite/gcc.dg/Wall.c
> +++ gcc/testsuite/gcc.dg/Wall.c
> @@ -1,7 +1,7 @@
> /* PR 30437: Test -Wall
> Don't change this without changing Wno-all.c as well. */
> /* { dg-do compile } */
> -/* { dg-options "-Wall" } */
> +/* { dg-options "-Wall -Wno-bool-operation" } */
>
Why did -Wno-bool-operation need to be added here?
> void foo(int a)
> {
> diff --git gcc/testsuite/gcc.dg/Wbool-operation-1.c
> gcc/testsuite/gcc.dg/Wbool-operation-1.c
> index e69de29..b24e763 100644
> --- gcc/testsuite/gcc.dg/Wbool-operation-1.c
> +++ gcc/testsuite/gcc.dg/Wbool-operation-1.c
> @@ -0,0 +1,16 @@
> +/* PR c/77490 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall" } */
> +
> +int
> +fn (_Bool b)
> +{
> + int r = 0;
> +
> + r += b++; /* { dg-warning "increment of a boolean expression" } */
> + r += ++b; /* { dg-warning "increment of a boolean expression" } */
> + r += b--; /* { dg-warning "decrement of a boolean expression" } */
> + r += --b; /* { dg-warning "decrement of a boolean expression" } */
> +
> + return r;
> +}
>
> Marek
>
Anyways, thanks for your work on this; I hope it goes in soon!
Eric
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-15 22:50 ` Eric Gallager
@ 2016-09-16 9:03 ` Marek Polacek
2016-09-19 19:18 ` Jason Merrill
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-09-16 9:03 UTC (permalink / raw)
To: Eric Gallager; +Cc: GCC Patches, Jason Merrill, Joseph Myers
On Thu, Sep 15, 2016 at 04:45:03PM -0400, Eric Gallager wrote:
> > +@item -Wbool-operation
> > +@opindex Wno-bool-operation
> > +@opindex Wbool-operation
> > +Warn about suspicious operations on expressions of a boolean type. For
> > +instance, bitwise negation of a boolean is very likely a bug in the program.
> > +For C, this warning also warns about incrementing or decrementing a boolean,
> > +which rarely makes sense.
> > +
>
>
> I'd also mention that C++ now warns about incrementing and
> decrementing anyways, even without this option.
Ok, I could have added that. Done now.
> > --- gcc/testsuite/gcc.dg/Wall.c
> > +++ gcc/testsuite/gcc.dg/Wall.c
> > @@ -1,7 +1,7 @@
> > /* PR 30437: Test -Wall
> > Don't change this without changing Wno-all.c as well. */
> > /* { dg-do compile } */
> > -/* { dg-options "-Wall" } */
> > +/* { dg-options "-Wall -Wno-bool-operation" } */
> >
>
>
> Why did -Wno-bool-operation need to be added here?
Oops, that is a mistake. This is a remnant from the previous version.
Removed.
> Anyways, thanks for your work on this; I hope it goes in soon!
Thanks.
Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?
2016-09-16 Marek Polacek <polacek@redhat.com>
PR c/77490
* c.opt (Wbool-operation): New.
* c-typeck.c (build_unary_op): Warn about bit not on expressions that
have boolean value. Warn about ++/-- on booleans.
* typeck.c (cp_build_unary_op): Warn about bit not on expressions that
have boolean value.
* doc/invoke.texi: Document -Wbool-operation.
* c-c++-common/Wbool-operation-1.c: New test.
* gcc.dg/Wbool-operation-1.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index c55c7c3..fb6f2d1 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -315,6 +315,10 @@ Wbool-compare
C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about boolean expression compared with an integer value different from true/false.
+Wbool-operation
+C ObjC C++ ObjC++ Var(warn_bool_op) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about certain operations on boolean expressions.
+
Wframe-address
C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 4dec397..c455d22 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4196,6 +4196,22 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
|| (typecode == VECTOR_TYPE
&& !VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg))))
{
+ tree e = arg;
+
+ /* Warn if the expression has boolean value. */
+ while (TREE_CODE (e) == COMPOUND_EXPR)
+ e = TREE_OPERAND (e, 1);
+
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ || truth_value_p (TREE_CODE (e)))
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on a boolean expression"))
+ {
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_insert_before (location, "!");
+ inform_at_rich_loc (&richloc, "did you mean to use logical "
+ "not?");
+ }
if (!noconvert)
arg = default_conversion (arg);
}
@@ -4306,6 +4322,16 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
"decrement of enumeration value is invalid in C++");
}
+ if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
+ {
+ if (code == PREINCREMENT_EXPR || code == POSTINCREMENT_EXPR)
+ warning_at (location, OPT_Wbool_operation,
+ "increment of a boolean expression");
+ else
+ warning_at (location, OPT_Wbool_operation,
+ "decrement of a boolean expression");
+ }
+
/* Ensure the argument is fully folded inside any SAVE_EXPR. */
arg = c_fully_fold (arg, false, NULL);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index c53a85a..9fc1a4b 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
arg, true)))
errstring = _("wrong type argument to bit-complement");
else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
- arg = cp_perform_integral_promotions (arg, complain);
+ {
+ /* Warn if the expression has boolean value. */
+ location_t location = EXPR_LOC_OR_LOC (arg, input_location);
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ || truth_value_p (TREE_CODE (arg)))
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on a boolean expression"))
+ inform (location, "did you mean to use logical not (%<!%>)?");
+ arg = cp_perform_integral_promotions (arg, complain);
+ }
break;
case ABS_EXPR:
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 8eb5eff..1c3ca3f 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -256,8 +256,8 @@ Objective-C and Objective-C++ Dialects}.
-pedantic-errors @gol
-w -Wextra -Wall -Waddress -Waggregate-return @gol
-Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
--Wc90-c99-compat -Wc99-c11-compat @gol
+-Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
-Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
-Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
-Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
@@ -3654,6 +3654,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
@gccoptlist{-Waddress @gol
-Warray-bounds=1 @r{(only with} @option{-O2}@r{)} @gol
-Wbool-compare @gol
+-Wbool-operation @gol
-Wc++11-compat -Wc++14-compat@gol
-Wchar-subscripts @gol
-Wcomment @gol
@@ -4761,6 +4762,17 @@ if ((n > 1) == 2) @{ @dots{} @}
@end smallexample
This warning is enabled by @option{-Wall}.
+@item -Wbool-operation
+@opindex Wno-bool-operation
+@opindex Wbool-operation
+Warn about suspicious operations on expressions of a boolean type. For
+instance, bitwise negation of a boolean is very likely a bug in the program.
+For C, this warning also warns about incrementing or decrementing a boolean,
+which rarely makes sense. (In C++, decrementing a boolean is always invalid.
+Incrementing a boolean is invalid in C++1z, and deprecated otherwise.)
+
+This warning is enabled by @option{-Wall}.
+
@item -Wduplicated-cond
@opindex Wno-duplicated-cond
@opindex Wduplicated-cond
diff --git gcc/testsuite/c-c++-common/Wbool-operation-1.c gcc/testsuite/c-c++-common/Wbool-operation-1.c
index e69de29..287bd03 100644
--- gcc/testsuite/c-c++-common/Wbool-operation-1.c
+++ gcc/testsuite/c-c++-common/Wbool-operation-1.c
@@ -0,0 +1,37 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+typedef volatile bool T;
+typedef int __attribute__ ((vector_size (4 * sizeof (int)))) v4si;
+extern bool foo (void);
+
+int
+fn (bool b, bool b2, T b3, int n, v4si v)
+{
+ int r = 0;
+
+ r += ~b; /* { dg-warning "on a boolean expression" } */
+ r += n + ~b; /* { dg-warning "on a boolean expression" } */
+ r += ~(n == 1); /* { dg-warning "on a boolean expression" } */
+ r += ~(n || 1); /* { dg-warning "on a boolean expression" } */
+ r += ~b == 1; /* { dg-warning "on a boolean expression" } */
+ r += ~(++n, n == 1); /* { dg-warning "on a boolean expression" } */
+ r += ~(++n, n > 1); /* { dg-warning "on a boolean expression" } */
+ r += ~(++n, n && 1); /* { dg-warning "on a boolean expression" } */
+ r += (++n, ~b); /* { dg-warning "on a boolean expression" } */
+ r += (++n, n + ~b); /* { dg-warning "on a boolean expression" } */
+ r += ~b3; /* { dg-warning "on a boolean expression" } */
+ r += ~foo (); /* { dg-warning "on a boolean expression" } */
+ r += ~(bool) !1; /* { dg-warning "on a boolean expression" } */
+
+ v = ~v;
+ r += ~(int) b;
+ r += -b;
+
+ return r;
+}
diff --git gcc/testsuite/gcc.dg/Wbool-operation-1.c gcc/testsuite/gcc.dg/Wbool-operation-1.c
index e69de29..b24e763 100644
--- gcc/testsuite/gcc.dg/Wbool-operation-1.c
+++ gcc/testsuite/gcc.dg/Wbool-operation-1.c
@@ -0,0 +1,16 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int
+fn (_Bool b)
+{
+ int r = 0;
+
+ r += b++; /* { dg-warning "increment of a boolean expression" } */
+ r += ++b; /* { dg-warning "increment of a boolean expression" } */
+ r += b--; /* { dg-warning "decrement of a boolean expression" } */
+ r += --b; /* { dg-warning "decrement of a boolean expression" } */
+
+ return r;
+}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-16 9:03 ` Marek Polacek
@ 2016-09-19 19:18 ` Jason Merrill
2016-09-20 17:27 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2016-09-19 19:18 UTC (permalink / raw)
To: Marek Polacek, Eric Gallager; +Cc: GCC Patches, Joseph Myers
On 09/16/2016 05:01 AM, Marek Polacek wrote:
> @@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
> arg, true)))
> errstring = _("wrong type argument to bit-complement");
> else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> - arg = cp_perform_integral_promotions (arg, complain);
> + {
> + /* Warn if the expression has boolean value. */
> + location_t location = EXPR_LOC_OR_LOC (arg, input_location);
Let's move this variable to the beginning of the function; hopefully it
will become a parameter soon.
> + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> + || truth_value_p (TREE_CODE (arg)))
You shouldn't need to check truth_value_p; in C++ all truth operations
have type bool.
> + && warning_at (location, OPT_Wbool_operation,
> + "%<~%> on a boolean expression"))
And let's refer to "expression of type bool" rather than "boolean
expression".
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-19 19:18 ` Jason Merrill
@ 2016-09-20 17:27 ` Marek Polacek
2016-09-21 15:43 ` Jason Merrill
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-09-20 17:27 UTC (permalink / raw)
To: Jason Merrill; +Cc: Eric Gallager, GCC Patches, Joseph Myers
On Mon, Sep 19, 2016 at 02:41:04PM -0400, Jason Merrill wrote:
> On 09/16/2016 05:01 AM, Marek Polacek wrote:
> > @@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
> > arg, true)))
> > errstring = _("wrong type argument to bit-complement");
> > else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> > - arg = cp_perform_integral_promotions (arg, complain);
> > + {
> > + /* Warn if the expression has boolean value. */
> > + location_t location = EXPR_LOC_OR_LOC (arg, input_location);
>
> Let's move this variable to the beginning of the function; hopefully it will
> become a parameter soon.
Done.
> > + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> > + || truth_value_p (TREE_CODE (arg)))
>
> You shouldn't need to check truth_value_p; in C++ all truth operations have
> type bool.
Oops, force of habit...
> > + && warning_at (location, OPT_Wbool_operation,
> > + "%<~%> on a boolean expression"))
>
> And let's refer to "expression of type bool" rather than "boolean
> expression".
Adjusted (and in the C FE too).
Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?
2016-09-20 Marek Polacek <polacek@redhat.com>
PR c/77490
* c.opt (Wbool-operation): New.
* c-typeck.c (build_unary_op): Warn about bit not on expressions that
have boolean value. Warn about ++/-- on booleans.
* typeck.c (cp_build_unary_op): Warn about bit not on expressions that
have boolean value.
* doc/invoke.texi: Document -Wbool-operation.
* c-c++-common/Wbool-operation-1.c: New test.
* gcc.dg/Wbool-operation-1.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 6cf915d..0e8d68a 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -315,6 +315,10 @@ Wbool-compare
C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about boolean expression compared with an integer value different from true/false.
+Wbool-operation
+C ObjC C++ ObjC++ Var(warn_bool_op) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about certain operations on boolean expressions.
+
Wframe-address
C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 059ad1f..4006b1a 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4196,6 +4196,22 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
|| (typecode == VECTOR_TYPE
&& !VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg))))
{
+ tree e = arg;
+
+ /* Warn if the expression has boolean value. */
+ while (TREE_CODE (e) == COMPOUND_EXPR)
+ e = TREE_OPERAND (e, 1);
+
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ || truth_value_p (TREE_CODE (e)))
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on an expression of type bool"))
+ {
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_insert_before (location, "!");
+ inform_at_rich_loc (&richloc, "did you mean to use logical "
+ "not?");
+ }
if (!noconvert)
arg = default_conversion (arg);
}
@@ -4306,6 +4322,16 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
"decrement of enumeration value is invalid in C++");
}
+ if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
+ {
+ if (code == PREINCREMENT_EXPR || code == POSTINCREMENT_EXPR)
+ warning_at (location, OPT_Wbool_operation,
+ "increment of an expression of type bool");
+ else
+ warning_at (location, OPT_Wbool_operation,
+ "decrement of an expression of type bool");
+ }
+
/* Ensure the argument is fully folded inside any SAVE_EXPR. */
arg = c_fully_fold (arg, false, NULL);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index c53a85a..dee17b3 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -5792,6 +5792,7 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
{
/* No default_conversion here. It causes trouble for ADDR_EXPR. */
tree arg = xarg;
+ location_t location = EXPR_LOC_OR_LOC (arg, input_location);
tree argtype = 0;
const char *errstring = NULL;
tree val;
@@ -5853,7 +5854,14 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
arg, true)))
errstring = _("wrong type argument to bit-complement");
else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
- arg = cp_perform_integral_promotions (arg, complain);
+ {
+ /* Warn if the expression has boolean value. */
+ if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on an expression of type bool"))
+ inform (location, "did you mean to use logical not (%<!%>)?");
+ arg = cp_perform_integral_promotions (arg, complain);
+ }
break;
case ABS_EXPR:
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 3c27283..6e8218c 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -256,8 +256,8 @@ Objective-C and Objective-C++ Dialects}.
-pedantic-errors @gol
-w -Wextra -Wall -Waddress -Waggregate-return @gol
-Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
--Wc90-c99-compat -Wc99-c11-compat @gol
+-Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
-Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
-Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
-Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
@@ -3654,6 +3654,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
@gccoptlist{-Waddress @gol
-Warray-bounds=1 @r{(only with} @option{-O2}@r{)} @gol
-Wbool-compare @gol
+-Wbool-operation @gol
-Wc++11-compat -Wc++14-compat@gol
-Wchar-subscripts @gol
-Wcomment @gol
@@ -4762,6 +4763,17 @@ if ((n > 1) == 2) @{ @dots{} @}
@end smallexample
This warning is enabled by @option{-Wall}.
+@item -Wbool-operation
+@opindex Wno-bool-operation
+@opindex Wbool-operation
+Warn about suspicious operations on expressions of a boolean type. For
+instance, bitwise negation of a boolean is very likely a bug in the program.
+For C, this warning also warns about incrementing or decrementing a boolean,
+which rarely makes sense. (In C++, decrementing a boolean is always invalid.
+Incrementing a boolean is invalid in C++1z, and deprecated otherwise.)
+
+This warning is enabled by @option{-Wall}.
+
@item -Wduplicated-cond
@opindex Wno-duplicated-cond
@opindex Wduplicated-cond
diff --git gcc/testsuite/c-c++-common/Wbool-operation-1.c gcc/testsuite/c-c++-common/Wbool-operation-1.c
index e69de29..e81e89d 100644
--- gcc/testsuite/c-c++-common/Wbool-operation-1.c
+++ gcc/testsuite/c-c++-common/Wbool-operation-1.c
@@ -0,0 +1,37 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+typedef volatile bool T;
+typedef int __attribute__ ((vector_size (4 * sizeof (int)))) v4si;
+extern bool foo (void);
+
+int
+fn (bool b, bool b2, T b3, int n, v4si v)
+{
+ int r = 0;
+
+ r += ~b; /* { dg-warning "on an expression of type bool" } */
+ r += n + ~b; /* { dg-warning "on an expression of type bool" } */
+ r += ~(n == 1); /* { dg-warning "on an expression of type bool" } */
+ r += ~(n || 1); /* { dg-warning "on an expression of type bool" } */
+ r += ~b == 1; /* { dg-warning "on an expression of type bool" } */
+ r += ~(++n, n == 1); /* { dg-warning "on an expression of type bool" } */
+ r += ~(++n, n > 1); /* { dg-warning "on an expression of type bool" } */
+ r += ~(++n, n && 1); /* { dg-warning "on an expression of type bool" } */
+ r += (++n, ~b); /* { dg-warning "on an expression of type bool" } */
+ r += (++n, n + ~b); /* { dg-warning "on an expression of type bool" } */
+ r += ~b3; /* { dg-warning "on an expression of type bool" } */
+ r += ~foo (); /* { dg-warning "on an expression of type bool" } */
+ r += ~(bool) !1; /* { dg-warning "on an expression of type bool" } */
+
+ v = ~v;
+ r += ~(int) b;
+ r += -b;
+
+ return r;
+}
diff --git gcc/testsuite/gcc.dg/Wbool-operation-1.c gcc/testsuite/gcc.dg/Wbool-operation-1.c
index e69de29..f3acc54 100644
--- gcc/testsuite/gcc.dg/Wbool-operation-1.c
+++ gcc/testsuite/gcc.dg/Wbool-operation-1.c
@@ -0,0 +1,16 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int
+fn (_Bool b)
+{
+ int r = 0;
+
+ r += b++; /* { dg-warning "increment of an expression of type bool" } */
+ r += ++b; /* { dg-warning "increment of an expression of type bool" } */
+ r += b--; /* { dg-warning "decrement of an expression of type bool" } */
+ r += --b; /* { dg-warning "decrement of an expression of type bool" } */
+
+ return r;
+}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-20 17:27 ` Marek Polacek
@ 2016-09-21 15:43 ` Jason Merrill
2016-09-21 15:44 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2016-09-21 15:43 UTC (permalink / raw)
To: Marek Polacek; +Cc: Eric Gallager, GCC Patches, Joseph Myers
On 09/20/2016 01:13 PM, Marek Polacek wrote:
> On Mon, Sep 19, 2016 at 02:41:04PM -0400, Jason Merrill wrote:
>> On 09/16/2016 05:01 AM, Marek Polacek wrote:
>>> @@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
>>> arg, true)))
>>> errstring = _("wrong type argument to bit-complement");
>>> else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
>>> - arg = cp_perform_integral_promotions (arg, complain);
>>> + {
>>> + /* Warn if the expression has boolean value. */
>>> + location_t location = EXPR_LOC_OR_LOC (arg, input_location);
>>
>> Let's move this variable to the beginning of the function; hopefully it will
>> become a parameter soon.
>
> Done.
>
>>> + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
>>> + || truth_value_p (TREE_CODE (arg)))
>>
>> You shouldn't need to check truth_value_p; in C++ all truth operations have
>> type bool.
>
> Oops, force of habit...
>
>>> + && warning_at (location, OPT_Wbool_operation,
>>> + "%<~%> on a boolean expression"))
>>
>> And let's refer to "expression of type bool" rather than "boolean
>> expression".
>
> Adjusted (and in the C FE too).
Hmm, I'm not sure that change is right for C. But the C++ hunk is OK.
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-21 15:43 ` Jason Merrill
@ 2016-09-21 15:44 ` Marek Polacek
2016-09-22 17:26 ` Joseph Myers
0 siblings, 1 reply; 11+ messages in thread
From: Marek Polacek @ 2016-09-21 15:44 UTC (permalink / raw)
To: Jason Merrill; +Cc: Eric Gallager, GCC Patches, Joseph Myers
On Wed, Sep 21, 2016 at 11:37:32AM -0400, Jason Merrill wrote:
> On 09/20/2016 01:13 PM, Marek Polacek wrote:
> > On Mon, Sep 19, 2016 at 02:41:04PM -0400, Jason Merrill wrote:
> > > On 09/16/2016 05:01 AM, Marek Polacek wrote:
> > > > @@ -5853,7 +5853,16 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
> > > > arg, true)))
> > > > errstring = _("wrong type argument to bit-complement");
> > > > else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> > > > - arg = cp_perform_integral_promotions (arg, complain);
> > > > + {
> > > > + /* Warn if the expression has boolean value. */
> > > > + location_t location = EXPR_LOC_OR_LOC (arg, input_location);
> > >
> > > Let's move this variable to the beginning of the function; hopefully it will
> > > become a parameter soon.
> >
> > Done.
> >
> > > > + if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> > > > + || truth_value_p (TREE_CODE (arg)))
> > >
> > > You shouldn't need to check truth_value_p; in C++ all truth operations have
> > > type bool.
> >
> > Oops, force of habit...
> >
> > > > + && warning_at (location, OPT_Wbool_operation,
> > > > + "%<~%> on a boolean expression"))
> > >
> > > And let's refer to "expression of type bool" rather than "boolean
> > > expression".
> >
> > Adjusted (and in the C FE too).
>
> Hmm, I'm not sure that change is right for C. But the C++ hunk is OK.
Thanks. Joseph, how about the C part?
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-21 15:44 ` Marek Polacek
@ 2016-09-22 17:26 ` Joseph Myers
2016-09-23 11:14 ` Marek Polacek
0 siblings, 1 reply; 11+ messages in thread
From: Joseph Myers @ 2016-09-22 17:26 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, Eric Gallager, GCC Patches
On Wed, 21 Sep 2016, Marek Polacek wrote:
> > > > And let's refer to "expression of type bool" rather than "boolean
> > > > expression".
> > >
> > > Adjusted (and in the C FE too).
> >
> > Hmm, I'm not sure that change is right for C. But the C++ hunk is OK.
>
> Thanks. Joseph, how about the C part?
You shouldn't refer to "type bool" for C. If it's the result of a boolean
operation, the type is int; an expression has type _Bool only if cast to
that type or if it's a variable of that type.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-22 17:26 ` Joseph Myers
@ 2016-09-23 11:14 ` Marek Polacek
2016-09-24 0:12 ` Joseph Myers
[not found] ` <20160930234601.GK7282@tucnak.redhat.com>
0 siblings, 2 replies; 11+ messages in thread
From: Marek Polacek @ 2016-09-23 11:14 UTC (permalink / raw)
To: Joseph Myers; +Cc: Jason Merrill, Eric Gallager, GCC Patches
On Thu, Sep 22, 2016 at 05:24:31PM +0000, Joseph Myers wrote:
> On Wed, 21 Sep 2016, Marek Polacek wrote:
>
> > > > > And let's refer to "expression of type bool" rather than "boolean
> > > > > expression".
> > > >
> > > > Adjusted (and in the C FE too).
> > >
> > > Hmm, I'm not sure that change is right for C. But the C++ hunk is OK.
> >
> > Thanks. Joseph, how about the C part?
>
> You shouldn't refer to "type bool" for C. If it's the result of a boolean
> operation, the type is int; an expression has type _Bool only if cast to
> that type or if it's a variable of that type.
Ok, so how about this one?
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-09-23 Marek Polacek <polacek@redhat.com>
PR c/77490
* c.opt (Wbool-operation): New.
* c-typeck.c (build_unary_op): Warn about bit not on expressions that
have boolean value. Warn about ++/-- on booleans.
* typeck.c (cp_build_unary_op): Warn about bit not on expressions that
have boolean value.
* doc/invoke.texi: Document -Wbool-operation.
* c-c++-common/Wbool-operation-1.c: New test.
* gcc.dg/Wbool-operation-1.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index a04618d..4a9b9e8 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -315,6 +315,10 @@ Wbool-compare
C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about boolean expression compared with an integer value different from true/false.
+Wbool-operation
+C ObjC C++ ObjC++ Var(warn_bool_op) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about certain operations on boolean expressions.
+
Wframe-address
C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 059ad1f..e5c7256 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4196,6 +4196,22 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
|| (typecode == VECTOR_TYPE
&& !VECTOR_FLOAT_TYPE_P (TREE_TYPE (arg))))
{
+ tree e = arg;
+
+ /* Warn if the expression has boolean value. */
+ while (TREE_CODE (e) == COMPOUND_EXPR)
+ e = TREE_OPERAND (e, 1);
+
+ if ((TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ || truth_value_p (TREE_CODE (e)))
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on a boolean expression"))
+ {
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_insert_before (location, "!");
+ inform_at_rich_loc (&richloc, "did you mean to use logical "
+ "not?");
+ }
if (!noconvert)
arg = default_conversion (arg);
}
@@ -4306,6 +4322,16 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
"decrement of enumeration value is invalid in C++");
}
+ if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
+ {
+ if (code == PREINCREMENT_EXPR || code == POSTINCREMENT_EXPR)
+ warning_at (location, OPT_Wbool_operation,
+ "increment of a boolean expression");
+ else
+ warning_at (location, OPT_Wbool_operation,
+ "decrement of a boolean expression");
+ }
+
/* Ensure the argument is fully folded inside any SAVE_EXPR. */
arg = c_fully_fold (arg, false, NULL);
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index c53a85a..dee17b3 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -5792,6 +5792,7 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
{
/* No default_conversion here. It causes trouble for ADDR_EXPR. */
tree arg = xarg;
+ location_t location = EXPR_LOC_OR_LOC (arg, input_location);
tree argtype = 0;
const char *errstring = NULL;
tree val;
@@ -5853,7 +5854,14 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool noconvert,
arg, true)))
errstring = _("wrong type argument to bit-complement");
else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
- arg = cp_perform_integral_promotions (arg, complain);
+ {
+ /* Warn if the expression has boolean value. */
+ if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+ && warning_at (location, OPT_Wbool_operation,
+ "%<~%> on an expression of type bool"))
+ inform (location, "did you mean to use logical not (%<!%>)?");
+ arg = cp_perform_integral_promotions (arg, complain);
+ }
break;
case ABS_EXPR:
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index e6f503f..a5481b5 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -256,8 +256,8 @@ Objective-C and Objective-C++ Dialects}.
-pedantic-errors @gol
-w -Wextra -Wall -Waddress -Waggregate-return @gol
-Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
--Wc90-c99-compat -Wc99-c11-compat @gol
+-Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
-Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
-Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
-Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
@@ -3656,6 +3656,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
@gccoptlist{-Waddress @gol
-Warray-bounds=1 @r{(only with} @option{-O2}@r{)} @gol
-Wbool-compare @gol
+-Wbool-operation @gol
-Wc++11-compat -Wc++14-compat@gol
-Wchar-subscripts @gol
-Wcomment @gol
@@ -4846,6 +4847,17 @@ if ((n > 1) == 2) @{ @dots{} @}
@end smallexample
This warning is enabled by @option{-Wall}.
+@item -Wbool-operation
+@opindex Wno-bool-operation
+@opindex Wbool-operation
+Warn about suspicious operations on expressions of a boolean type. For
+instance, bitwise negation of a boolean is very likely a bug in the program.
+For C, this warning also warns about incrementing or decrementing a boolean,
+which rarely makes sense. (In C++, decrementing a boolean is always invalid.
+Incrementing a boolean is invalid in C++1z, and deprecated otherwise.)
+
+This warning is enabled by @option{-Wall}.
+
@item -Wduplicated-cond
@opindex Wno-duplicated-cond
@opindex Wduplicated-cond
diff --git gcc/testsuite/c-c++-common/Wbool-operation-1.c gcc/testsuite/c-c++-common/Wbool-operation-1.c
index e69de29..bb72784 100644
--- gcc/testsuite/c-c++-common/Wbool-operation-1.c
+++ gcc/testsuite/c-c++-common/Wbool-operation-1.c
@@ -0,0 +1,36 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+typedef volatile bool T;
+typedef int __attribute__ ((vector_size (4 * sizeof (int)))) v4si;
+extern bool foo (void);
+
+int
+fn (bool b, bool b2, T b3, int n, v4si v)
+{
+ int r = 0;
+
+ r += ~b; /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += n + ~b; /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~(n == 1); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~(n || 1); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~b == 1; /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~(++n, n == 1); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~(++n, n > 1); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~(++n, n && 1); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += (++n, ~b); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~b3; /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~foo (); /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+ r += ~(bool) !1; /* { dg-warning "on an expression of type bool|on a boolean expression" } */
+
+ v = ~v;
+ r += ~(int) b;
+ r += -b;
+
+ return r;
+}
diff --git gcc/testsuite/gcc.dg/Wbool-operation-1.c gcc/testsuite/gcc.dg/Wbool-operation-1.c
index e69de29..b24e763 100644
--- gcc/testsuite/gcc.dg/Wbool-operation-1.c
+++ gcc/testsuite/gcc.dg/Wbool-operation-1.c
@@ -0,0 +1,16 @@
+/* PR c/77490 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int
+fn (_Bool b)
+{
+ int r = 0;
+
+ r += b++; /* { dg-warning "increment of a boolean expression" } */
+ r += ++b; /* { dg-warning "increment of a boolean expression" } */
+ r += b--; /* { dg-warning "decrement of a boolean expression" } */
+ r += --b; /* { dg-warning "decrement of a boolean expression" } */
+
+ return r;
+}
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
2016-09-23 11:14 ` Marek Polacek
@ 2016-09-24 0:12 ` Joseph Myers
[not found] ` <20160930234601.GK7282@tucnak.redhat.com>
1 sibling, 0 replies; 11+ messages in thread
From: Joseph Myers @ 2016-09-24 0:12 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jason Merrill, Eric Gallager, GCC Patches
On Fri, 23 Sep 2016, Marek Polacek wrote:
> On Thu, Sep 22, 2016 at 05:24:31PM +0000, Joseph Myers wrote:
> > On Wed, 21 Sep 2016, Marek Polacek wrote:
> >
> > > > > > And let's refer to "expression of type bool" rather than "boolean
> > > > > > expression".
> > > > >
> > > > > Adjusted (and in the C FE too).
> > > >
> > > > Hmm, I'm not sure that change is right for C. But the C++ hunk is OK.
> > >
> > > Thanks. Joseph, how about the C part?
> >
> > You shouldn't refer to "type bool" for C. If it's the result of a boolean
> > operation, the type is int; an expression has type _Bool only if cast to
> > that type or if it's a variable of that type.
>
> Ok, so how about this one?
The C changes here are OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: C/C++ PATCH to implement -Wbool-operation (PR c/77490)
[not found] ` <20160930234601.GK7282@tucnak.redhat.com>
@ 2016-10-01 11:34 ` Marek Polacek
0 siblings, 0 replies; 11+ messages in thread
From: Marek Polacek @ 2016-10-01 11:34 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Joseph Myers, Jason Merrill, Eric Gallager, GCC Patches
On Sat, Oct 01, 2016 at 01:46:02AM +0200, Jakub Jelinek wrote:
> On Fri, Sep 23, 2016 at 12:44:22PM +0200, Marek Polacek wrote:
> > 2016-09-23 Marek Polacek <polacek@redhat.com>
> >
> > PR c/77490
> ...
> > * c-c++-common/Wbool-operation-1.c: New test.
>
> I've noticed this test fails on i686-linux and likely also on powerpc-linux.
Sorry. I must remember this when testing sth with vectors...
Marek
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-10-01 11:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 15:44 C/C++ PATCH to implement -Wbool-operation (PR c/77490) Marek Polacek
2016-09-15 22:50 ` Eric Gallager
2016-09-16 9:03 ` Marek Polacek
2016-09-19 19:18 ` Jason Merrill
2016-09-20 17:27 ` Marek Polacek
2016-09-21 15:43 ` Jason Merrill
2016-09-21 15:44 ` Marek Polacek
2016-09-22 17:26 ` Joseph Myers
2016-09-23 11:14 ` Marek Polacek
2016-09-24 0:12 ` Joseph Myers
[not found] ` <20160930234601.GK7282@tucnak.redhat.com>
2016-10-01 11:34 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).