On Tue, 2016-08-23 at 21:57 -0400, Eric Gallager wrote:
> On 8/23/16, David Malcolm <[email protected]> wrote:
> > Currently our diagnostics subsystem can emit fix-it hints
> > suggesting edits the user can make to their code to fix
> > warnings/errors,
> > but currently the user must make these edits manually [1].
> >
> > The following patch kit adds two command-line options with which
> > we can make the changes on behalf of the user:
> >
> > (A) -fdiagnostics-generate-patch emits a diff to stderr, which
> > could potentially be applied using "patch"; it also gives another
> > way to visualize the fix-its.
> >
> > (B) -fdiagnostics-apply-fixits, which writes back the changes
> > to the input files.
> >
> > Patches 1 and 2 are enabling work.
> >
> > Patch 3 is the heart of the kit: a new class edit_context,
> > representing a set of changes to be applied to source file(s).
> > The changes are atomic: all are applied or none [2], and locations
> > are
> > specified relative to the initial inputs (and there's some fiddly
> > logic for fixing up locations so that the order in which edits are
> > applied doesn't matter).
> >
> > Patch 4 uses the edit_context class to add the new options.
> >
> > The kit also unlocks the possibility of writing refactoring tools
> > as gcc plugins, or could be used to build a parser for the output
> > of -fdiagnostics-parseable-fixits.
> >
> > Successfully bootstrapped®rtested the combination of the patches
> > on x86_64-pc-linux-gnu.
> >
> > I believe I can self-approve patch 3, and, potentially patch 4
> > as part of my "diagnostics maintainer" role, though this is
> > clearly a significant extension of the scope of diagnostics.
> >
> > OK for trunk? (assuming individual bootstraps®rtesting)
> >
> > [1] or use -fdiagnostics-parseable-fixits - but we don't have a
> > way to parse that output format yet
> > [2] the error-handling for write-back isn't great right now, so
> > the claim of atomicity is a stretch; is there a good cross-platform
> > way to guarantee atomicity of a set of filesystem operations?
> > (maybe create the replacements as tempfiles, rename the originals,
> > try to move all the replacements into place, and then unlink the
> > backups, with a rollback involving moving the backups into place)
> >
> > David Malcolm (4):
> > selftest: split out named_temp_file from temp_source_file
> > Improvements to typed_splay_tree
> > Introduce class edit_context
> > Add -fdiagnostics-generate-patch and -fdiagnostics-apply-fixits
> >
> > gcc/Makefile.in | 2 +
> > gcc/common.opt | 8 +
> > gcc/diagnostic-color.c | 8 +-
> > gcc/diagnostic.c | 11 +
> > gcc/diagnostic.h | 7 +
> > gcc/doc/invoke.texi | 28 +-
> > gcc/edit-context.c | 1341
> > ++++++++++++++++++++
> > gcc/edit-context.h | 66 +
> > gcc/selftest-run-tests.c | 2 +
> > gcc/selftest.c | 49 +-
> > gcc/selftest.h | 26 +-
> > .../diagnostic-test-show-locus-generate-patch.c | 77 ++
> > gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 +-
> > gcc/toplev.c | 20 +
> > gcc/typed-splay-tree.c | 79 ++
> > gcc/typed-splay-tree.h | 67 +
> > 16 files changed, 1770 insertions(+), 24 deletions(-)
> > create mode 100644 gcc/edit-context.c
> > create mode 100644 gcc/edit-context.h
> > create mode 100644
> > gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate
> > -patch.c
> > create mode 100644 gcc/typed-splay-tree.c
> >
> > --
> > 1.8.5.3
> >
> >
>
>
> So, if -fdiagnostics-apply-fixits writes the fixits back into the
> file
> as actual code, I'm wondering, could there also be a separate option
> to write them in as comments? Say if I want to be reminded to fix
> something, but am not quite sure if the fixit is right or not, and
> don't want to have a separate patch file lying around... e.g.,
> instead
> of changing
> return ptr.x;
> to
> return ptr->x;
> it could change it to
> return ptr.x; // ->
> or maybe put it below like:
> return ptr.x;
> /* -> */
> Just an idea I thought might be useful as a user... I dunno...
Thanks - interesting idea. I'm keen on making life easier for gcc
users, and I think there's "low hanging fruit" around fix-its (hence my
patches), so please keep suggesting ideas like this.
That said, I think that implementing this particular one robustly could
be non-trivial. Consider the case of a gcc plugin that tidies up code
to fix a particular coding style: it might emit this fix-it:
test.c:1:20: missing trailing '.' at end of comment
/* Open the pod bay doors */
^
.
i.e. in diff form as:
@@ -20,1 +20,1 @@
- /* Open the pod bay doors */
+ /* Open the pod bay doors. */
where the diagnostic simply has to emit an insertion fix-it.
Turning the fix-its into comments seems to me like a transformation
applied on top of the fix-its to turn them into insertions; Adding them
as trailing C++-style comments to the same line could work, but C-style
comments could go wrong quickly (think nested comments, also, what if
there are several changes in close proximity?).
Currently I feel like my typical interaction with a compiler looks like
this:
user: compiler, please compile this
compiler: DENIED! [dozens of lines of errors]
(user attempts to fix)
user: compiler, please compile this
compiler: DENIED! [hopefully
fewer lines of errors]
(user attempts to fix)
[...etc...]
user:
compiler, please compile this
compiler: DENIED! [one or two errors]
(u
ser attempts to fix)
user: compiler, please compile this
compiler:
(silently succeeds)
and it would be great if we could smooth the process a bit, so ideas are
welcome.
Thanks
Dave
