Subject: Re: bug#25987: 25.2; support gcc fixit notes

From: David Malcolm <[email-masked]>
Date: Mon, 12 Oct 2020 18:27:29 -0400

On Tue, 2020-10-06 at 21:37 +0300, Eli Zaretskii wrote:
> > From: David Malcolm <[email-masked]>
> > Cc: [email-masked]
> > Date: Tue, 06 Oct 2020 14:17:55 -0400
> > 
> > In GCC 11 we've revamped the column number handling in how we emit
> > diagnostics; see:
> >   
> > https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555632.html
> > 
> > GCC 11 diagnostics now (by default) should use actual column
> > numbers,
> > rather than byte counts.
> 
> That's good news, thanks.
> 
> > We haven't changed -fdiagnostics-parseable-fixits; it still emits
> > its
> > range information in terms of byte offsets (and e.g. Eclipse
> > already
> > consumes that option); this is bug-for-bug compatible with clang, I
> > believe (which had that option first).
> 
> So fixit hints will still count bytes?  Or will GCC 11 emit such
> hints even without -fdiagnostics-parseable-fixits on the command
> line?
> 
> > Note that characters != columns, or, at least, we have to be
> > careful
> > about what we mean.  Consider the case of 🙂 aka SLIGHTLY SMILING
> > FACE
> > (U+1F642) which is a single unicode code point, but occupies two
> > columns, with its UTF-8 encoding requiring four bytes.
> > 
> > When I type it into an Emacs buffer, and look at the column number
> > I
> > see that Emacs appears to treat the character as occupying two
> > columns.
> >  Is that the kind of column numbering you would want for machine-
> > readable fix-it hints?
> 
> Yes.  Emacs computes the width of each character by using UCD, the
> Unicode Character Database (specifically, the EastAsianWidth.txt file
> that is part of the UCD).  If GCC gets its column counts from a
> similar DB, then it will match what Emacs does.
> 
> > Similarly, the column numbering emitted by GCC 11 diagnostics is
> > affected by tab characters as tab stops, which seems to reflect
> > Emacs
> > behavior as well.
> 
> Yes.
> 
> > I can add an additional option for fix-it hints that's similar to
> > -fdiagnostics-parseable-fixits, but with a different output format
> > (or
> > to add an argument to that option, perhaps).
> 
> If an option is needed for getting the hints, then a special option
> which reports columns in hints will be appreciated, as it will make
> the Emacs support for processing those hints 100% accurate and devoid
> of encoding guesswork.
> 
> > Before I do that, I wanted to check that it would be consumable by
> > Emacs.  What works for you?  Would it help to send the proposed GCC
> > patch to this bug address (or to the emacs-devel list?).
> 
> I don't know how many people here build their own GCC, and thus could
> try the patch, but if sending the patch is not too much trouble,
> perhaps posting it on emacs-devel would be a good idea.  If you do
> that, please cite this bug report, so that people who try that could
> respond here with their experience.
> 
> > Alternatively, we already have a JSON output option (-fdiagnostics-
> > format=json); perhaps something like that could be used?
> 
> Emacs can parse JSON.  What are the pros and cons of the JSON
> alternative wrt to the text alternative?

The existing "-fdiagnostics-format=json" GCC option replaces the
existing diagnostic output with a big blob of JSON to stderr, all on
one line.  Although I implemented it, I now feel it to be rather half-
baked.

I like how -fdiagnostics-parseable-fixits adds extra lines of output,
with a prefix that's ought to be easy to detect.

BTW, does Emacs set anything in the environment that GCC could detect?

> > Feature freeze for GCC 11 is about a month away; I'd love for Emacs
> > to
> > be able to consume GCC fix-it hints (and have GCC and Emacs fix my
> > typos for me)
> 
> Agreed; let's try to make that happen.

I put together a test file showing various features to try to ensure
that GCC and Emacs interoperate on this.  I'm attaching it.

This can also be seen on Compiler Explorer at:
  https://godbolt.org/z/zazejq
which adds the existing
  -fdiagnostics-parseable-fixits -fdiagnostics-generate-patch
options.

Ideas for other test cases are most welcome.

Does Emacs have an automated test suite that could test this feature?
A long-postponed goal for me for GCC's testsuite is to ensure that fix-
it hints apply and actually fix the problem (clang's testsuite has
tests that verify this for clang's fix-it hints).

Another complication to consider: the locations in a fix-it hint refer
to the original source file, before any changes are made.  If the user
interface supports the user e.g. clicking on fix-it hints and
selectively apply them one by one, then after each fix-it hint is
applied, all locations after that point potentially need to be "fixed
up" somehow, to reflect the changes to the buffer.  GCC's own code
implements this in gcc/edit-context.{c|h}, for implementing -
fdiagnostics-generate-patch, which applies all fix-its "atomically" -
but the use-case I thinking of involves clicking on fix-it hints one-
by-one in the compilation buffer, perhaps replacing the fix-it output
with an "Apply fix" button.

(to cover this case I made sure the demo file contained examples of a
fix-it hint that adds a line, and one that has multiple fix-it hints on
the same line).

Thoughts?

Dave