On Wed, Aug 24, 2016 at 3:28 AM, David Malcolm <dmalc...@redhat.com> 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.
I don't like this very much - option (A) looks good with the user having to manually apply a patch. But (B) is just asking for trouble ;) I'd rather have -fdiagnostics-apply-fixits apply the fixit hints for the current compilation only (so you'll see if the compile is successful after applying the suggestions). That is, work more in a "error recovery" mode. Richard. > 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 >
