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
