On 11/15/18, Martin Sebor <mse...@gmail.com> wrote: > 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 like the -fdiagnostics-show-location=elide option but I also agree about the '=' looking weird. However, I don't think the '=' is that big a problem that needs to provoke huge rethinking, just some minor bikeshedding about which symbol to use instead. How about '+'? >>> 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. >
