On Fri, Jul 24, 2020 at 02:49:36PM +0100, Richard Sandiford wrote:
> Lewis Hyatt via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On Thu, Jul 23, 2020 at 05:47:28PM -0400, David Malcolm wrote:
> >> On Thu, 2020-07-23 at 12:28 -0400, Lewis Hyatt via Gcc-patches wrote:
> >> > Hello-
> >> > 
> >> > The attached patch is complete including docs, but I tagged as RFC
> >> > because I am not sure if anyone will like it, or if the general
> >> > reaction may
> >> > be closer to recoiling in horror :). Would appreciate your thoughts,
> >> > please...
> >> 
> >> Thanks for working on this.  I'm interested in other people's thoughts
> >> on this.  Various comments inline throughout below.
> 
> +1 in favour FWIW.
>

Thanks for the feedback!

> > […]
> > By the way, I was wondering separately what you think about adding an option
> > like -fplain-diagnostics or something, which would achieve basically the 
> > same
> > thing you get in the testsuite right now (-fno-diagnostics-show-caret
> > -fno-diagnostics-show-line-numbers -fdiagnostics-color=never
> > -fdiagnostics-urls=never) but would change as necessary whenever diagnostics
> > evolve. It seems rather involved currently to add a new option like
> > -fdiagnostics-unicode-drawing but keep the testsuite working, in addition to
> > adding to prune.exp and to the libstdc++.exp, you also need to update the
> > compat.exp so that it can figure out to pass the option only to sufficiently
> > new compilers. With -fplain-diagnostics, this could just be part of the code
> > change and the testsuite could stay the same; this may also make it easier 
> > on
> > IDE type utilities since they could rely on a more stable format for the
> > diagnostics, assuming they don't already use JSON format.
> 
> Also agree that this would be a nice feature to have.  I guess it would
> act as an alias for all the -fno-* options at the point that it occurs
> on the command line, so that it would be possible to use:
> 
>   -fplain-diagnostics -fthe-diagnostic-feature-i-like
>

Yes, that's what I was thinking. Currently the option -fdiagnostics-color
requires special handling because it applies even before it appears in the
command line (so that, say, a wrong option which appears earlier can still get a
colorized diagnostic). I was thinking the way to go would be to expand
-fplain-diagnostics into its constituents around the same place that this
special handling is done. I'll go ahead and submit a patch separately for this
sometime soon, in case it is found useful.

> > […]
> >> * maybe have a different character for separating the line numbers as
> >> opposed to those for labels and for showing interprocedural paths.
> >>
> >
> > Something like that would be easy to add, sure, perhaps a double vertical 
> > line
> > instead:
> >
> > diagnostic-ranges.c:196:28: warning: field width specifier ‘*’ expects 
> > argument of type ‘int’, but argument 3 has type ‘long int’ [-Wformat=]
> >   196 ║   __builtin_sprintf (d, " %*ld ", foo + bar, foo);
> >       ║                           ═∧══    ════╤════
> >       ║                            │          │
> >       ║                            int        long int
> 
> Guess it's just personal taste, but that seems a bit too busy to me.
> Most diagnostics don't have interprocedural paths, and on its own,
> there doesn't seem to be a specific reason to have a double line
> on the left.

I tend to agree with this assessment as well.

-Lewis

Reply via email to