fdiagnostics-color=never does not disable color for some diagnostics
diff mbox

Message ID CAESRpQDMkfkgu-4kcZhCM_qSzYNiOXZYaub1UySf7JOMynYSqA@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Sept. 22, 2015, 8:23 p.m. UTC
Actually, I was trying to reject non-warning options as argument to
-Werror=. However, the new test fails because -fdiagnostics-color=never is
always placed by the driver after the warning options when calling the compiler
proper. This patch prunes all -fdiagnostics-color from the command-line but the
last one, which is moved to the first position.

Boot&tested on x86_64-linux-gnu

OK?

gcc/ChangeLog:

2015-09-22  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR driver/67640
    * opts-common.c (prune_options): Discard all -fdiagnostics-color
    but the last one, which is moved to the front to be processed
    first.
    * opts.c (enable_warning_as_error): Reject options that do not
    control warnings.


gcc/testsuite/ChangeLog:

2015-09-22  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR driver/67640
    * gcc.dg/Werror-13.c: New test.

Comments

Jason Merrill Sept. 24, 2015, 1:06 p.m. UTC | #1
On 09/22/2015 04:23 PM, Manuel López-Ibáñez wrote:
> +    error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
> +  else if (!(cl_options[option_index].flags & CL_WARNING))
> +    error_at (loc, "-Werror=%s: -%s is not an option that controls warnings",

Won't these incorrectly start with "-Werror=Wsomething:" rather than the 
"-Werror=something" that the user wrote?

Jason
Manuel López-Ibáñez Sept. 24, 2015, 3:32 p.m. UTC | #2
On 24 September 2015 at 15:06, Jason Merrill <jason@redhat.com> wrote:
> On 09/22/2015 04:23 PM, Manuel López-Ibáñez wrote:
>>
>> +    error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
>> +  else if (!(cl_options[option_index].flags & CL_WARNING))
>> +    error_at (loc, "-Werror=%s: -%s is not an option that controls
>> warnings",
>
>
> Won't these incorrectly start with "-Werror=Wsomething:" rather than the
> "-Werror=something" that the user wrote?

They follow the pattern of the code they replace:

-    {
-      error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
-    }

where 'arg' is what the user wrote after '=', and new_option is:

   new_option[0] = 'W';
   strcpy (new_option + 1, arg);

Or am I misunderstanding you?

Cheers,

Manuel.
Jason Merrill Sept. 24, 2015, 3:59 p.m. UTC | #3
On 09/24/2015 11:32 AM, Manuel López-Ibáñez wrote:
> On 24 September 2015 at 15:06, Jason Merrill <jason@redhat.com> wrote:
>> On 09/22/2015 04:23 PM, Manuel López-Ibáñez wrote:
>>>
>>> +    error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
>>> +  else if (!(cl_options[option_index].flags & CL_WARNING))
>>> +    error_at (loc, "-Werror=%s: -%s is not an option that controls
>>> warnings",
>>
>>
>> Won't these incorrectly start with "-Werror=Wsomething:" rather than the
>> "-Werror=something" that the user wrote?
>
> They follow the pattern of the code they replace:
>
> -    {
> -      error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
> -    }
>
> where 'arg' is what the user wrote after '=', and new_option is:
>
>     new_option[0] = 'W';
>     strcpy (new_option + 1, arg);
>
> Or am I misunderstanding you?

No, you're right, I was misreading.  The patch is OK.

Jason

Patch
diff mbox

Index: gcc/opts-common.c
===================================================================
--- gcc/opts-common.c	(revision 228011)
+++ gcc/opts-common.c	(working copy)
@@ -823,10 +823,11 @@  prune_options (struct cl_decoded_option 
   unsigned int new_decoded_options_count;
   struct cl_decoded_option *new_decoded_options
     = XNEWVEC (struct cl_decoded_option, old_decoded_options_count);
   unsigned int i;
   const struct cl_option *option;
+  unsigned int fdiagnostics_color_idx = 0;
 
   /* Remove arguments which are negated by others after them.  */
   new_decoded_options_count = 0;
   for (i = 0; i < old_decoded_options_count; i++)
     {
@@ -842,10 +843,15 @@  prune_options (struct cl_decoded_option 
 	case OPT_SPECIAL_ignore:
 	case OPT_SPECIAL_program_name:
 	case OPT_SPECIAL_input_file:
 	  goto keep;
 
+	/* Do not save OPT_fdiagnostics_color_, just remember the last one.  */
+	case OPT_fdiagnostics_color_:
+	  fdiagnostics_color_idx = i;
+	  continue;
+
 	default:
 	  gcc_assert (opt_idx < cl_options_count);
 	  option = &cl_options[opt_idx];
 	  if (option->neg_index < 0)
 	    goto keep;
@@ -877,10 +883,21 @@  keep:
 	    }
 	  break;
 	}
     }
 
+  if (fdiagnostics_color_idx > 1)
+    {
+      /* We put the last -fdiagnostics-color= at the first position
+	 after argv[0] so it can take effect immediately.  */
+      memmove (new_decoded_options + 2, new_decoded_options + 1,
+	       sizeof (struct cl_decoded_option) 
+	       * (new_decoded_options_count - 1));
+      new_decoded_options[1] = old_decoded_options[fdiagnostics_color_idx];
+      new_decoded_options_count++;
+    }
+
   free (old_decoded_options);
   new_decoded_options = XRESIZEVEC (struct cl_decoded_option,
 				    new_decoded_options,
 				    new_decoded_options_count);
   *decoded_options = new_decoded_options;
Index: gcc/testsuite/gcc.dg/Werror-13.c
===================================================================
--- gcc/testsuite/gcc.dg/Werror-13.c	(revision 0)
+++ gcc/testsuite/gcc.dg/Werror-13.c	(revision 0)
@@ -0,0 +1,8 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Werror=error -Werror=p, -Werror=l, -Werror=fatal-errors" } */
+/* { dg-error "-Wp, is not an option that controls warnings" "" { target *-*-* } 0 } */
+/* { dg-error "-Wl, is not an option that controls warnings" "" { target *-*-* } 0 } */
+/* { dg-error "-Werror is not an option that controls warnings" "" { target *-*-* } 0 } */
+/* { dg-error "-Wfatal-errors is not an option that controls warnings" "" { target *-*-* } 0 } */
+
+int i;
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 228011)
+++ gcc/opts.c	(working copy)
@@ -2357,13 +2357,14 @@  enable_warning_as_error (const char *arg
   new_option = XNEWVEC (char, strlen (arg) + 2);
   new_option[0] = 'W';
   strcpy (new_option + 1, arg);
   option_index = find_opt (new_option, lang_mask);
   if (option_index == OPT_SPECIAL_unknown)
-    {
-      error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
-    }
+    error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
+  else if (!(cl_options[option_index].flags & CL_WARNING))
+    error_at (loc, "-Werror=%s: -%s is not an option that controls warnings",
+	      arg, new_option);
   else
     {
       const diagnostic_t kind = value ? DK_ERROR : DK_WARNING;
 
       control_warning_option (option_index, (int) kind, value,