Patchwork [C] Warn if switch has boolean value (PR c/60439)

login
register
mail settings
Submitter Marek Polacek
Date April 18, 2014, 11:50 a.m.
Message ID <20140418115013.GK20332@redhat.com>
Download mbox | patch
Permalink /patch/340294/
State New
Headers show

Comments

Marek Polacek - April 18, 2014, 11:50 a.m.
On Fri, Apr 18, 2014 at 01:20:59PM +0200, Steven Bosscher wrote:
> On Fri, Apr 18, 2014 at 7:30 AM, Marek Polacek wrote:
> > +         if (TREE_CODE (type) == BOOLEAN_TYPE
> > +             || exp_code == TRUTH_ANDIF_EXPR
> > +             || exp_code == TRUTH_AND_EXPR
> > +             || exp_code == TRUTH_ORIF_EXPR
> > +             || exp_code == TRUTH_OR_EXPR
> > +             || exp_code == TRUTH_XOR_EXPR
> > +             || exp_code == TRUTH_NOT_EXPR
> > +             || exp_code == EQ_EXPR
> > +             || exp_code == NE_EXPR
> > +             || exp_code == LE_EXPR
> > +             || exp_code == GE_EXPR
> > +             || exp_code == LT_EXPR
> > +             || exp_code == GT_EXPR)
> 
> Is there a TREE_CODE_CLASS or a #define for this?

Good question.  I was looking for something nicer and found nothing,
no T_C_C or #define.
But now when I'm looking again, I found truth_value_p...  Thanks.

2014-04-18  Marek Polacek  <polacek@redhat.com>

	PR c/60439
	* doc/invoke.texi: Document -Wswitch-bool.
c/
	* c-typeck.c (c_start_case): Warn if switch condition has boolean
	value.
c-family/
	* c.opt (Wswitch-bool): New option.
testsuite/
	* gcc.dg/pr60439.c: New test.


	Marek
Jeff Law - May 2, 2014, 5:09 a.m.
On 04/18/14 05:50, Marek Polacek wrote:
> On Fri, Apr 18, 2014 at 01:20:59PM +0200, Steven Bosscher wrote:
>> On Fri, Apr 18, 2014 at 7:30 AM, Marek Polacek wrote:
>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE
>>> +             || exp_code == TRUTH_ANDIF_EXPR
>>> +             || exp_code == TRUTH_AND_EXPR
>>> +             || exp_code == TRUTH_ORIF_EXPR
>>> +             || exp_code == TRUTH_OR_EXPR
>>> +             || exp_code == TRUTH_XOR_EXPR
>>> +             || exp_code == TRUTH_NOT_EXPR
>>> +             || exp_code == EQ_EXPR
>>> +             || exp_code == NE_EXPR
>>> +             || exp_code == LE_EXPR
>>> +             || exp_code == GE_EXPR
>>> +             || exp_code == LT_EXPR
>>> +             || exp_code == GT_EXPR)
>>
>> Is there a TREE_CODE_CLASS or a #define for this?
>
> Good question.  I was looking for something nicer and found nothing,
> no T_C_C or #define.
> But now when I'm looking again, I found truth_value_p...  Thanks.
>
> 2014-04-18  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/60439
> 	* doc/invoke.texi: Document -Wswitch-bool.
> c/
> 	* c-typeck.c (c_start_case): Warn if switch condition has boolean
> 	value.
> c-family/
> 	* c.opt (Wswitch-bool): New option.
> testsuite/
> 	* gcc.dg/pr60439.c: New test.
Looks reasonable, though I do wonder if we should be warning for this in 
the C++ front-end as well?

jeff

Patch

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 390c056..9089496 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -529,6 +529,10 @@  Wswitch-enum
 C ObjC C++ ObjC++ Var(warn_switch_enum) Warning
 Warn about all enumerated switches missing a specific case
 
+Wswitch-bool
+C ObjC Warning Init(1)
+Warn about switches with boolean controlling expression
+
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Alias(Wsuggest-attribute=format)
 ;
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 65aad45..1b37f83 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9344,6 +9344,15 @@  c_start_case (location_t switch_loc,
       else
 	{
 	  tree type = TYPE_MAIN_VARIANT (orig_type);
+	  tree e = exp;
+
+	  while (TREE_CODE (e) == COMPOUND_EXPR)
+	    e = TREE_OPERAND (e, 1);
+
+	  if (TREE_CODE (type) == BOOLEAN_TYPE
+	      || truth_value_p (TREE_CODE (e)))
+	    warning_at (switch_cond_loc, OPT_Wswitch_bool,
+			"switch condition has boolean value");
 
 	  if (!in_system_header_at (input_location)
 	      && (type == long_integer_type_node
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 8004da8..04e1c41 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -268,7 +268,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
 -Wmissing-format-attribute @gol
--Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
+-Wswitch  -Wswitch-default  -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
 -Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
 -Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
@@ -3822,6 +3822,12 @@  between @option{-Wswitch} and this option is that this option gives a
 warning about an omitted enumeration code even if there is a
 @code{default} label.
 
+@item -Wswitch-bool
+@opindex Wswitch-bool
+@opindex Wno-switch-bool
+Warn whenever a @code{switch} statement has an index of boolean type.
+This warning is enabled by default for C programs.
+
 @item -Wsync-nand @r{(C and C++ only)}
 @opindex Wsync-nand
 @opindex Wno-sync-nand
diff --git gcc/testsuite/gcc.dg/pr60439.c gcc/testsuite/gcc.dg/pr60439.c
index e69de29..26e7c25 100644
--- gcc/testsuite/gcc.dg/pr60439.c
+++ gcc/testsuite/gcc.dg/pr60439.c
@@ -0,0 +1,112 @@ 
+/* PR c/60439 */
+/* { dg-do compile } */
+
+typedef _Bool bool;
+extern _Bool foo (void);
+
+void
+f1 (const _Bool b)
+{
+  switch (b) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+}
+
+void
+f2 (int a, int b)
+{
+  switch (a && b) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch ((bool) (a && b)) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch ((a && b) || a) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+}
+
+void
+f3 (int a)
+{
+  switch (!!a) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (!a) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+}
+
+void
+f4 (void)
+{
+  switch (foo ()) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+}
+
+void
+f5 (int a)
+{
+  switch (a == 3) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (a != 3) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (a > 3) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (a < 3) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (a <= 3) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (a >= 3) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (foo (), foo (), a >= 42) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (a == 3, a & 4, a ^ 5, a)
+    case 1:
+      break;
+}
+
+void
+f6 (bool b)
+{
+  switch (b) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (!b) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+  switch (b++) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+}
+
+void
+f7 (void)
+{
+  bool b;
+  switch (b = 1) /* { dg-warning "switch condition has boolean value" } */
+    case 1:
+      break;
+}
+
+void
+f8 (int i)
+{
+  switch (i)
+    case 0:
+      break;
+  switch ((unsigned int) i)
+    case 0:
+      break;
+  switch ((bool) i) /* { dg-warning "switch condition has boolean value" } */
+    case 0:
+      break;
+}