On 11/14/2018 02:12 PM, David Malcolm wrote:
On Mon, 2018-11-12 at 13:37 -0700, Martin Sebor wrote:
On 11/11/2018 07:43 PM, David Malcolm wrote:
We often emit more than one diagnostic at the same source location.
For example, the C++ frontend can emit many diagnostics at
the same source location when suggesting overload candidates.
For example:
../../src/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C: In
function 'int test_3(s, t)':
../../src/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C:38:18:
error: no match for 'operator&&' (operand types are 's' and 't')
38 | return param_s && param_t;
| ~~~~~~~~^~~~~~~~~~
../../src/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C:38:18:
note: candidate: 'operator&&(bool, bool)' <built-in>
../../src/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C:38:18:
note: no known conversion for argument 2 from 't' to 'bool'
This is overly verbose. Note how the same location has been
printed
three times, obscuring the pertinent messages.
This patch add a new "elide" value to -fdiagnostics-show-location=
and makes it the default (previously it was "once"). With elision
the above is printed as:
../../src/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C: In
function 'int test_3(s, t)':
../../src/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C:38:18:
error: no match for 'operator&&' (operand types are 's' and 't')
38 | return param_s && param_t;
| ~~~~~~~~^~~~~~~~~~
= note: candidate: 'operator&&(bool, bool)' <built-in>
= note: no known conversion for argument 2 from 't' to
'bool'
where the followup notes are printed with a '=' lined up with
the source code margin.
Thoughts?
I agree the long pathname in the notes is at first glance redundant
but I'm not sure about using '=' as a shorthand for it. I have
written many scripts to parse GCC output to extract all diagnostics
(including notes) and publish those on a Web page somewhere, as I'm
sure must have others. All those scripts would stop working with
this change and require changes to the build system to work again.
Making those changes can be a substantial undertaking in some
organizations.
Have you considered printing just the file name instead? Or any
other alternatives?
"-fdiagnostics-show-location=once" will restore the old behavior.
Alternatively, if you want to parse GCC output, I'm adding a JSON
output format; see:
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01038.html
(I'm testing an updated version of that patch)
I understand the change can be disabled by an option. I also
like making the repetitive pathnames shorter, but the concern
I have is that the change to use the '=' notation by default,
besides being rather unusual and not necessarily intuitive, will
break scripts that search for the pattern:
"[^:]*:[1-9][0-9]*:[1-9][0-9]*: (error|note|warning): "
and adding a new option to a large build system can be quite
difficult. I suspect it would also make the notes difficult
or even impossible to associate with the corresponding errors
or warnings in parallel builds (where they may be interleaved
with diagnostics from different compilations).
I think it's possible to make the output shorter/less repetitive
without these side-effects by keeping the current format and
instead abbreviating the pathname, e.g. by printing just the file
name (or ".../filename.c:" with the ellipsis standing in for the
long pathname, though the ellipsis would require adjusting
the scripts that sort diagnostics by the pathname).
Martin
PS As an aside, if we wanted to shorten all pathnames this same
idea could be extended to errors and warnings as well by printing
the pathname in the first one and just the filename in subsequent
ones in the same file.
