gcc/diagnostic.c: make -Werror message more helpful

  Hello.


I propose to make that message more verbose.  It sure would have helped
me once.  You don't always have a Web search available :)


Andrea Monaco
  

On 12/12/21 3:13 AM, Andrea Monaco via Gcc-patches wrote:
> 
> Hello.
> 
> 
> I propose to make that message more verbose.  It sure would have helped
> me once.  You don't always have a Web search available :)

Warnings turned into errors have the [-Werror=...] tag at the end
so I'm not sure I see when reiterating -Werror at the end of output
would be helpful.  Can you explain the circumstances when it would
have helped you?

For what it's worth, a change here that I think might be more useful
is printing the number of diagnostics of each kind (e.g., 2 warnings
and 5 errors found).

> Andrea Monaco
> 
> 
> 
> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> index 4ded1760705..8b67662390e 100644
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -156,7 +156,7 @@ default_diagnostic_final_cb (diagnostic_context *context)
>         /* -Werror was given.  */
>         if (context->warning_as_error_requested)
>          pp_verbatim (context->printer,
> -                    _("%s: all warnings being treated as errors"),
> +                    _("%s: all warnings being treated as errors (-Werror; disable with -Wno-error)"),

If this change should move forward, -Werror needs to be quoted
(e.g., passed as an argument to %qs or surrounded in a pair of
%< and %> directives).  The "disable with -Wno-error" part
is superfluous and would not be entirely accurate for warnings
promoted to errors by #pragma GCC diagnostic (those cannot be
demoted back to warnings by -Wno-error).

Martin

>                       progname);
>         /* At least one -Werror= was given.  */
>         else
>
  

On Mon, Dec 13, 2021 at 1:17 PM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On 12/12/21 3:13 AM, Andrea Monaco via Gcc-patches wrote:
> >
> > Hello.
> >
> >
> > I propose to make that message more verbose.  It sure would have helped
> > me once.  You don't always have a Web search available :)
>
> Warnings turned into errors have the [-Werror=...] tag at the end
> so I'm not sure I see when reiterating -Werror at the end of output
> would be helpful.  Can you explain the circumstances when it would
> have helped you?
>
> For what it's worth, a change here that I think might be more useful
> is printing the number of diagnostics of each kind (e.g., 2 warnings
> and 5 errors found).
>

I swear we already had a bug open for this suggestion, but after much
searching I can't seem to find it anymore, so if anyone has any ideas
of what keywords I forgot to try, feel free to send them...

> > Andrea Monaco
> >
> >
> >
> > diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> > index 4ded1760705..8b67662390e 100644
> > --- a/gcc/diagnostic.c
> > +++ b/gcc/diagnostic.c
> > @@ -156,7 +156,7 @@ default_diagnostic_final_cb (diagnostic_context *context)
> >         /* -Werror was given.  */
> >         if (context->warning_as_error_requested)
> >          pp_verbatim (context->printer,
> > -                    _("%s: all warnings being treated as errors"),
> > +                    _("%s: all warnings being treated as errors (-Werror; disable with -Wno-error)"),
>
> If this change should move forward, -Werror needs to be quoted
> (e.g., passed as an argument to %qs or surrounded in a pair of
> %< and %> directives).  The "disable with -Wno-error" part
> is superfluous and would not be entirely accurate for warnings
> promoted to errors by #pragma GCC diagnostic (those cannot be
> demoted back to warnings by -Wno-error).
>
> Martin
>
> >                       progname);
> >         /* At least one -Werror= was given.  */
> >         else
> >
>
  

On Tue, Dec 14, 2021 at 1:33 PM Eric Gallager <egall@gwmail.gwu.edu> wrote:
>
> On Mon, Dec 13, 2021 at 1:17 PM Martin Sebor via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On 12/12/21 3:13 AM, Andrea Monaco via Gcc-patches wrote:
> > >
> > > Hello.
> > >
> > >
> > > I propose to make that message more verbose.  It sure would have helped
> > > me once.  You don't always have a Web search available :)
> >
> > Warnings turned into errors have the [-Werror=...] tag at the end
> > so I'm not sure I see when reiterating -Werror at the end of output
> > would be helpful.  Can you explain the circumstances when it would
> > have helped you?
> >
> > For what it's worth, a change here that I think might be more useful
> > is printing the number of diagnostics of each kind (e.g., 2 warnings
> > and 5 errors found).
> >
>
> I swear we already had a bug open for this suggestion, but after much
> searching I can't seem to find it anymore, so if anyone has any ideas
> of what keywords I forgot to try, feel free to send them...

Never mind, I managed to find it after all: it's bug 26061:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=26061

> > > Andrea Monaco
> > >
> > >
> > >
> > > diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
> > > index 4ded1760705..8b67662390e 100644
> > > --- a/gcc/diagnostic.c
> > > +++ b/gcc/diagnostic.c
> > > @@ -156,7 +156,7 @@ default_diagnostic_final_cb (diagnostic_context *context)
> > >         /* -Werror was given.  */
> > >         if (context->warning_as_error_requested)
> > >          pp_verbatim (context->printer,
> > > -                    _("%s: all warnings being treated as errors"),
> > > +                    _("%s: all warnings being treated as errors (-Werror; disable with -Wno-error)"),
> >
> > If this change should move forward, -Werror needs to be quoted
> > (e.g., passed as an argument to %qs or surrounded in a pair of
> > %< and %> directives).  The "disable with -Wno-error" part
> > is superfluous and would not be entirely accurate for warnings
> > promoted to errors by #pragma GCC diagnostic (those cannot be
> > demoted back to warnings by -Wno-error).
> >
> > Martin
> >
> > >                       progname);
> > >         /* At least one -Werror= was given.  */
> > >         else
> > >
> >
  

Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On 12/12/21 3:13 AM, Andrea Monaco via Gcc-patches wrote:
>> 
>> Hello.
>> 
>> 
>> I propose to make that message more verbose.  It sure would have helped
>> me once.  You don't always have a Web search available :)
>
> Warnings turned into errors have the [-Werror=...] tag at the end
> so I'm not sure I see when reiterating -Werror at the end of output
> would be helpful.  Can you explain the circumstances when it would
> have helped you?

Printing -Werror=foo might give the impression that that was the option
that was actually passed.  The message is the same if -Werror=foo was
passed and if -Werror was passed (or something in between, for groups
of options).

The final “all warnings being treated as errors” line is only printed
for -Werror, not -Werror=foo, so I think including -Werror there makes
sense, and is more consistent with the individual error messages.

So personally I think we should take the patch.

Thanks,
Richard

> For what it's worth, a change here that I think might be more useful
> is printing the number of diagnostics of each kind (e.g., 2 warnings
> and 5 errors found).
>
>> Andrea Monaco
>> 
>> 
>> 
>> diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
>> index 4ded1760705..8b67662390e 100644
>> --- a/gcc/diagnostic.c
>> +++ b/gcc/diagnostic.c
>> @@ -156,7 +156,7 @@ default_diagnostic_final_cb (diagnostic_context *context)
>>         /* -Werror was given.  */
>>         if (context->warning_as_error_requested)
>>          pp_verbatim (context->printer,
>> -                    _("%s: all warnings being treated as errors"),
>> +                    _("%s: all warnings being treated as errors (-Werror; disable with -Wno-error)"),
>
> If this change should move forward, -Werror needs to be quoted
> (e.g., passed as an argument to %qs or surrounded in a pair of
> %< and %> directives).  The "disable with -Wno-error" part
> is superfluous and would not be entirely accurate for warnings
> promoted to errors by #pragma GCC diagnostic (those cannot be
> demoted back to warnings by -Wno-error).
>
> Martin
>
>>                       progname);
>>         /* At least one -Werror= was given.  */
>>         else
>>