diff mbox

Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)

Message ID 20151204175343.GR5675@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 4, 2015, 5:53 p.m. UTC 
On Fri, Dec 04, 2015 at 06:01:58PM +0100, Bernd Schmidt wrote:
> I think marking stuff with Warning as appropriate qualifies as obvious.
> 
> On 12/04/2015 05:37 PM, Jakub Jelinek wrote:
> >+	  /* If the switch takes an integer, convert it.  */
> >+	  if (arg && cl_options[opt_index].cl_uinteger)
> >+	    {
> >+	      value = integral_argument (arg);
> >+	      if (value == -1)
> >+		return;
> >+	    }
> 
> So does this issue an error message anywhere or just silently drop the
> option on the floor if the argument is invalid?

Silently accepted.  Following updated patch accepts e.g.:
-Wlarger-than=5
-Werror=larger-than=5
-Wnormalized=none
-Werror=normalized=none
and rejects (with the same diagnostics between -WXXX=YYY and -Werror=XXX=YYY):
-Wlarger-than=none
-Werror=larger-than=none
-Wlarger-than=
-Werror=larger-than=
-Wnormalized=all
-Werror=normalized=all
-Wnormalized=
-Werror=normalized=
(as examples of Warning UInteger Joined and Warning Enum Joined options).

> >+	  /* If the switch takes an enumerated argument, convert it.  */
> >+	  if (arg && (cl_options[opt_index].var_type == CLVC_ENUM))
> 
> Unnecessary parens.

Fixed.

2015-12-04  Jakub Jelinek  <jakub@redhat.com>

	PR c/48088
	PR c/68657
	* common.opt (Wframe-larger-than=): Add Warning.
	* opts.h (control_warning_option): Add ARG argument.
	* opts-common.c (control_warning_option): Likewise.
	If non-NULL, decode it if needed and pass through
	to handle_generated_option.  Handle CLVC_ENUM like
	CLVC_BOOLEAN.
	* opts.c (common_handle_option): Adjust control_warning_option
	caller.
	(enable_warning_as_error): Likewise.
c-family/
	* c.opt (Wfloat-conversion, Wsign-conversion): Add Warning.
	* c-pragma.c (handle_pragma_diagnostic): Adjust
	control_warning_option caller.
ada/
	* gcc-interface/trans.c (Pragma_to_gnu): Adjust
	control_warning_option caller.
testsuite/
	* c-c++-common/pr68657-1.c: New test.
	* c-c++-common/pr68657-2.c: New test.
	* c-c++-common/pr68657-3.c: New test.



	Jakub

Comments

Manuel López-Ibáñez Dec. 4, 2015, 6:19 p.m. UTC | #1 
On 4 December 2015 at 17:53, Jakub Jelinek <jakub@redhat.com> wrote:
> +
> +                 if (e->unknown_error)
> +                   error_at (loc, e->unknown_error, option->opt_text);
> +                 else
> +                   error_at (loc, "unrecognized argument in option %qs",
> +                             option->opt_text);

The same code that handles command-line options has:

      if (e->unknown_error)
    error_at (loc, e->unknown_error, decoded->arg);
      else
    error_at (loc, "unrecognized argument in option %qs", opt);

My guess is that the first error_at should use arg instead of
option->opt_text to be equivalent. Of course, ideally, this code would
not be duplicated, but rather merged "somehow".

Cheers,

Manuel.
Jakub Jelinek Dec. 4, 2015, 7:36 p.m. UTC | #2 
On Fri, Dec 04, 2015 at 06:19:19PM +0000, Manuel López-Ibáñez wrote:
> On 4 December 2015 at 17:53, Jakub Jelinek <jakub@redhat.com> wrote:
> > +
> > +                 if (e->unknown_error)
> > +                   error_at (loc, e->unknown_error, option->opt_text);
> > +                 else
> > +                   error_at (loc, "unrecognized argument in option %qs",
> > +                             option->opt_text);
> 
> The same code that handles command-line options has:
> 
>       if (e->unknown_error)
>     error_at (loc, e->unknown_error, decoded->arg);
>       else
>     error_at (loc, "unrecognized argument in option %qs", opt);
> 
> My guess is that the first error_at should use arg instead of
> option->opt_text to be equivalent. Of course, ideally, this code would
> not be duplicated, but rather merged "somehow".

Consider that fixed.  As for duplication, as one operates on
cl_decoded_option and the other not etc., this is harder, plus
the missing and non-int cases are IMHO short enough that it is not worth
trying hard to avoid the duplication.
For the enum case which is larger, it is maybe worth adding
a helper routine for it, which would need probably only
location_t loc, const struct cl_enum *e, const char *opt, unsigned int lang_mask
arguments.  Can try that on Monday.

	Jakub
Bernd Schmidt Dec. 7, 2015, 10:24 a.m. UTC | #3 
On 12/04/2015 08:36 PM, Jakub Jelinek wrote:
> On Fri, Dec 04, 2015 at 06:19:19PM +0000, Manuel López-Ibáñez wrote:
>> My guess is that the first error_at should use arg instead of
>> option->opt_text to be equivalent. Of course, ideally, this code would
>> not be duplicated, but rather merged "somehow".
>
> Consider that fixed.  As for duplication, as one operates on
> cl_decoded_option and the other not etc., this is harder, plus
> the missing and non-int cases are IMHO short enough that it is not worth
> trying hard to avoid the duplication.
> For the enum case which is larger, it is maybe worth adding
> a helper routine for it, which would need probably only
> location_t loc, const struct cl_enum *e, const char *opt, unsigned int lang_mask
> arguments.  Can try that on Monday.

Maybe you can split the error printing code out of read_cmdline_option. 
For the original patch I noticed the duplication but figured it was not 
enough to really worry about, but for the error handling I think we 
should make an effort.


Bernd
diff mbox

Patch

--- gcc/common.opt.jj	2015-12-04 17:19:01.873180339 +0100
+++ gcc/common.opt	2015-12-04 18:07:31.901544973 +0100
@@ -576,7 +576,7 @@  Common Var(flag_fatal_errors)
 Exit on the first error occurred.
 
 Wframe-larger-than=
-Common RejectNegative Joined UInteger
+Common RejectNegative Joined UInteger Warning
 -Wframe-larger-than=<number>	Warn if a function's stack frame requires more than <number> bytes.
 
 Wfree-nonheap-object
--- gcc/opts.h.jj	2015-12-04 17:19:01.939179455 +0100
+++ gcc/opts.h	2015-12-04 18:07:31.901544973 +0100
@@ -363,7 +363,7 @@  extern void read_cmdline_option (struct
 				 const struct cl_option_handlers *handlers,
 				 diagnostic_context *dc);
 extern void control_warning_option (unsigned int opt_index, int kind,
-				    bool imply, location_t loc,
+				    const char *arg, bool imply, location_t loc,
 				    unsigned int lang_mask,
 				    const struct cl_option_handlers *handlers,
 				    struct gcc_options *opts,
--- gcc/opts-common.c.jj	2015-12-04 17:19:01.854180594 +0100
+++ gcc/opts-common.c	2015-12-04 18:44:15.956606061 +0100
@@ -1332,8 +1332,8 @@  get_option_state (struct gcc_options *op
    used by -Werror= and #pragma GCC diagnostic.  */
 
 void
-control_warning_option (unsigned int opt_index, int kind, bool imply,
-			location_t loc, unsigned int lang_mask,
+control_warning_option (unsigned int opt_index, int kind, const char *arg,
+			bool imply, location_t loc, unsigned int lang_mask,
 			const struct cl_option_handlers *handlers,
 			struct gcc_options *opts,
 			struct gcc_options *opts_set,
@@ -1347,10 +1347,89 @@  control_warning_option (unsigned int opt
     diagnostic_classify_diagnostic (dc, opt_index, (diagnostic_t) kind, loc);
   if (imply)
     {
+      const struct cl_option *option = &cl_options[opt_index];
+
       /* -Werror=foo implies -Wfoo.  */
-      if (cl_options[opt_index].var_type == CLVC_BOOLEAN)
-	handle_generated_option (opts, opts_set,
-				 opt_index, NULL, 1, lang_mask,
-				 kind, loc, handlers, dc);
+      if (option->var_type == CLVC_BOOLEAN || option->var_type == CLVC_ENUM)
+	{
+	  int value = 1;
+
+	  if (arg && *arg == '\0' && !option->cl_missing_ok)
+	    arg = NULL;
+
+	  if ((option->flags & CL_JOINED) && arg == NULL)
+	    {
+	      if (option->missing_argument_error)
+		error_at (loc, option->missing_argument_error,
+			  option->opt_text);
+	      else
+		error_at (loc, "missing argument to %qs", option->opt_text);
+	      return;
+	    }
+
+	  /* If the switch takes an integer, convert it.  */
+	  if (arg && option->cl_uinteger)
+	    {
+	      value = integral_argument (arg);
+	      if (value == -1)
+		{
+		  error_at (loc,
+			    "argument to %qs should be a non-negative integer",
+			    option->opt_text);
+		  return;
+		}
+	    }
+
+	  /* If the switch takes an enumerated argument, convert it.  */
+	  if (arg && option->var_type == CLVC_ENUM)
+	    {
+	      const struct cl_enum *e = &cl_enums[option->var_enum];
+
+	      if (enum_arg_to_value (e->values, arg, &value, lang_mask))
+		{
+		  const char *carg = NULL;
+
+		  if (enum_value_to_arg (e->values, &carg, value, lang_mask))
+		    arg = carg;
+		  gcc_assert (carg != NULL);
+		}
+	      else
+		{
+		  unsigned int i;
+		  size_t len;
+		  char *s, *p;
+
+		  if (e->unknown_error)
+		    error_at (loc, e->unknown_error, option->opt_text);
+		  else
+		    error_at (loc, "unrecognized argument in option %qs",
+			      option->opt_text);
+
+		  len = 0;
+		  for (i = 0; e->values[i].arg != NULL; i++)
+		    len += strlen (e->values[i].arg) + 1;
+
+		  s = XALLOCAVEC (char, len);
+		  p = s;
+		  for (i = 0; e->values[i].arg != NULL; i++)
+		    {
+		      if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
+			continue;
+		      size_t arglen = strlen (e->values[i].arg);
+		      memcpy (p, e->values[i].arg, arglen);
+		      p[arglen] = ' ';
+		      p += arglen + 1;
+		    }
+		  p[-1] = 0;
+		  inform (loc, "valid arguments to %qs are: %s",
+			  option->opt_text, s);
+		  return;
+		}
+	    }
+
+	  handle_generated_option (opts, opts_set,
+				   opt_index, arg, value, lang_mask,
+				   kind, loc, handlers, dc);
+	}
     }
 }
--- gcc/opts.c.jj	2015-12-04 17:19:01.901179964 +0100
+++ gcc/opts.c	2015-12-04 18:07:31.902544959 +0100
@@ -2114,7 +2114,7 @@  common_handle_option (struct gcc_options
 
     case OPT_pedantic_errors:
       dc->pedantic_errors = 1;
-      control_warning_option (OPT_Wpedantic, DK_ERROR, value,
+      control_warning_option (OPT_Wpedantic, DK_ERROR, NULL, value,
 			      loc, lang_mask,
 			      handlers, opts, opts_set,
                               dc);
@@ -2437,8 +2437,11 @@  enable_warning_as_error (const char *arg
   else
     {
       const diagnostic_t kind = value ? DK_ERROR : DK_WARNING;
+      const char *arg = NULL;
 
-      control_warning_option (option_index, (int) kind, value,
+      if (cl_options[option_index].flags & CL_JOINED)
+	arg = new_option + cl_options[option_index].opt_len;
+      control_warning_option (option_index, (int) kind, arg, value,
 			      loc, lang_mask,
 			      handlers, opts, opts_set, dc);
     }
--- gcc/c-family/c.opt.jj	2015-12-04 17:19:02.048177994 +0100
+++ gcc/c-family/c.opt	2015-12-04 18:07:31.902544959 +0100
@@ -435,7 +435,7 @@  C ObjC RejectNegative Warning Alias(Werr
 This switch is deprecated; use -Werror=implicit-function-declaration instead.
 
 Wfloat-conversion
-C ObjC C++ ObjC++ Var(warn_float_conversion) LangEnabledBy(C ObjC C++ ObjC++,Wconversion)
+C ObjC C++ ObjC++ Var(warn_float_conversion) Warning LangEnabledBy(C ObjC C++ ObjC++,Wconversion)
 Warn for implicit type conversions that cause loss of floating point precision.
 
 Wfloat-equal
@@ -837,7 +837,7 @@  C ObjC C++ ObjC++ EnabledBy(Wextra)
 ;
 
 Wsign-conversion
-C ObjC C++ ObjC++ Var(warn_sign_conversion) LangEnabledBy(C ObjC,Wconversion)
+C ObjC C++ ObjC++ Var(warn_sign_conversion) Warning LangEnabledBy(C ObjC,Wconversion)
 Warn for implicit type conversions between signed and unsigned integers.
 
 Wsign-promo
--- gcc/c-family/c-pragma.c.jj	2015-12-04 17:19:02.020178369 +0100
+++ gcc/c-family/c-pragma.c	2015-12-04 18:07:31.902544959 +0100
@@ -814,7 +814,11 @@  handle_pragma_diagnostic(cpp_reader *ARG
 
   struct cl_option_handlers handlers;
   set_default_handlers (&handlers);
-  control_warning_option (option_index, (int) kind, kind != DK_IGNORED,
+  const char *arg = NULL;
+  if (cl_options[option_index].flags & CL_JOINED)
+    arg = option_string + 1 + cl_options[option_index].opt_len;
+  control_warning_option (option_index, (int) kind,
+			  arg, kind != DK_IGNORED,
 			  loc, lang_mask, &handlers,
 			  &global_options, &global_options_set,
 			  global_dc);
--- gcc/ada/gcc-interface/trans.c.jj	2015-11-30 13:40:37.408862964 +0100
+++ gcc/ada/gcc-interface/trans.c	2015-12-04 18:07:31.904544931 +0100
@@ -1441,6 +1441,7 @@  Pragma_to_gnu (Node_Id gnat_node)
 
 	/* This is the same implementation as in the C family of compilers.  */
 	const unsigned int lang_mask = CL_Ada | CL_COMMON;
+	const char *arg = NULL;
 	if (Present (gnat_expr))
 	  {
 	    tree gnu_expr = gnat_to_gnu (gnat_expr);
@@ -1464,12 +1465,14 @@  Pragma_to_gnu (Node_Id gnat_node)
 		post_error ("?-W switch not valid for Ada", gnat_node);
 		break;
 	      }
+	    if (cl_options[option_index].flags & CL_JOINED)
+	      arg = option_string + 1 + cl_options[option_index].opt_len;
 	  }
 	else
 	  option_index = 0;
 
 	set_default_handlers (&handlers);
-	control_warning_option (option_index, (int) kind, imply, location,
+	control_warning_option (option_index, (int) kind, arg, imply, location,
 				lang_mask, &handlers, &global_options,
 				&global_options_set, global_dc);
       }
--- gcc/testsuite/c-c++-common/pr68657-1.c.jj	2015-12-04 18:07:31.904544931 +0100
+++ gcc/testsuite/c-c++-common/pr68657-1.c	2015-12-04 18:07:31.904544931 +0100
@@ -0,0 +1,26 @@ 
+/* PR c/68657 */
+/* { dg-options "-Werror=sign-conversion -Werror=float-conversion -Werror=frame-larger-than=65536" } */
+
+void
+f1 (void)
+{
+  unsigned int a = -5;	/* { dg-error "negative integer implicitly converted to unsigned type" } */
+  (void) a;
+}
+
+int
+f2 (void)
+{
+  return 3.1f;	/* { dg-error "conversion to 'int' alters 'float' constant value" } */
+}
+
+int f3 (char *);
+
+int
+f4 (void)
+{
+  char buf[131072];
+  return f3 (buf);
+}		/* { dg-error "the frame size of 1\[0-9]* bytes is larger than 65536 bytes" } */
+
+/* { dg-prune-output "treated as errors" } */
--- gcc/testsuite/c-c++-common/pr68657-2.c.jj	2015-12-04 18:07:31.904544931 +0100
+++ gcc/testsuite/c-c++-common/pr68657-2.c	2015-12-04 18:07:31.904544931 +0100
@@ -0,0 +1,8 @@ 
+/* PR c/68657 */
+/* { dg-do compile } */
+/* { dg-options "-Werror=larger-than=65536" } */
+
+int a[131072];	/* { dg-error "size of 'a' is \[1-9]\[0-9]* bytes" } */
+int b[1024];	/* { dg-bogus "size of 'b' is \[1-9]\[0-9]* bytes" } */
+
+/* { dg-prune-output "treated as errors" } */
--- gcc/testsuite/c-c++-common/pr68657-3.c.jj	2015-12-04 18:07:31.904544931 +0100
+++ gcc/testsuite/c-c++-common/pr68657-3.c	2015-12-04 18:07:31.904544931 +0100
@@ -0,0 +1,13 @@ 
+/* PR c/68657 */
+/* { dg-do compile } */
+
+#pragma GCC diagnostic error "-Wlarger-than=65536"
+int a[131072];	/* { dg-error "size of 'a' is \[1-9]\[0-9]* bytes" } */
+int b[1024];	/* { dg-bogus "size of 'b' is \[1-9]\[0-9]* bytes" } */
+#pragma GCC diagnostic ignored "-Wlarger-than=65536"
+int c[131072];	/* { dg-bogus "size of 'c' is \[1-9]\[0-9]* bytes" } */
+int d[1024];	/* { dg-bogus "size of 'd' is \[1-9]\[0-9]* bytes" } */
+#pragma GCC diagnostic warning "-Wlarger-than=65536"
+int e[131072];	/* { dg-warning "size of 'e' is \[1-9]\[0-9]* bytes" } */
+int f[1024];	/* { dg-bogus "size of 'f' is \[1-9]\[0-9]* bytes" } */
+/* { dg-prune-output "treated as errors" } */