PATCH RFA: Change -Wswitch behaviour when there is a default case

[Ian Lance Taylor <iant at google dot com>]

-Wswitch issues two warnings about switches on values of enum type.
First, it warns if the switch omits any value of the enum type.  Second,
it warns if the switch includes any value which is not of the enum type.

Currently, both warnings are disabled if the switch has a default case.
This does not make sense.  It makes sense to disable the first warning
if the switch has a default label.  However, it does not make sense to
disable the second warning.  A case statement using a value which is not
of the enum type does not make more sense because there is a default
case.

I think the current gcc behaviour is simply a historical legacy of how
the code happened to work.  I can't think of any good reason for it to
work this way.  Therefore, I propose that we change it.

The first patch changes the behaviour of -Wswitch.  This patch requires
approval from the C or C++ frontend maintainers.

The second patch fixes the two cases in the gcc sources for which the
first patch gives new warnings.  This patch requires approval from the
Java frontend maintainers.  I could have simply used casts in the switch
statements, but I applied what I thought would be better fixes.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for mainline?

Ian

gcc/ChangeLog:

2009-06-05  Ian Lance Taylor  <iant@google.com>

	* dwarf2.h (enum dwarf_location_atom): Add
	INTERNAL_DW_OP_tls_addr.
	* dwarf2out.c (INTERNAL_DW_OP_tls_addr): Don't #define.

	* c-common.c (c_do_switch_warnings): Don't exit early for -Wswitch
	with no default node.  Change warning with %H to warning_at.
	Don't clear warn_switch around case checking.
	* doc/invoke.texi (Warning Options): Clarify distinction between
	-Wswitch and -Wswitch-enum.

gcc/testsuite/ChangeLog:

2009-06-05  Ian Lance Taylor  <iant@google.com>

	* g++.dg/warn/Wswitch-3.C: New testcase.
	* gcc.dg/Wswitch.c: Adjust for -Wswitch change.
	* gcc.dg/Wswitch-enum-error.c: Likewise.
	* gcc.dg/Wswitch-error.c: Likewise.


Index: c-common.c
===================================================================
--- c-common.c	(revision 148209)
+++ c-common.c	(working copy)
@@ -5425,7 +5425,6 @@ c_do_switch_warnings (splay_tree cases, 
   splay_tree_node default_node;
   splay_tree_node node;
   tree chain;
-  int saved_warn_switch;
 
   if (!warn_switch && !warn_switch_enum && !warn_switch_default)
     return;
@@ -5439,15 +5438,15 @@ c_do_switch_warnings (splay_tree cases, 
   if (!type || TREE_CODE (type) != ENUMERAL_TYPE)
     return;
 
-  /* If the switch expression was an enumerated type, check that
-     exactly all enumeration literals are covered by the cases.
-     The check is made when -Wswitch was specified and there is no
-     default case, or when -Wswitch-enum was specified.  */
-
-  if (!warn_switch_enum
-      && !(warn_switch && !default_node))
+  /* From here on, we only care about -Wswitch and -Wswitch-enum.  */
+  if (!warn_switch_enum && !warn_switch)
     return;
 
+  /* Check the cases.  Warn about case values which are not members of
+     the enumerated type.  For -Wswitch-enum, or for -Wswitch when
+     there is no default case, check that exactly all enumeration
+     literals are covered by the cases.  */
+
   /* Clearing COND if it is not an integer constant simplifies
      the tests inside the loop below.  */
   if (TREE_CODE (cond) != INTEGER_CST)
@@ -5498,13 +5497,15 @@ c_do_switch_warnings (splay_tree cases, 
 	continue;
 
       /* If there is a default_node, the only relevant option is
-	 Wswitch-enum. Otherwise, if both are enabled then we prefer
+	 Wswitch-enum.  Otherwise, if both are enabled then we prefer
 	 to warn using -Wswitch because -Wswitch is enabled by -Wall
 	 while -Wswitch-enum is explicit.  */
-      warning ((default_node || !warn_switch) 
-	       ? OPT_Wswitch_enum : OPT_Wswitch,
-	       "%Henumeration value %qE not handled in switch",
-	       &switch_location, TREE_PURPOSE (chain));
+      warning_at (switch_location,
+		  (default_node || !warn_switch
+		   ? OPT_Wswitch_enum
+		   : OPT_Wswitch),
+		  "enumeration value %qE not handled in switch",
+		  TREE_PURPOSE (chain));
     }
 
   /* Warn if there are case expressions that don't correspond to
@@ -5516,16 +5517,7 @@ c_do_switch_warnings (splay_tree cases, 
      every disjoint case label, with CASE_LOW_SEEN and CASE_HIGH_SEEN
      above.  This scan also resets those fields.  */
 
-  /* If there is a default_node, the only relevant option is
-     Wswitch-enum. Otherwise, if both are enabled then we prefer
-     to warn using -Wswitch because -Wswitch is enabled by -Wall
-     while -Wswitch-enum is explicit.  */
-  saved_warn_switch = warn_switch;
-  if (default_node)
-    warn_switch = 0;
   splay_tree_foreach (cases, match_case_to_enum, type);
-  warn_switch = saved_warn_switch;
-
 }
 
 /* Finish an expression taking the address of LABEL (an
Index: testsuite/gcc.dg/Wswitch.c
===================================================================
--- testsuite/gcc.dg/Wswitch.c	(revision 148209)
+++ testsuite/gcc.dg/Wswitch.c	(working copy)
@@ -56,8 +56,8 @@ foo (int i, int j, enum e ei, enum e ej,
     {
     case e1: return 1;
     case e2: return 2;
-    case 3: return 3;
+    case 3: return 3; /* { dg-warning "case value '3' not in enumerated type 'enum e'" "excess 3" } */
     default: break;
-    } /* Since there is a default, no warning about ``case 3'' */
+    }
   return 0;
 }
Index: testsuite/gcc.dg/Wswitch-enum-error.c
===================================================================
--- testsuite/gcc.dg/Wswitch-enum-error.c	(revision 148209)
+++ testsuite/gcc.dg/Wswitch-enum-error.c	(working copy)
@@ -56,7 +56,7 @@ foo (int i, int j, enum e ei, enum e ej,
     {
     case e1: return 1;
     case e2: return 2;
-    case 3: return 3; /* { dg-error "case value '3' not in enumerated type 'enum e'" "excess 3" } */
+    case 3: return 3; /* { dg-warning "case value '3' not in enumerated type 'enum e'" "excess 3" } */
     default: break;
     }
   return 0;
Index: testsuite/gcc.dg/Wswitch-error.c
===================================================================
--- testsuite/gcc.dg/Wswitch-error.c	(revision 148209)
+++ testsuite/gcc.dg/Wswitch-error.c	(working copy)
@@ -56,7 +56,7 @@ foo (int i, int j, enum e ei, enum e ej,
     {
     case e1: return 1;
     case e2: return 2;
-    case 3: return 3; /* { dg-warning "case value '3' not in enumerated type 'enum e'" "excess 3" } */
+    case 3: return 3; /* { dg-error "case value '3' not in enumerated type 'enum e'" "excess 3" } */
     default: break;
     }
   return 0;
Index: testsuite/g++.dg/warn/Wswitch-3.C
===================================================================
--- testsuite/g++.dg/warn/Wswitch-3.C	(revision 0)
+++ testsuite/g++.dg/warn/Wswitch-3.C	(revision 0)
@@ -0,0 +1,37 @@
+/* { dg-do compile } */
+/* { dg-options "-Wswitch" } */
+
+enum E { A, B, C, D, E, F };
+
+int
+f1 (enum E e)
+{
+  switch (e)
+    {
+    case A: return 1;
+    case B: return 2;
+    case C: return 3;
+    case D: return 4;
+    case E: return 5;
+    case F: return 6;
+    case 7: return 7;	/* { dg-warning "not in enumerated type" } */
+    }
+  return 0;
+}
+
+int
+f2 (enum E e)
+{
+  switch (e)
+    {
+    case A: return 1;
+    case B: return 2;
+    case C: return 3;
+    case D: return 4;
+    case E: return 5;
+    case F: return 6;
+    case 7: return 7;	/* { dg-warning "not in enumerated type" } */
+    default: return 8;
+    }
+  return 0;
+}
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 148209)
+++ doc/invoke.texi	(working copy)
@@ -3174,7 +3174,8 @@ Warn whenever a @code{switch} statement 
 and lacks a @code{case} for one or more of the named codes of that
 enumeration.  (The presence of a @code{default} label prevents this
 warning.)  @code{case} labels outside the enumeration range also
-provoke warnings when this option is used.
+provoke warnings when this option is used (even if there is a
+@code{default} label).
 This warning is enabled by @option{-Wall}.
 
 @item -Wswitch-default
@@ -3189,7 +3190,10 @@ case.
 Warn whenever a @code{switch} statement has an index of enumerated type
 and lacks a @code{case} for one or more of the named codes of that
 enumeration.  @code{case} labels outside the enumeration range also
-provoke warnings when this option is used.
+provoke warnings when this option is used.  The only difference
+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 -Wsync-nand @r{(C and C++ only)}
 @opindex Wsync-nand


==================================================

gcc/ChangeLog:

2009-06-05  Ian Lance Taylor  <iant@google.com>

	* dwarf2.h (enum dwarf_location_atom): Add
	INTERNAL_DW_OP_tls_addr.
	* dwarf2out.c (INTERNAL_DW_OP_tls_addr): Don't #define.

gcc/java/ChangeLog:

2009-06-05  Ian Lance Taylor  <iant@google.com>

	* jcf-parse.c (handle_constant): Change local variable 'kind' to
	unsigned int.


Index: java/jcf-parse.c
===================================================================
--- java/jcf-parse.c	(revision 148209)
+++ java/jcf-parse.c	(working copy)
@@ -498,7 +498,7 @@ handle_long_constant (JCF *jcf, CPool *c
 static uint16
 handle_constant (JCF *jcf, int index, enum cpool_tag purpose)
 {
-  enum cpool_tag kind;
+  unsigned int kind;
   CPool *cpool = cpool_for_class (output_class);
   
   if (index == 0)
@@ -507,7 +507,7 @@ handle_constant (JCF *jcf, int index, en
   if (! CPOOL_INDEX_IN_RANGE (&jcf->cpool, index))
     error ("<constant pool index %d not in range>", index);
   
-  kind = (enum cpool_tag) JPOOL_TAG (jcf, index);
+  kind = JPOOL_TAG (jcf, index);
 
   if ((kind & ~CONSTANT_ResolvedFlag) != purpose)
     {
@@ -555,12 +555,12 @@ handle_constant (JCF *jcf, int index, en
       break;
 
     case CONSTANT_Long:
-      index = handle_long_constant (jcf, cpool, kind, index, 
+      index = handle_long_constant (jcf, cpool, CONSTANT_Long, index,
 				    WORDS_BIG_ENDIAN);
       break;
       
     case CONSTANT_Double:
-      index = handle_long_constant (jcf, cpool, kind, index, 
+      index = handle_long_constant (jcf, cpool, CONSTANT_Double, index,
 				    FLOAT_WORDS_BIG_ENDIAN);
       break;
 
Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 148209)
+++ dwarf2out.c	(working copy)
@@ -3758,11 +3758,6 @@ struct GTY(()) dwarf_file_data {
   int emitted_number;
 };
 
-/* We need some way to distinguish DW_OP_addr with a direct symbol
-   relocation from DW_OP_addr with a dtp-relative symbol relocation.  */
-#define INTERNAL_DW_OP_tls_addr		(0x100 + DW_OP_addr)
-
-
 typedef struct dw_val_struct *dw_val_ref;
 typedef struct die_struct *dw_die_ref;
 typedef const struct die_struct *const_dw_die_ref;
Index: dwarf2.h
===================================================================
--- dwarf2.h	(revision 148209)
+++ dwarf2.h	(working copy)
@@ -562,7 +562,13 @@ enum dwarf_location_atom
     DW_OP_HP_fltconst8   = 0xe3,
     DW_OP_HP_mod_range   = 0xe4,
     DW_OP_HP_unmod_range = 0xe5,
-    DW_OP_HP_tls         = 0xe6
+    DW_OP_HP_tls         = 0xe6,
+
+    /* Used internally in dwarf2out.c to distinguish DW_OP_addr with a
+       direct symbol relocation from DW_OP_addr with a dtp-relative
+       symbol relocation.  */
+    INTERNAL_DW_OP_tls_addr = 0x103
+
   };
 
 /* Type encodings.  */