PATCH RFA: Add -Wc++-compat warning for va_arg with enum type

PATCH RFA: Add -Wc++-compat warning for va_arg with enum type

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]

In C++ using va_arg with an enum type leads to a warning from
gimplify_va_arg_expr in builtins.c:

foo.cc: In function ‘int foo(int, ...)’:
foo.cc:7: warning: ‘E’ is promoted to ‘int’ when passed through ‘...’
foo.cc:7: note: (so you should pass ‘int’ not ‘E’ to ‘va_arg’)
foo.cc:7: note: if this code is reached, the program will abort

The warning itself is vaguely language independent, and is issued when
the type passed to va_arg is not the same as that type when promoted.
In C there is no warning, because enum types are basically int, so
va_arg (ap, enum E) is more or less the same as va_arg (ap, int).

The first patch in this message adds a -Wc++-compat warning to the C
frontend when using va_arg with an enum type.  This patch requires
approval from the C frontend maintainers.

The second patch fixes the three cases in gcc where this occurs.  I
believe that this patch counts as obvious given the first patch.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  I also built the
arm, ia64, mips, pa, rs6000, s390, sh, sparc and spu backends with the
new compiler, and found no additional warnings.

OK for mainline?

Ian


gcc/ChangeLog:

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

	* c-typeck.c (c_build_va_arg): New function.
	* c-tree.h (c_build_va_arg): Declare.
	* c-parser.c (c_parser_postfix_expression): Call c_build_va_arg
	instead of build_va_arg.

gcc/testsuite/ChangeLog:

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

	* gcc.dg/Wcxx-compat-11.c: New testcase.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: -Wc++-compat warning about va_arg with enum type --]
[-- Type: text/x-patch, Size: 2619 bytes --]

Index: c-parser.c
===================================================================
--- c-parser.c	(revision 147927)
+++ c-parser.c	(working copy)
@@ -5338,6 +5338,7 @@ c_parser_postfix_expression (c_parser *p
 	      expr.value = error_mark_node;
 	      break;
 	    }
+	  loc = c_parser_peek_token (parser)->location;
 	  t1 = c_parser_type_name (parser);
 	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
 				     "expected %<)%>");
@@ -5348,9 +5349,9 @@ c_parser_postfix_expression (c_parser *p
 	  else
 	    {
 	      tree type_expr = NULL_TREE;
-	      expr.value = build_va_arg (e1.value, groktypename (t1,
-								 &type_expr,
-								 NULL));
+	      expr.value = c_build_va_arg (e1.value,
+					   groktypename (t1, &type_expr, NULL),
+					   loc);
 	      if (type_expr)
 		{
 		  expr.value = build2 (C_MAYBE_CONST_EXPR,
Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 147927)
+++ c-typeck.c	(working copy)
@@ -9976,3 +9976,14 @@ c_build_qualified_type (tree type, int t
 
   return build_qualified_type (type, type_quals);
 }
+
+/* Build a VA_ARG_EXPR for the C parser.  */
+
+tree
+c_build_va_arg (tree expr, tree type, location_t loc)
+{
+  if (warn_cxx_compat && TREE_CODE (type) == ENUMERAL_TYPE)
+    warning_at (loc, OPT_Wc___compat,
+		"C++ requires promoted type, not enum type, in %<va_arg%>");
+  return build_va_arg (expr, type);
+}
Index: c-tree.h
===================================================================
--- c-tree.h	(revision 147927)
+++ c-tree.h	(working copy)
@@ -650,6 +650,7 @@ extern tree c_finish_omp_parallel (tree,
 extern tree c_begin_omp_task (void);
 extern tree c_finish_omp_task (tree, tree);
 extern tree c_finish_omp_clauses (tree);
+extern tree c_build_va_arg (tree, tree, location_t);
 
 /* Set to 0 at beginning of a function definition, set to 1 if
    a return statement that specifies a return value is seen.  */
Index: testsuite/gcc.dg/Wcxx-compat-11.c
===================================================================
--- testsuite/gcc.dg/Wcxx-compat-11.c	(revision 0)
+++ testsuite/gcc.dg/Wcxx-compat-11.c	(revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-Wc++-compat" } */
+
+#include <stdarg.h>
+
+enum E { A };
+
+extern void f2 (int);
+void
+f1 (int n, ...)
+{
+  va_list ap;
+
+  va_start (ap, n);
+  f2 (va_arg (ap, int));
+  f2 (va_arg (ap, _Bool));	/* { dg-warning "promoted" } */
+  f2 (va_arg (ap, enum E));	/* { dg-warning "promoted" } */
+}
+
+/* Match extra informative notes.  */
+/* { dg-message "note:" "expected" { target *-*-* } 0 } */

[-- Attachment #3: Type: text/plain, Size: 374 bytes --]




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

gcc/ChangeLog:

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

	* builtins.c (validate_gimple_arglist): Don't use va_arg with
	enum type.
	* calls.c (emit_library_call_value_1): Likewise.

gcc/cp/ChangeLog:

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

	* error.c (cp_printer): Don't use va_arg with enum type.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: Don't use va_arg with enum type --]
[-- Type: text/x-patch, Size: 1785 bytes --]

Index: builtins.c
===================================================================
--- builtins.c	(revision 147927)
+++ builtins.c	(working copy)
@@ -11231,7 +11231,7 @@ validate_gimple_arglist (const_gimple ca
 
   do
     {
-      code = va_arg (ap, enum tree_code);
+      code = (enum tree_code) va_arg (ap, int);
       switch (code)
 	{
 	case 0:
@@ -11282,7 +11282,7 @@ validate_arglist (const_tree callexpr, .
 
   do
     {
-      code = va_arg (ap, enum tree_code);
+      code = (enum tree_code) va_arg (ap, int);
       switch (code)
 	{
 	case 0:
Index: cp/error.c
===================================================================
--- cp/error.c	(revision 147927)
+++ cp/error.c	(working copy)
@@ -2761,8 +2761,8 @@ cp_printer (pretty_printer *pp, text_inf
   const char *result;
   tree t = NULL;
 #define next_tree    (t = va_arg (*text->args_ptr, tree))
-#define next_tcode   va_arg (*text->args_ptr, enum tree_code)
-#define next_lang    va_arg (*text->args_ptr, enum languages)
+#define next_tcode   ((enum tree_code) va_arg (*text->args_ptr, int))
+#define next_lang    ((enum languages) va_arg (*text->args_ptr, int))
 #define next_int     va_arg (*text->args_ptr, int)
 
   if (precision != 0 || wide)
Index: calls.c
===================================================================
--- calls.c	(revision 147927)
+++ calls.c	(working copy)
@@ -3445,7 +3445,7 @@ emit_library_call_value_1 (int retval, r
   for (; count < nargs; count++)
     {
       rtx val = va_arg (p, rtx);
-      enum machine_mode mode = va_arg (p, enum machine_mode);
+      enum machine_mode mode = (enum machine_mode) va_arg (p, int);
 
       /* We cannot convert the arg value to the mode the library wants here;
 	 must do it earlier where we know the signedness of the arg.  */

Re: PATCH RFA: Add -Wc++-compat warning for va_arg with enum type

[Joseph S. Myers]

On Thu, 28 May 2009, Ian Lance Taylor wrote:

> 2009-05-28  Ian Lance Taylor  <iant@google.com>
> 
> 	* c-typeck.c (c_build_va_arg): New function.
> 	* c-tree.h (c_build_va_arg): Declare.
> 	* c-parser.c (c_parser_postfix_expression): Call c_build_va_arg
> 	instead of build_va_arg.
> 
> gcc/testsuite/ChangeLog:
> 
> 2009-05-28  Ian Lance Taylor  <iant@google.com>
> 
> 	* gcc.dg/Wcxx-compat-11.c: New testcase.

These C changes are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com