[PATCH] diagnostics: Add new option -fdiagnostics-plain-output

[PATCH] diagnostics: Add new option -fdiagnostics-plain-output

[Lewis Hyatt]

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

Hello-

Attached is the patch I mentioned in another discussion here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551442.html

This adds a new option -fdiagnostics-plain-output that currently means the
same thing as:
    -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
    -fdiagnostics-color=never -fdiagnostics-urls=never

The idea is that over time, if diagnostics output changes to get more bells
and whistles by default (such as the UTF-8 output I suggested in the above
discussion), -fdiagnostics-plain-output will adjust to turn that back off,
so that the testsuite needs only the one option and doesn't need to get
updated every time something new is added. It seems to me that when
diagnostics change, it's otherwise a bit hard to update the testsuite
correctly, especially for compat.exp that is often not run by default. I
think this would also be useful for utilities that want to parse the
diagnostics (but aren't using -fdiagnostics-output-format=json).

BTW, I considered avoiding adding a new switch by having this option take
the form -fdiagnostics-output-format=plain instead, but this seems to have
problematic semantics when multiple related options are specified. Given that
this option needs to be expanded early in the parsing process, so that it
can be compatible with the special handling for -fdiagnostics-color, it
seemed best to just make it a simple option with no negated form.

I hope this may be useful, please let me know if you'd like me to push
it. bootstrap and regtest were done for all languages on x86-64 Linux, all
tests the same before and after, and same for the compat.exp with
alternate compiler GCC 8.2.

-Lewis

[-- Attachment #2: diagnostics_plain_output.txt --]
[-- Type: text/plain, Size: 10784 bytes --]

Subject: [PATCH] diagnostics: Add new option -fdiagnostics-plain-output

Adds the new option -fdiagnostics-plain-output, which is an alias for
several others:

    -fno-diagnostics-show-caret
    -fno-diagnostics-show-line-numbers
    -fdiagnostics-color=never
    -fdiagnostics-urls=never

The idea is that in the future, if the default behavior of diagnostics is
changed to add some fancy feature or other, then the
-fdiagnostics-plain-output option will also be changed accordingly so that
the old behavior is preserved in the presence of this option. This allows
us to use -fdiagnostics-plain-output in in the testsuite, such that the
testsuite (specifically the setting of TEST_ALWAYS_FLAGS in prune.exp)
does not need to be touched whenever diagnostics get a new look. This also
removes the need to add workarounds to compat.exp for every new option
that may be needed in a newer version of the compiler, but is not
supported in older versions.

gcc/ChangeLog:

	* common.opt: Add new option -fdiagnostics-plain-output.
	* doc/invoke.texi: Document it.
	* opts-common.c (decode_cmdline_options_to_array): Implement it.
	(decode_cmdline_option): Add missing const qualifier to argv.

libstdc++-v3/ChangeLog:

	* testsuite/lib/libstdc++.exp: Use the new option
	-fdiagnostics-plain-output.

gcc/testsuite/ChangeLog:

	* lib/prune.exp: Change TEST_ALWAYS_FLAGS to use -fdiagnostics-plain-output.
	* lib/c-compat.exp: Adapt to the prune.exp change.

diff --git a/gcc/common.opt b/gcc/common.opt
index c16d1faff88..20fdcc45fe9 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1378,6 +1378,10 @@ fdiagnostics-path-format=
 Common Joined RejectNegative Var(flag_diagnostics_path_format) Enum(diagnostic_path_format) Init(DPF_INLINE_EVENTS)
 Specify how to print any control-flow path associated with a diagnostic.
 
+fdiagnostics-plain-output
+Driver Common RejectNegative
+Turn off any diagnostics features that complicate the output, such as line numbers, color, and warning URLs.
+
 ftabstop=
 Common Joined RejectNegative UInteger
 -ftabstop=<number>      Distance between tab stops for column reporting.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index dea1e1866a4..70dc1ab73a1 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -280,6 +280,7 @@ Objective-C and Objective-C++ Dialects}.
 @item Diagnostic Message Formatting Options
 @xref{Diagnostic Message Formatting Options,,Options to Control Diagnostic Messages Formatting}.
 @gccoptlist{-fmessage-length=@var{n}  @gol
+-fdiagnostics-plain-output @gol
 -fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]}  @gol
 -fdiagnostics-color=@r{[}auto@r{|}never@r{|}always@r{]}  @gol
 -fdiagnostics-urls=@r{[}auto@r{|}never@r{|}always@r{]}  @gol
@@ -4291,6 +4292,19 @@ Note - this option also affects the display of the @samp{#error} and
 function/type/variable attribute.  It does not however affect the
 @samp{pragma GCC warning} and @samp{pragma GCC error} pragmas.
 
+@item -fdiagnostics-plain-output
+This option requests that diagnostic output look as plain as possible, which
+may be useful when running @command{dejagnu} or other utilities that need to
+parse diagnostics output and prefer that it remain more stable over time.
+@option{-fdiagnostics-plain-output} is currently equivalent to the following
+options:
+@gccoptlist{-fno-diagnostics-show-caret @gol
+-fno-diagnostics-show-line-numbers @gol
+-fdiagnostics-color=never @gol
+-fdiagnostics-urls=never}
+In the future, if GCC changes the default appearance of its diagnostics, the
+corresponding option to disable the new behavior will be added to this list.
+
 @item -fdiagnostics-show-location=once
 @opindex fdiagnostics-show-location
 Only meaningful in line-wrapping mode.  Instructs the diagnostic messages
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index de9510abd64..2119aaefc5c 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -529,7 +529,7 @@ add_misspelling_candidates (auto_vec<char *> *candidates,
    consumed.  */
 
 static unsigned int
-decode_cmdline_option (const char **argv, unsigned int lang_mask,
+decode_cmdline_option (const char *const *argv, unsigned int lang_mask,
 		       struct cl_decoded_option *decoded)
 {
   size_t opt_index;
@@ -944,7 +944,8 @@ decode_cmdline_options_to_array (unsigned int argc, const char **argv,
   struct cl_decoded_option *opt_array;
   unsigned int num_decoded_options;
 
-  opt_array = XNEWVEC (struct cl_decoded_option, argc);
+  int opt_array_len = argc;
+  opt_array = XNEWVEC (struct cl_decoded_option, opt_array_len);
 
   opt_array[0].opt_index = OPT_SPECIAL_program_name;
   opt_array[0].warn_message = NULL;
@@ -981,6 +982,42 @@ decode_cmdline_options_to_array (unsigned int argc, const char **argv,
 	  argv[++i] = replacement;
 	}
 
+      /* Expand -fdiagnostics-plain-output to its constituents.  This needs
+	 to happen here so that prune_options can handle -fdiagnostics-color
+	 specially.  */
+      if (!strcmp (opt, "-fdiagnostics-plain-output"))
+	{
+	  /* If you have changed the default diagnostics output, and this new
+	  output is not appropriately "plain" (e.g., the change needs to be
+	  undone in order for the testsuite to work properly), then please do
+	  the following:
+		 1.  Add the necessary option to undo the new behavior to
+		     the array below.
+		 2.  Update the documentation for -fdiagnostics-plain-output
+		     in invoke.texi.
+	  */
+	  const char *const expanded_args[] = {
+	    "-fno-diagnostics-show-caret",
+	    "-fno-diagnostics-show-line-numbers",
+	    "-fdiagnostics-color=never",
+	    "-fdiagnostics-urls=never",
+	  };
+	  const int num_expanded
+	    = sizeof expanded_args / sizeof (*expanded_args);
+	  opt_array_len += num_expanded - 1;
+	  opt_array = XRESIZEVEC (struct cl_decoded_option,
+				  opt_array, opt_array_len);
+	  for (int j = 0; j < num_expanded;)
+	    {
+	      j += decode_cmdline_option (expanded_args + j, lang_mask,
+					  &opt_array[num_decoded_options]);
+	      num_decoded_options++;
+	    }
+
+	  n = 1;
+	  continue;
+	}
+
       n = decode_cmdline_option (argv + i, lang_mask,
 				 &opt_array[num_decoded_options]);
       num_decoded_options++;
diff --git a/gcc/testsuite/lib/c-compat.exp b/gcc/testsuite/lib/c-compat.exp
index 9493c214aea..5f43c5a6a57 100644
--- a/gcc/testsuite/lib/c-compat.exp
+++ b/gcc/testsuite/lib/c-compat.exp
@@ -36,24 +36,34 @@ load_lib target-libpath.exp
 proc compat-use-alt-compiler { } {
     global GCC_UNDER_TEST ALT_CC_UNDER_TEST
     global compat_same_alt compat_alt_caret compat_alt_color compat_no_line_no
-    global compat_alt_urls
+    global compat_alt_urls compat_alt_plain_output
     global TEST_ALWAYS_FLAGS
 
     # We don't need to do this if the alternate compiler is actually
     # the same as the compiler under test.
     if { $compat_same_alt == 0 } then {
 	set GCC_UNDER_TEST $ALT_CC_UNDER_TEST
+
+	#These flags are no longer added to TEST_ALWAYS_FLAGS by prune.exp
+	#because they are subsumed by -fdiagnostics-plain-output. Add them back
+	#for compatibility testing with older compilers that do not understand
+	#-fdiagnostics-plain-output.
+	set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -fdiagnostics-urls=never $TEST_ALWAYS_FLAGS"
+
 	if { $compat_alt_caret == 0 } then {
-	    regsub -- "-fno-diagnostics-show-caret" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
+	    regsub -all -- "-fno-diagnostics-show-caret" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
 	}
 	if { $compat_alt_color == 0 } then {
-	    regsub -- "-fdiagnostics-color=never" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
+	    regsub -all -- "-fdiagnostics-color=never" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
 	}
 	if { $compat_alt_urls == 0 } then {
-	    regsub -- "-fdiagnostics-urls=never" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
+	    regsub -all -- "-fdiagnostics-urls=never" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
 	}
 	if { $compat_no_line_no == 0 } then {
-	    regsub -- "-fno-diagnostics-show-line-numbers" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
+	    regsub -all -- "-fno-diagnostics-show-line-numbers" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
+	}
+	if { $compat_alt_plain_output == 0 } then {
+	    regsub -all -- "-fdiagnostics-plain-output" $TEST_ALWAYS_FLAGS "" TEST_ALWAYS_FLAGS
 	}
 	restore_gcc_exec_prefix_env_var
     }
@@ -85,12 +95,14 @@ proc compat_setup_dfp { } {
     global compat_alt_caret
     global compat_alt_color
     global compat_alt_urls
+    global compat_alt_plain_output
     global compat_no_line_no
     global TEST_ALWAYS_FLAGS compat_save_TEST_ALWAYS_FLAGS
 
     set compat_alt_caret 0
     set compat_alt_color 0
     set compat_alt_urls 0
+    set compat_alt_plain_output 0
     set compat_no_line_no 0
     set compat_save_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
 
@@ -119,6 +131,10 @@ proc compat_setup_dfp { } {
 		int dummy; } "-fno-diagnostics-show-line-numbers"] != 0 } {
 	    set compat_no_line_no 1
 	}
+	if { [check_no_compiler_messages_nocache compat_alt_has_plain_output object {
+		int dummy; } "-fdiagnostics-plain-output"] != 0 } {
+	    set compat_alt_plain_output 1
+	}
 	
 	compat-use-tst-compiler
     }
diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index 1c776249f1a..df12c75dce7 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -18,10 +18,16 @@
 
 load_lib multiline.exp
 
+#Add options to TEST_ALWAYS_FLAGS so that diagnostics have the expected output
+#format.  Note: You should not normally need to add more options here.  If you
+#have made a change to the default diagnostic output format and are wanting to
+#undo that in the testsuite here, then please update the handling of
+#-fdiagnostics-plain-output in opts-common.c instead.
+
 if ![info exists TEST_ALWAYS_FLAGS] {
     set TEST_ALWAYS_FLAGS ""
 }
-set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never  -fdiagnostics-urls=never $TEST_ALWAYS_FLAGS"
+set TEST_ALWAYS_FLAGS "-fdiagnostics-plain-output $TEST_ALWAYS_FLAGS"
 
 proc prune_gcc_output { text } {
     global srcdir
diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 16963f2edd5..78484f7c9af 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -482,7 +482,7 @@ proc v3_target_compile { source dest type options } {
     global STATIC_LIBCXXFLAGS
     global tool
 
-    lappend options "additional_flags=-fno-diagnostics-show-caret -fdiagnostics-color=never -fdiagnostics-urls=never"
+    lappend options "additional_flags=-fdiagnostics-plain-output"
 
     if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } {
 	lappend options "libs=${gluefile}"

Re: [PATCH] diagnostics: Add new option -fdiagnostics-plain-output

[Richard Sandiford]

Lewis Hyatt <lhyatt@gmail.com> writes:
> Hello-
>
> Attached is the patch I mentioned in another discussion here:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551442.html
>
> This adds a new option -fdiagnostics-plain-output that currently means the
> same thing as:
>     -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
>     -fdiagnostics-color=never -fdiagnostics-urls=never
>
> The idea is that over time, if diagnostics output changes to get more bells
> and whistles by default (such as the UTF-8 output I suggested in the above
> discussion), -fdiagnostics-plain-output will adjust to turn that back off,
> so that the testsuite needs only the one option and doesn't need to get
> updated every time something new is added. It seems to me that when
> diagnostics change, it's otherwise a bit hard to update the testsuite
> correctly, especially for compat.exp that is often not run by default. I
> think this would also be useful for utilities that want to parse the
> diagnostics (but aren't using -fdiagnostics-output-format=json).
>
> BTW, I considered avoiding adding a new switch by having this option take
> the form -fdiagnostics-output-format=plain instead, but this seems to have
> problematic semantics when multiple related options are specified. Given that
> this option needs to be expanded early in the parsing process, so that it
> can be compatible with the special handling for -fdiagnostics-color, it
> seemed best to just make it a simple option with no negated form.
>
> I hope this may be useful, please let me know if you'd like me to push
> it. bootstrap and regtest were done for all languages on x86-64 Linux, all
> tests the same before and after, and same for the compat.exp with
> alternate compiler GCC 8.2.

Thanks for doing this.  LGTM except for a couple of very minor things:

> […]
> @@ -981,6 +982,42 @@ decode_cmdline_options_to_array (unsigned int argc, const char **argv,
>  	  argv[++i] = replacement;
>  	}
>  
> +      /* Expand -fdiagnostics-plain-output to its constituents.  This needs
> +	 to happen here so that prune_options can handle -fdiagnostics-color
> +	 specially.  */
> +      if (!strcmp (opt, "-fdiagnostics-plain-output"))
> +	{
> +	  /* If you have changed the default diagnostics output, and this new
> +	  output is not appropriately "plain" (e.g., the change needs to be
> +	  undone in order for the testsuite to work properly), then please do
> +	  the following:

With GCC formatting, the paragraph should be indented under “If you…”.

> +		 1.  Add the necessary option to undo the new behavior to
> +		     the array below.
> +		 2.  Update the documentation for -fdiagnostics-plain-output
> +		     in invoke.texi.
> +	  */

…and this should be on the previous line (“.  */”).

> +	  const char *const expanded_args[] = {
> +	    "-fno-diagnostics-show-caret",
> +	    "-fno-diagnostics-show-line-numbers",
> +	    "-fdiagnostics-color=never",
> +	    "-fdiagnostics-urls=never",
> +	  };

Hadn't expected it to be this complicated :-)  But I agree with your
reasoning: it looks like this is the correct way given the special
handling of -fdiagnostic-color (and potentially other -fdiagnostic
options in future).

> +	  const int num_expanded
> +	    = sizeof expanded_args / sizeof (*expanded_args);

Simplifies to “ARRAY_SIZE (expanded_args)”.

> +	  opt_array_len += num_expanded - 1;
> +	  opt_array = XRESIZEVEC (struct cl_decoded_option,
> +				  opt_array, opt_array_len);
> +	  for (int j = 0; j < num_expanded;)
> +	    {
> +	      j += decode_cmdline_option (expanded_args + j, lang_mask,
> +					  &opt_array[num_decoded_options]);

Might be worth using the same "for" loop structure as the outer loop,
assigning the number of decoded options to “n”.  Neither's better than
the other, but it makes it clearer that there's nothing special going on.

> +	      num_decoded_options++;
> +	    }
> +
> +	  n = 1;
> +	  continue;
> +	}
> +
>        n = decode_cmdline_option (argv + i, lang_mask,
>  				 &opt_array[num_decoded_options]);
>        num_decoded_options++;
> diff --git a/gcc/testsuite/lib/c-compat.exp b/gcc/testsuite/lib/c-compat.exp
> index 9493c214aea..5f43c5a6a57 100644
> --- a/gcc/testsuite/lib/c-compat.exp
> +++ b/gcc/testsuite/lib/c-compat.exp
> @@ -36,24 +36,34 @@ load_lib target-libpath.exp
>  proc compat-use-alt-compiler { } {
>      global GCC_UNDER_TEST ALT_CC_UNDER_TEST
>      global compat_same_alt compat_alt_caret compat_alt_color compat_no_line_no
> -    global compat_alt_urls
> +    global compat_alt_urls compat_alt_plain_output
>      global TEST_ALWAYS_FLAGS
>  
>      # We don't need to do this if the alternate compiler is actually
>      # the same as the compiler under test.
>      if { $compat_same_alt == 0 } then {
>  	set GCC_UNDER_TEST $ALT_CC_UNDER_TEST
> +
> +	#These flags are no longer added to TEST_ALWAYS_FLAGS by prune.exp
> +	#because they are subsumed by -fdiagnostics-plain-output. Add them back
> +	#for compatibility testing with older compilers that do not understand
> +	#-fdiagnostics-plain-output.

I think the convention (in GCC) is to have a space after the “#”.
Same for the other comments.

> +	set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -fdiagnostics-urls=never $TEST_ALWAYS_FLAGS"

I was initially surprised that the existing code didn't reset
TEST_ALWAYS_FLAGS to compat_save_TEST_ALWAYS_FLAGS before altering it,
but I guess there's no need if all it's doing is removing flags.
(So I agree what the patch is doing is correct as-is.)

OK with those changes in 24 hrs if noone objects or asks for a delay.

Thanks,
Richard

Re: [PATCH] diagnostics: Add new option -fdiagnostics-plain-output

[Lewis Hyatt]

On Wed, Aug 12, 2020 at 12:54 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Lewis Hyatt <lhyatt@gmail.com> writes:
> > Hello-
> >
> > Attached is the patch I mentioned in another discussion here:
> > https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551442.html
> >
> > This adds a new option -fdiagnostics-plain-output that currently means the
> > same thing as:
> >     -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
> >     -fdiagnostics-color=never -fdiagnostics-urls=never
> >
> > The idea is that over time, if diagnostics output changes to get more bells
> > and whistles by default (such as the UTF-8 output I suggested in the above
> > discussion), -fdiagnostics-plain-output will adjust to turn that back off,
> > so that the testsuite needs only the one option and doesn't need to get
> > updated every time something new is added. It seems to me that when
> > diagnostics change, it's otherwise a bit hard to update the testsuite
> > correctly, especially for compat.exp that is often not run by default. I
> > think this would also be useful for utilities that want to parse the
> > diagnostics (but aren't using -fdiagnostics-output-format=json).
> >
> > BTW, I considered avoiding adding a new switch by having this option take
> > the form -fdiagnostics-output-format=plain instead, but this seems to have
> > problematic semantics when multiple related options are specified. Given that
> > this option needs to be expanded early in the parsing process, so that it
> > can be compatible with the special handling for -fdiagnostics-color, it
> > seemed best to just make it a simple option with no negated form.
> >
> > I hope this may be useful, please let me know if you'd like me to push
> > it. bootstrap and regtest were done for all languages on x86-64 Linux, all
> > tests the same before and after, and same for the compat.exp with
> > alternate compiler GCC 8.2.
>
> Thanks for doing this.  LGTM except for a couple of very minor things:
>
> > […]
> > @@ -981,6 +982,42 @@ decode_cmdline_options_to_array (unsigned int argc, const char **argv,
> >         argv[++i] = replacement;
> >       }
> >
> > +      /* Expand -fdiagnostics-plain-output to its constituents.  This needs
> > +      to happen here so that prune_options can handle -fdiagnostics-color
> > +      specially.  */
> > +      if (!strcmp (opt, "-fdiagnostics-plain-output"))
> > +     {
> > +       /* If you have changed the default diagnostics output, and this new
> > +       output is not appropriately "plain" (e.g., the change needs to be
> > +       undone in order for the testsuite to work properly), then please do
> > +       the following:
>
> With GCC formatting, the paragraph should be indented under “If you…”.
>
> > +              1.  Add the necessary option to undo the new behavior to
> > +                  the array below.
> > +              2.  Update the documentation for -fdiagnostics-plain-output
> > +                  in invoke.texi.
> > +       */
>
> …and this should be on the previous line (“.  */”).
>
> > +       const char *const expanded_args[] = {
> > +         "-fno-diagnostics-show-caret",
> > +         "-fno-diagnostics-show-line-numbers",
> > +         "-fdiagnostics-color=never",
> > +         "-fdiagnostics-urls=never",
> > +       };
>
> Hadn't expected it to be this complicated :-)  But I agree with your
> reasoning: it looks like this is the correct way given the special
> handling of -fdiagnostic-color (and potentially other -fdiagnostic
> options in future).
>
> > +       const int num_expanded
> > +         = sizeof expanded_args / sizeof (*expanded_args);
>
> Simplifies to “ARRAY_SIZE (expanded_args)”.
>
> > +       opt_array_len += num_expanded - 1;
> > +       opt_array = XRESIZEVEC (struct cl_decoded_option,
> > +                               opt_array, opt_array_len);
> > +       for (int j = 0; j < num_expanded;)
> > +         {
> > +           j += decode_cmdline_option (expanded_args + j, lang_mask,
> > +                                       &opt_array[num_decoded_options]);
>
> Might be worth using the same "for" loop structure as the outer loop,
> assigning the number of decoded options to “n”.  Neither's better than
> the other, but it makes it clearer that there's nothing special going on.
>
> > +           num_decoded_options++;
> > +         }
> > +
> > +       n = 1;
> > +       continue;
> > +     }
> > +
> >        n = decode_cmdline_option (argv + i, lang_mask,
> >                                &opt_array[num_decoded_options]);
> >        num_decoded_options++;
> > diff --git a/gcc/testsuite/lib/c-compat.exp b/gcc/testsuite/lib/c-compat.exp
> > index 9493c214aea..5f43c5a6a57 100644
> > --- a/gcc/testsuite/lib/c-compat.exp
> > +++ b/gcc/testsuite/lib/c-compat.exp
> > @@ -36,24 +36,34 @@ load_lib target-libpath.exp
> >  proc compat-use-alt-compiler { } {
> >      global GCC_UNDER_TEST ALT_CC_UNDER_TEST
> >      global compat_same_alt compat_alt_caret compat_alt_color compat_no_line_no
> > -    global compat_alt_urls
> > +    global compat_alt_urls compat_alt_plain_output
> >      global TEST_ALWAYS_FLAGS
> >
> >      # We don't need to do this if the alternate compiler is actually
> >      # the same as the compiler under test.
> >      if { $compat_same_alt == 0 } then {
> >       set GCC_UNDER_TEST $ALT_CC_UNDER_TEST
> > +
> > +     #These flags are no longer added to TEST_ALWAYS_FLAGS by prune.exp
> > +     #because they are subsumed by -fdiagnostics-plain-output. Add them back
> > +     #for compatibility testing with older compilers that do not understand
> > +     #-fdiagnostics-plain-output.
>
> I think the convention (in GCC) is to have a space after the “#”.
> Same for the other comments.
>
> > +     set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -fdiagnostics-urls=never $TEST_ALWAYS_FLAGS"
>
> I was initially surprised that the existing code didn't reset
> TEST_ALWAYS_FLAGS to compat_save_TEST_ALWAYS_FLAGS before altering it,
> but I guess there's no need if all it's doing is removing flags.
> (So I agree what the patch is doing is correct as-is.)
>
> OK with those changes in 24 hrs if noone objects or asks for a delay.
>
> Thanks,
> Richard

Thanks for the review, and sorry about the formatting glitches. I
pushed it just now with your fixes.

-Lewis

Re: [PATCH] diagnostics: Add new option -fdiagnostics-plain-output

[David Malcolm]

On Fri, 2020-08-14 at 10:01 -0400, Lewis Hyatt wrote:
> On Wed, Aug 12, 2020 at 12:54 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:

[...]

> > OK with those changes in 24 hrs if noone objects or asks for a
> > delay.
> > 
> > Thanks,
> > Richard
> 
> Thanks for the review, and sorry about the formatting glitches. I
> pushed it just now with your fixes.

Thanks! (both to you and to Richard)