On Wed, Aug 24, 2016 at 09:13:51PM -0400, David Malcolm wrote: > This version removes edit_context::apply_changes and related > functionality, retaining the ability to generate diffs. > > It also improves error-handling (adding a selftest for this). > > gcc/ChangeLog: > * Makefile.in (OBJS-libcommon): Add edit-context.o. > * diagnostic-color.c (color_dict): Add "diff-filename", > "diff-hunk", "diff-delete", and "diff-insert". > (parse_gcc_colors): Update default value of GCC_COLORS in comment > to reflect above changes. > * edit-context.c: New file. > * edit-context.h: New file. > * selftest-run-tests.c (selftest::run_tests): Call > edit_context_c_tests. > * selftest.h (edit_context_c_tests): New decl. > --- > gcc/Makefile.in | 1 + > gcc/diagnostic-color.c | 8 +- > gcc/edit-context.c | 1347 > ++++++++++++++++++++++++++++++++++++++++++++++ > gcc/edit-context.h | 65 +++ > gcc/selftest-run-tests.c | 1 + > gcc/selftest.h | 1 + > 6 files changed, 1422 insertions(+), 1 deletion(-) > create mode 100644 gcc/edit-context.c > create mode 100644 gcc/edit-context.h > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index f5f3339..506f0d4 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -1561,6 +1561,7 @@ OBJS = \ > # Objects in libcommon.a, potentially used by all host binaries and with > # no target dependencies. > OBJS-libcommon = diagnostic.o diagnostic-color.o diagnostic-show-locus.o \ > + edit-context.o \ > pretty-print.o intl.o \ > vec.o input.o version.o hash-table.o ggc-none.o memory-block.o \ > selftest.o > diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c > index f76c87b..decbe84 100644 > --- a/gcc/diagnostic-color.c > +++ b/gcc/diagnostic-color.c > @@ -168,6 +168,10 @@ static struct color_cap color_dict[] = > { "range2", SGR_SEQ (COLOR_FG_BLUE), 6, false }, > { "locus", SGR_SEQ (COLOR_BOLD), 5, false }, > { "quote", SGR_SEQ (COLOR_BOLD), 5, false }, > + { "diff-filename", SGR_SEQ (COLOR_BOLD), 13, false }, > + { "diff-hunk", SGR_SEQ (COLOR_FG_CYAN), 9, false }, > + { "diff-delete", SGR_SEQ (COLOR_FG_RED), 11, false }, > + { "diff-insert", SGR_SEQ (COLOR_FG_GREEN), 11, false }, > { NULL, NULL, 0, false } > }; > > @@ -196,7 +200,9 @@ colorize_stop (bool show_color) > } > > /* Parse GCC_COLORS. The default would look like: > - > GCC_COLORS='error=01;31:warning=01;35:note=01;36:range1=32:range2=34;locus=01:quote=01' > + GCC_COLORS='error=01;31:warning=01;35:note=01;36:\ > + range1=32:range2=34:locus=01:quote=01:\ > + diff-filename=01:diff-hunk=32:diff-delete=31:diff-insert=32' > No character escaping is needed or supported. */ > static bool > parse_gcc_colors (void) > diff --git a/gcc/edit-context.c b/gcc/edit-context.c > new file mode 100644 > index 0000000..6b79b45 > --- /dev/null > +++ b/gcc/edit-context.c > @@ -0,0 +1,1347 @@ > +/* Determining the results of applying fix-its. > + Copyright (C) 2016 Free Software Foundation, Inc. > + > +This file is part of GCC. > + > +GCC is free software; you can redistribute it and/or modify it under > +the terms of the GNU General Public License as published by the Free > +Software Foundation; either version 3, or (at your option) any later > +version. > + > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY > +WARRANTY; without even the implied warranty of MERCHANTABILITY or > +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. */ > + > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "line-map.h" > +#include "edit-context.h" > +#include "pretty-print.h" > +#include "diagnostic-color.h" > +#include "selftest.h" > + > +/* This file implements a way to track the effect of fix-its, > + via a class edit_context; the other classes are support classes for > + edit_context. > + > + A complication here is that fix-its are expressed relative to coordinates > + in the file when it was parsed, before any changes have been made, and > + so if there's more that one fix-it to be applied, we have to adjust > + later fix-its to allow for the changes made by earlier ones. This > + is done by the various "get_effective_column" methods. > + > + The "filename" params are required to outlive the edit_context (no > + copy of the underlying str is taken, just the ptr). */ > + > +/* Forward decls. class edit_context is declared within edit-context.h. > + The other types are declared here. */ > +class edit_context; > +class edited_file; > +class line_state; > +class line_event; > + class insert_event; > + class replace_event; > + > +/* A struct to hold the params of a print_diff call. */ > + > +struct diff > +{ > + diff (pretty_printer *pp, bool show_filenames) > + : m_pp (pp), m_show_filenames (show_filenames) {} > + > + pretty_printer *m_pp; > + bool m_show_filenames; > +}; > + > +/* The state of one named file within an edit_context: the filename, > + the current content of the file after applying all changes so far, and > + a record of the changes, so that further changes can be applied in > + the correct place. */ > + > +class edited_file > +{ > + public: > + edited_file (const char *filename); > + bool read_from_file (); > + ~edited_file ();
nit picking, but I think it would be nice to have the ctor and dtor next to each other. > + static void delete_cb (edited_file *file); > + > + const char *get_filename () const { return m_filename; } > + const char *get_content () const { return m_content; } > + > + bool apply_insert (int line, int column, const char *str, int len); > + bool apply_replace (int line, int start_column, > + int finish_column, > + const char *replacement_str, > + int replacement_len); > + int get_effective_column (int line, int column); > + > + static int call_print_diff (const char *, edited_file *file, > + void *user_data) > + { > + diff *d = (diff *)user_data; > + file->print_diff (d->m_pp, d->m_show_filenames); > + return 0; > + } > + > + private: > + void print_diff (pretty_printer *pp, bool show_filenames); > + void print_line_in_diff (pretty_printer *pp, int line_num); > + line_state *get_line (int line); > + line_state &get_or_insert_line (int line); > + void ensure_capacity (size_t len); > + void ensure_terminated (); > + char *get_line_start (int line_num); > + void ensure_line_start_index (); > + void evict_line_start_index (); > + int get_num_lines (); > + > + const char *m_filename; > + char *m_content; > + size_t m_len; > + size_t m_alloc_sz; > + typed_splay_tree<int, line_state *> m_edited_lines; > + auto_vec<int> m_line_start_index; have you considered parsing lines when you read the file, and then storing a vec<char *> where element I is line I in the file? Its more allocations, but when you need to edit a line you'll only need to copy around that line instead of the whole rest of the file. > +}; > + > +/* A mapping from pre-edit columns to post-edit columns > + within one line of one file. */ > + > +class line_state > +{ > + public: > + line_state (int line_num) : m_line_num (line_num), m_line_events () {} > + ~line_state (); > + static void delete_cb (line_state *ls); > + > + int get_line_num () const { return m_line_num; } > + > + int get_effective_column (int orig_column) const; > + void apply_insert (int column, int len); > + void apply_replace (int start, int finish, int len); > + > + private: > + int m_line_num; > + auto_vec <line_event *> m_line_events; > +}; > + > +/* Abstract base class for representing events that have occurred > + on one line of one file. */ > + > +class line_event > +{ > + public: > + virtual ~line_event () {} > + virtual int get_effective_column (int orig_column) const = 0; > +}; > + > +/* Concrete subclass of line_event: an insertion of some text > + at some column on the line. > + > + Subsequent events will need their columns adjusting if they're > + are on this line and their column is >= the insertion point. */ > + > +class insert_event : public line_event > +{ > + public: > + insert_event (int column, int len) : m_column (column), m_len (len) {} > + int get_effective_column (int orig_column) const FINAL OVERRIDE > + { > + if (orig_column >= m_column) > + return orig_column + m_len; > + else > + return orig_column; > + } > + > + private: > + int m_column; > + int m_len; > +}; > + > +/* Concrete subclass of line_event: the replacement of some text > + betweeen some columns on the line. > + > + Subsequent events will need their columns adjusting if they're > + are on this line and their column is >= the finish point. */ > + > +class replace_event : public line_event > +{ > + public: > + replace_event (int start, int finish, int len) : m_start (start), > + m_finish (finish), m_delta (len - (finish + 1 - start)) {} > + > + int get_effective_column (int orig_column) const FINAL OVERRIDE > + { > + if (orig_column >= m_start) > + return orig_column += m_delta; > + else > + return orig_column; What happens when orig_column is within the text being replaced? > + } > + > + private: > + int m_start; > + int m_finish; > + int m_delta; > +}; It seems like it would greatly simplify things to merge fixit_insert and fixit_replace so that an insertion is just a replacement where the start and end of the replaced text are the same. That would also have the nice effect of making fixit_replace smaller since you wouldn't need the vtable any more. > +change_line (edit_context &edit, int line_num) I guess this is test only, put I'm not a fan of non const references. Trev