* [PATCH 0/4] Applying fix-its on behalf of the user to their code
@ 2016-08-24 0:59 David Malcolm
2016-08-24 0:59 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
` (6 more replies)
0 siblings, 7 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-24 0:59 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/4] selftest: split out named_temp_file from temp_source_file
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
@ 2016-08-24 0:59 ` David Malcolm
2016-08-24 1:00 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
` (5 subsequent siblings)
6 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-24 0:59 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
Split out a new base class for temp_source_file, named_temp_file,
moving the deletion to the base class dtor, so that we can write
out temporary files in other ways in selftests.
gcc/ChangeLog:
* selftest.c (selftest::named_temp_file::named_temp_file): New
ctor.
(selftest::temp_source_file::~temp_source_file): Move to...
(selftest::named_temp_file::~named_temp_file): ...here.
(selftest::test_named_temp_file): New function.
(selftest::selftest_c_tests): Call test_named_temp_file.
* selftest.h (class named_temp_file): New class.
(class temp_source_file): Convert to a subclass of named_temp_file.
---
gcc/selftest.c | 49 +++++++++++++++++++++++++++++++++++--------------
gcc/selftest.h | 24 +++++++++++++++++-------
2 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/gcc/selftest.c b/gcc/selftest.c
index 629db98..e6c9510 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -120,34 +120,40 @@ selftest::assert_str_contains (const location &loc,
desc_haystack, desc_needle, val_haystack, val_needle);
}
-/* Constructor. Create a tempfile using SUFFIX, and write CONTENT to
- it. Abort if anything goes wrong, using LOC as the effective
- location in the problem report. */
+/* Constructor. Generate a name for the file. */
-selftest::temp_source_file::temp_source_file (const location &loc,
- const char *suffix,
- const char *content)
+selftest::named_temp_file::named_temp_file (const char *suffix)
{
m_filename = make_temp_file (suffix);
ASSERT_NE (m_filename, NULL);
-
- FILE *out = fopen (m_filename, "w");
- if (!out)
- ::selftest::fail_formatted (loc, "unable to open tempfile: %s",
- m_filename);
- fprintf (out, "%s", content);
- fclose (out);
}
/* Destructor. Delete the tempfile. */
-selftest::temp_source_file::~temp_source_file ()
+selftest::named_temp_file::~named_temp_file ()
{
unlink (m_filename);
diagnostics_file_cache_forcibly_evict_file (m_filename);
free (m_filename);
}
+/* Constructor. Create a tempfile using SUFFIX, and write CONTENT to
+ it. Abort if anything goes wrong, using LOC as the effective
+ location in the problem report. */
+
+selftest::temp_source_file::temp_source_file (const location &loc,
+ const char *suffix,
+ const char *content)
+: named_temp_file (suffix)
+{
+ FILE *out = fopen (get_filename (), "w");
+ if (!out)
+ ::selftest::fail_formatted (loc, "unable to open tempfile: %s",
+ get_filename ());
+ fprintf (out, "%s", content);
+ fclose (out);
+}
+
/* Selftests for the selftest system itself. */
namespace selftest {
@@ -167,12 +173,27 @@ test_assertions ()
ASSERT_STR_CONTAINS ("foo bar baz", "bar");
}
+/* Verify named_temp_file. */
+
+static void
+test_named_temp_file ()
+{
+ named_temp_file t (".txt");
+ FILE *f = fopen (t.get_filename (), "w");
+ if (!f)
+ selftest::fail_formatted (SELFTEST_LOCATION,
+ "unable to open %s for writing",
+ t.get_filename ());
+ fclose (f);
+}
+
/* Run all of the selftests within this file. */
void
selftest_c_tests ()
{
test_assertions ();
+ test_named_temp_file ();
}
} // namespace selftest
diff --git a/gcc/selftest.h b/gcc/selftest.h
index b073ed6..fd3c6b0 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -77,22 +77,32 @@ extern void assert_str_contains (const location &loc,
const char *val_haystack,
const char *val_needle);
-/* A class for writing out a temporary sourcefile for use in selftests
- of input handling. */
+/* A named temporary file for use in selftests.
+ Usable for writing out files, and as the base class for
+ temp_source_file.
+ The file is unlinked in the destructor. */
-class temp_source_file
+class named_temp_file
{
public:
- temp_source_file (const location &loc, const char *suffix,
- const char *content);
- ~temp_source_file ();
-
+ named_temp_file (const char *suffix);
+ ~named_temp_file ();
const char *get_filename () const { return m_filename; }
private:
char *m_filename;
};
+/* A class for writing out a temporary sourcefile for use in selftests
+ of input handling. */
+
+class temp_source_file : public named_temp_file
+{
+ public:
+ temp_source_file (const location &loc, const char *suffix,
+ const char *content);
+};
+
/* Various selftests involving location-handling require constructing a
line table and one or more line maps within it.
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Introduce class edit_context
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
` (2 preceding siblings ...)
2016-08-24 1:00 ` [PATCH 4/4] Add -fdiagnostics-generate-patch and -fdiagnostics-apply-fixits David Malcolm
@ 2016-08-24 1:00 ` David Malcolm
2016-08-24 1:58 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Eric Gallager
` (2 subsequent siblings)
6 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-24 1:00 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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 | 1341 ++++++++++++++++++++++++++++++++++++++++++++++
gcc/edit-context.h | 66 +++
gcc/selftest-run-tests.c | 1 +
gcc/selftest.h | 1 +
6 files changed, 1417 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..e6654e1
--- /dev/null
+++ b/gcc/edit-context.c
@@ -0,0 +1,1341 @@
+/* Applying fix-its on behalf of the user to their code.
+ 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 apply fix-its on behalf of the user,
+ 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 ();
+ 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;
+ }
+ static int call_apply_changes (const char *, edited_file *file,
+ void *)
+ {
+ file->apply_changes ();
+ return 0;
+ }
+
+ private:
+ void print_diff (pretty_printer *pp, bool show_filenames);
+ void print_line_in_diff (pretty_printer *pp, int line_num);
+ void apply_changes ();
+ 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;
+};
+
+/* 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;
+ }
+
+ private:
+ int m_start;
+ int m_finish;
+ int m_delta;
+};
+
+/* Implementation of class edit_context. */
+
+/* edit_context's ctor. */
+
+edit_context::edit_context ()
+: m_valid (true),
+ m_files (strcmp, NULL, edited_file::delete_cb)
+{}
+
+/* Add any fixits within RICHLOC to this context, recording the
+ changes that they make. */
+
+void
+edit_context::add_fixits (rich_location *richloc)
+{
+ if (!m_valid)
+ return;
+ for (unsigned i = 0; i < richloc->get_num_fixit_hints (); i++)
+ {
+ const fixit_hint *hint = richloc->get_fixit_hint (i);
+ switch (hint->get_kind ())
+ {
+ case fixit_hint::INSERT:
+ if (!apply_insert ((const fixit_insert *)hint))
+ {
+ /* Failure. */
+ m_valid = false;
+ return;
+ }
+ break;
+ case fixit_hint::REPLACE:
+ if (!apply_replace ((const fixit_replace *)hint))
+ {
+ /* Failure. */
+ m_valid = false;
+ return;
+ }
+ break;
+ default:
+ gcc_unreachable ();
+ }
+ }
+}
+
+/* Get the content of the given file, with fix-its applied.
+ If any errors occurred in this edit_context, return NULL. */
+
+const char *
+edit_context::get_content (const char *filename)
+{
+ if (!m_valid)
+ return NULL;
+ edited_file &file = get_or_insert_file (filename);
+ return file.get_content ();
+}
+
+/* Map a location before the edits to a column number after the edits.
+ This method is for the selftests. */
+
+int
+edit_context::get_effective_column (const char *filename, int line,
+ int column)
+{
+ edited_file *file = get_file (filename);
+ if (!file)
+ return column;
+ return file->get_effective_column (line, column);
+}
+
+/* Generate a unified diff. The resulting string should be freed by the
+ caller. Primarily for selftests.
+ If any errors occurred in this edit_context, return NULL. */
+
+char *
+edit_context::generate_diff (bool show_filenames)
+{
+ if (!m_valid)
+ return NULL;
+
+ pretty_printer pp;
+ print_diff (&pp, show_filenames);
+ return xstrdup (pp_formatted_text (&pp));
+}
+
+/* Print a unified diff to PP, showing the changes made within the
+ context. */
+
+void
+edit_context::print_diff (pretty_printer *pp, bool show_filenames)
+{
+ if (!m_valid)
+ return;
+ diff d (pp, show_filenames);
+ m_files.foreach (edited_file::call_print_diff, &d);
+}
+
+/* If the context is valid, write back all changes to the underlying
+ files in the filesystem and return true.
+
+ Otherwise return false. */
+
+bool
+edit_context::apply_changes ()
+{
+ if (!m_valid)
+ return false;
+ m_files.foreach (edited_file::call_apply_changes, NULL);
+ return true;
+}
+
+/* Attempt to apply the given fixit. Return true if it can be
+ applied, or false otherwise. */
+
+bool
+edit_context::apply_insert (const fixit_insert *insert)
+{
+ expanded_location exploc = expand_location (insert->get_location ());
+
+ if (exploc.column == 0)
+ return false;
+
+ edited_file &file = get_or_insert_file (exploc.file);
+ return file.apply_insert (exploc.line, exploc.column, insert->get_string (),
+ insert->get_length ());
+}
+
+/* Attempt to apply the given fixit. Return true if it can be
+ applied, or false otherwise. */
+
+bool
+edit_context::apply_replace (const fixit_replace *replace)
+{
+ source_range range = replace->get_range ();
+
+ expanded_location start = expand_location (range.m_start);
+ expanded_location finish = expand_location (range.m_finish);
+ if (start.file != finish.file)
+ return false;
+ if (start.line != finish.line)
+ return false;
+ if (start.column == 0)
+ return false;
+ if (finish.column == 0)
+ return false;
+
+ edited_file &file = get_or_insert_file (start.file);
+ return file.apply_replace (start.line, start.column, finish.column,
+ replace->get_string (),
+ replace->get_length ());
+}
+
+/* Locate the edited_file * for FILENAME, if any
+ Return NULL if there isn't one. */
+
+edited_file *
+edit_context::get_file (const char *filename)
+{
+ gcc_assert (filename);
+ return m_files.lookup (filename);
+}
+
+/* Locate the edited_file for FILENAME, adding one if there isn't one. */
+
+edited_file &
+edit_context::get_or_insert_file (const char *filename)
+{
+ gcc_assert (filename);
+
+ edited_file *file = get_file (filename);
+ if (file)
+ return *file;
+
+ /* Not found. */
+ file = new edited_file (filename);
+ if (!file->read_from_file ())
+ m_valid = false;
+ m_files.insert (filename, file);
+ return *file;
+}
+
+/* Implementation of class edited_file. */
+
+/* Callback for m_edited_lines, for comparing line numbers. */
+
+static int line_comparator (int a, int b)
+{
+ return a - b;
+}
+
+/* edited_file's constructor. */
+
+edited_file::edited_file (const char *filename)
+: m_filename (filename),
+ m_content (NULL), m_len (0), m_alloc_sz (0),
+ m_edited_lines (line_comparator, NULL, line_state::delete_cb)
+{
+}
+
+/* Read the contents of the file from the filesystem into memory.
+ Return true if successful; false if any problems occurred. */
+
+bool
+edited_file::read_from_file ()
+{
+ FILE *f_in = fopen (m_filename, "r");
+ if (!f_in)
+ return false;
+
+ char buf[4096];
+ size_t iter_sz_in;
+
+ while ( (iter_sz_in = fread (buf, 1, sizeof (buf), f_in)) )
+ {
+ gcc_assert (m_alloc_sz >= m_len);
+ size_t old_len = m_len;
+ m_len += iter_sz_in;
+ ensure_capacity (m_len);
+ memcpy (m_content + old_len, buf, iter_sz_in);
+ }
+
+ if (!feof (f_in))
+ return false;
+
+ fclose (f_in);
+
+ ensure_terminated ();
+
+ return true;
+}
+
+/* edited_file's dtor. */
+
+edited_file::~edited_file ()
+{
+ free (m_content);
+}
+
+/* A callback for deleting edited_file *, for use as a
+ delete_value_fn for edit_context::m_files. */
+
+void
+edited_file::delete_cb (edited_file *file)
+{
+ delete file;
+}
+
+/* Insert the string INSERT_STR with length INSERT_LEN at LINE and COLUMN,
+ updating the in-memory copy of the file, and the record of edits to
+ the line. */
+
+bool
+edited_file::apply_insert (int line, int column,
+ const char *insert_str,
+ int insert_len)
+{
+ column = get_effective_column (line, column);
+
+ int start_offset = column - 1;
+ gcc_assert (start_offset >= 0);
+
+ /* Ensure buffer is big enough. */
+ size_t new_len = m_len + insert_len;
+ ensure_capacity (new_len);
+
+ char *line_start = get_line_start (line);
+ if (!line_start)
+ return false;
+
+ char *suffix = line_start + start_offset;
+ size_t len_suffix = (m_content + m_len) - suffix;
+
+ /* Move successor content into position. They overlap, so use memmove. */
+ memmove (line_start + start_offset + insert_len,
+ suffix, len_suffix);
+
+ /* Replace target content. They don't overlap, so use memcpy. */
+ memcpy (line_start + start_offset,
+ insert_str,
+ insert_len);
+
+ m_len = new_len;
+
+ ensure_terminated ();
+ evict_line_start_index ();
+
+ /* Record the insertion, so that followup changes to this line
+ can have their columns adjusted as appropriate. */
+ line_state &ls = get_or_insert_line (line);
+ ls.apply_insert (column, insert_len);
+
+ return true;
+}
+
+/* Replace columns START_COLUMN through FINISH_COLUMN of LINE
+ with the string REPLACEMENT_STR of length REPLACEMENT_LEN,
+ updating the in-memory copy of the file, and the record of edits to
+ the line. */
+
+bool
+edited_file::apply_replace (int line, int start_column,
+ int finish_column,
+ const char *replacement_str,
+ int replacement_len)
+{
+ start_column = get_effective_column (line, start_column);
+ finish_column = get_effective_column (line, finish_column);
+
+ int start_offset = start_column - 1;
+ int end_offset = finish_column - 1;
+
+ gcc_assert (start_offset >= 0);
+ gcc_assert (end_offset >= 0);
+
+ gcc_assert (start_column <= finish_column);
+
+ size_t victim_len = end_offset - start_offset + 1;
+
+ /* Ensure buffer is big enough. */
+ size_t new_len = m_len + replacement_len - victim_len;
+ ensure_capacity (new_len);
+
+ char *line_start = get_line_start (line);
+ if (!line_start)
+ return false;
+
+ char *suffix = line_start + end_offset + 1;
+ size_t len_suffix = (m_content + m_len) - suffix;
+
+ /* Move successor content into position. They overlap, so use memmove. */
+ memmove (line_start + start_offset + replacement_len,
+ suffix, len_suffix);
+
+ /* Replace target content. They don't overlap, so use memcpy. */
+ memcpy (line_start + start_offset,
+ replacement_str,
+ replacement_len);
+
+ m_len = new_len;
+
+ ensure_terminated ();
+ evict_line_start_index ();
+
+ /* Record the replacement, so that followup changes to this line
+ can have their columns adjusted as appropriate. */
+ line_state &ls = get_or_insert_line (line);
+ ls.apply_replace (start_column, finish_column, replacement_len);
+
+ return true;
+}
+
+/* Given line LINE, map from COLUMN in the input file to its current
+ column after edits have been applied. */
+
+int
+edited_file::get_effective_column (int line, int column)
+{
+ const line_state *ls = get_line (line);
+ if (!ls)
+ return column;
+ return ls->get_effective_column (column);
+}
+
+/* Print a unified diff to PP, showing any changes that have occurred
+ to this file. */
+
+void
+edited_file::print_diff (pretty_printer *pp, bool show_filenames)
+{
+ if (show_filenames)
+ {
+ pp_string (pp, colorize_start (pp_show_color (pp), "diff-filename"));
+ pp_printf (pp, "--- %s\n", m_filename);
+ pp_printf (pp, "+++ %s\n", m_filename);
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+ }
+
+ line_state *ls = m_edited_lines.min ();
+
+ const int context_lines = 3;
+
+ while (ls)
+ {
+ int start_of_hunk = ls->get_line_num ();
+ start_of_hunk -= context_lines;
+ if (start_of_hunk < 1)
+ start_of_hunk = 1;
+
+ /* Locate end of hunk, merging in changed lines
+ that are sufficiently close. */
+ while (true)
+ {
+ line_state *next_ls
+ = m_edited_lines.successor (ls->get_line_num ());
+ if (!next_ls)
+ break;
+ if (ls->get_line_num () + context_lines
+ >= next_ls->get_line_num () - context_lines)
+ ls = next_ls;
+ else
+ break;
+ }
+ int end_of_hunk = ls->get_line_num ();
+ end_of_hunk += context_lines;
+ if (end_of_hunk > get_num_lines ())
+ end_of_hunk = get_num_lines ();
+
+ int num_lines = end_of_hunk - start_of_hunk + 1;
+
+ pp_string (pp, colorize_start (pp_show_color (pp), "diff-hunk"));
+ pp_printf (pp, "@@ -%i,%i +%i,%i @@\n", start_of_hunk, num_lines,
+ start_of_hunk, num_lines);
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+ int line_num = start_of_hunk;
+ while (line_num <= end_of_hunk)
+ {
+ line_state *ls = get_line (line_num);
+ if (ls)
+ {
+ /* We have an edited line.
+ Consolidate into runs of changed lines. */
+ const int first_changed_line_in_run = line_num;
+ while (get_line (line_num))
+ line_num++;
+ const int last_changed_line_in_run = line_num - 1;
+
+ pp_string (pp, colorize_start (pp_show_color (pp),
+ "diff-delete"));
+
+ /* Show old version of lines. */
+ for (line_num = first_changed_line_in_run;
+ line_num <= last_changed_line_in_run;
+ line_num++)
+ {
+ int line_size;
+ const char *old_line
+ = location_get_source_line (m_filename, line_num,
+ &line_size);
+ pp_character (pp, '-');
+ for (int i = 0; i < line_size; i++)
+ pp_character (pp, old_line[i]);
+ pp_character (pp, '\n');
+ }
+
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+ pp_string (pp, colorize_start (pp_show_color (pp),
+ "diff-insert"));
+
+ /* Show new version of lines. */
+ for (line_num = first_changed_line_in_run;
+ line_num <= last_changed_line_in_run;
+ line_num++)
+ {
+ pp_character (pp, '+');
+ print_line_in_diff (pp, line_num);
+ }
+
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+ }
+ else
+ {
+ /* Unchanged line. */
+ pp_character (pp, ' ');
+ print_line_in_diff (pp, line_num);
+ line_num++;
+ }
+ }
+
+ ls = m_edited_lines.successor (ls->get_line_num ());
+ }
+}
+
+/* Subroutine of edited_file::print_diff. Print the given line
+ (after edits) to PP (with a trailing newline). */
+
+void
+edited_file::print_line_in_diff (pretty_printer *pp, int line_num)
+{
+ const char *line = get_line_start (line_num);
+ while (char ch = *line++)
+ {
+ pp_character (pp, ch);
+ if (ch == '\n')
+ break;
+ }
+}
+
+/* Write the state from memory back to the filesystem. */
+
+void
+edited_file::apply_changes ()
+{
+ FILE *f = fopen (get_filename (), "w");
+ if (!f)
+ return; /* unfortunately this part is not atomic. */
+
+ fwrite (m_content, 1, m_len, f);
+
+ fclose (f);
+}
+
+/* Get the state of LINE within the file, or NULL if it is untouched. */
+
+line_state *
+edited_file::get_line (int line)
+{
+ return m_edited_lines.lookup (line);
+}
+
+/* Get the state of LINE within the file, creating a state for it
+ if necessary. */
+
+line_state &
+edited_file::get_or_insert_line (int line)
+{
+ line_state *ls = get_line (line);
+ if (ls)
+ return *ls;
+ ls = new line_state (line);
+ m_edited_lines.insert (line, ls);
+ return *ls;
+}
+
+/* Ensure that the buffer for m_content is at least large enough to hold
+ a string of length LEN and its 0-terminator, doubling on repeated
+ allocations. */
+
+void
+edited_file::ensure_capacity (size_t len)
+{
+ /* Allow 1 extra byte for 0-termination. */
+ if (m_alloc_sz < (len + 1))
+ {
+ size_t new_alloc_sz = (len + 1) * 2;
+ m_content = (char *)xrealloc (m_content, new_alloc_sz);
+ m_alloc_sz = new_alloc_sz;
+ }
+}
+
+/* Ensure that m_content is 0-terminated. */
+
+void
+edited_file::ensure_terminated ()
+{
+ /* 0-terminate the buffer. */
+ gcc_assert (m_len < m_alloc_sz);
+ m_content[m_len] = '\0';
+}
+
+/* Return a pointer to the start of the given line within the
+ m_content buffer. */
+
+char *
+edited_file::get_line_start (int line_num)
+{
+ ensure_line_start_index ();
+ return &m_content[m_line_start_index[line_num - 1]];
+}
+
+/* If necessary, regenerate the index of line-start offsets
+ based on the current state of m_content. */
+
+void
+edited_file::ensure_line_start_index ()
+{
+ if (m_line_start_index.length () == 0)
+ {
+ int offset = 0;
+ m_line_start_index.safe_push (0);
+ while (char ch = m_content[offset++])
+ {
+ if (ch == '\n')
+ if (m_content[offset])
+ m_line_start_index.safe_push (offset);
+ if (ch == '\0')
+ break;
+ }
+ }
+}
+
+/* Evict the index of line-start offsets. */
+
+void
+edited_file::evict_line_start_index ()
+{
+ m_line_start_index.truncate (0);
+}
+
+/* Get the total number of lines in m_content. */
+
+int
+edited_file::get_num_lines ()
+{
+ ensure_line_start_index ();
+ return m_line_start_index.length ();
+}
+
+/* Implementation of class line_state. */
+
+/* A callback for deleting line_state *, for use as a
+ delete_value_fn for edited_file::m_edited_lines. */
+
+void
+line_state::delete_cb (line_state *ls)
+{
+ delete ls;
+}
+
+/* line_state's dtor. */
+
+line_state::~line_state ()
+{
+ int i;
+ line_event *event;
+ FOR_EACH_VEC_ELT (m_line_events, i, event)
+ delete event;
+}
+
+/* Map a location before the edits to a column number after the edits,
+ within a specific line. */
+
+int
+line_state::get_effective_column (int orig_column) const
+{
+ int i;
+ line_event *event;
+ FOR_EACH_VEC_ELT (m_line_events, i, event)
+ orig_column = event->get_effective_column (orig_column);
+ return orig_column;
+}
+
+/* Record that a string of length LEN was inserted at COLUMN within
+ this line, so that future changes to the line can have their
+ column information adjusted accordingly. */
+
+void
+line_state::apply_insert (int column, int len)
+{
+ m_line_events.safe_push (new insert_event (column, len));
+}
+
+/* Record that the string between columns START and FINISH
+ of this line was replaced with a string of length LEN,
+ so that future changes to the line can have their
+ column information adjusted accordingly. */
+
+void
+line_state::apply_replace (int start, int finish, int len)
+{
+ m_line_events.safe_push (new replace_event (start, finish, len));
+}
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Selftests of code-editing. */
+
+/* Test applying an "insert" fixit. */
+
+static void
+test_applying_fixits_insert (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................0000000001111111.
+ .....................1234567890123456. */
+ const char *content = ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, tmp.get_filename (), 2);
+
+ /* Add a comment in front of "bar.field". */
+ location_t start = linemap_position_for_column (line_table, 7);
+ rich_location richloc (line_table, start);
+ richloc.add_fixit_insert (start, "/* inserted */");
+
+ if (start > LINE_MAP_MAX_LOCATION_WITH_COLS)
+ return;
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (start <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ ASSERT_STREQ ("/* before */\n"
+ "foo = /* inserted */bar.field;\n"
+ "/* after */\n", result);
+
+ /* Verify that locations on other lines aren't affected by the change. */
+ ASSERT_EQ (100, edit.get_effective_column (filename, 1, 100));
+ ASSERT_EQ (100, edit.get_effective_column (filename, 3, 100));
+
+ /* Verify locations on the line before the change. */
+ ASSERT_EQ (1, edit.get_effective_column (filename, 2, 1));
+ ASSERT_EQ (6, edit.get_effective_column (filename, 2, 6));
+
+ /* Verify locations on the line at and after the change. */
+ ASSERT_EQ (21, edit.get_effective_column (filename, 2, 7));
+ ASSERT_EQ (22, edit.get_effective_column (filename, 2, 8));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.field;\n"
+ "+foo = /* inserted */bar.field;\n"
+ " /* after */\n", diff);
+ free (diff);
+}
+
+/* Test applying a "replace" fixit that grows the affected line. */
+
+static void
+test_applying_fixits_growing_replace (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................0000000001111111.
+ .....................1234567890123456. */
+ const char *content = ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Replace "field" with "m_field". */
+ location_t start = linemap_position_for_column (line_table, 11);
+ location_t finish = linemap_position_for_column (line_table, 15);
+ location_t field = make_location (start, start, finish);
+ rich_location richloc (line_table, field);
+ source_range range;
+ range.m_start = start;
+ range.m_finish = finish;
+ richloc.add_fixit_replace (range, "m_field");
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (finish <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = bar.m_field;\n"
+ "/* after */\n", result);
+
+ /* Verify location of ";" after the change. */
+ ASSERT_EQ (18, edit.get_effective_column (filename, 2, 16));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.field;\n"
+ "+foo = bar.m_field;\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Test applying a "replace" fixit that shrinks the affected line. */
+
+static void
+test_applying_fixits_shrinking_replace (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................000000000111111111.
+ .....................123456789012345678. */
+ const char *content = ("/* before */\n"
+ "foo = bar.m_field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Replace "field" with "m_field". */
+ location_t start = linemap_position_for_column (line_table, 11);
+ location_t finish = linemap_position_for_column (line_table, 17);
+ location_t m_field = make_location (start, start, finish);
+ rich_location richloc (line_table, m_field);
+ source_range range;
+ range.m_start = start;
+ range.m_finish = finish;
+ richloc.add_fixit_replace (range, "field");
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (finish <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n", result);
+
+ /* Verify location of ";" after the change. */
+ ASSERT_EQ (16, edit.get_effective_column (filename, 2, 18));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.m_field;\n"
+ "+foo = bar.field;\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Test applying a "remove" fixit. */
+
+static void
+test_applying_fixits_remove (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................000000000111111111.
+ .....................123456789012345678. */
+ const char *content = ("/* before */\n"
+ "foo = bar.m_field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Remove ".m_field". */
+ location_t start = linemap_position_for_column (line_table, 10);
+ location_t finish = linemap_position_for_column (line_table, 17);
+ rich_location richloc (line_table, start);
+ source_range range;
+ range.m_start = start;
+ range.m_finish = finish;
+ richloc.add_fixit_remove (range);
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (finish <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = bar;\n"
+ "/* after */\n", result);
+
+ /* Verify location of ";" after the change. */
+ ASSERT_EQ (10, edit.get_effective_column (filename, 2, 18));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.m_field;\n"
+ "+foo = bar;\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Test applying multiple fixits to one line. */
+
+static void
+test_applying_fixits_multiple (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................00000000011111111.
+ .....................12345678901234567. */
+ const char *content = ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ location_t c7 = linemap_position_for_column (line_table, 7);
+ location_t c9 = linemap_position_for_column (line_table, 9);
+ location_t c11 = linemap_position_for_column (line_table, 11);
+ location_t c15 = linemap_position_for_column (line_table, 15);
+ location_t c17 = linemap_position_for_column (line_table, 17);
+
+ if (c17 > LINE_MAP_MAX_LOCATION_WITH_COLS)
+ return;
+
+ /* Add a comment in front of "bar.field". */
+ rich_location insert_a (line_table, c7);
+ insert_a.add_fixit_insert (c7, "/* alpha */");
+
+ /* Add a comment after "bar.field;". */
+ rich_location insert_b (line_table, c17);
+ insert_b.add_fixit_insert (c17, "/* beta */");
+
+ /* Replace "bar" with "pub". */
+ rich_location replace_a (line_table, c7);
+ replace_a.add_fixit_replace (source_range::from_locations (c7, c9),
+ "pub");
+
+ /* Replace "field" with "meadow". */
+ rich_location replace_b (line_table, c7);
+ replace_b.add_fixit_replace (source_range::from_locations (c11, c15),
+ "meadow");
+
+ edit_context edit;
+ edit.add_fixits (&insert_a);
+ ASSERT_EQ (100, edit.get_effective_column (filename, 1, 100));
+ ASSERT_EQ (1, edit.get_effective_column (filename, 2, 1));
+ ASSERT_EQ (6, edit.get_effective_column (filename, 2, 6));
+ ASSERT_EQ (18, edit.get_effective_column (filename, 2, 7));
+ ASSERT_EQ (27, edit.get_effective_column (filename, 2, 16));
+ ASSERT_EQ (100, edit.get_effective_column (filename, 3, 100));
+
+ edit.add_fixits (&insert_b);
+ edit.add_fixits (&replace_a);
+ edit.add_fixits (&replace_b);
+
+ if (c17 <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = /* alpha */pub.meadow;/* beta */\n"
+ "/* after */\n",
+ edit.get_content (filename));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.field;\n"
+ "+foo = /* alpha */pub.meadow;/* beta */\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Subroutine of test_applying_fixits_multiple_lines.
+ Add the text "CHANGED: " to the front of the given line. */
+
+static void
+change_line (edit_context &edit, int line_num)
+{
+ const line_map_ordinary *ord_map
+ = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+ const int column = 1;
+ location_t loc =
+ linemap_position_for_line_and_column (line_table, ord_map,
+ line_num, column);
+
+ expanded_location exploc = expand_location (loc);
+ ASSERT_EQ (line_num, exploc.line);
+ ASSERT_EQ (column, exploc.column);
+
+ rich_location insert (line_table, loc);
+ insert.add_fixit_insert (loc, "CHANGED: ");
+ edit.add_fixits (&insert);
+}
+
+/* Test of editing multiple lines within a long file,
+ to ensure that diffs are generated as expected. */
+
+static void
+test_applying_fixits_multiple_lines ()
+{
+ /* Create a tempfile and write many lines of text to it. */
+ named_temp_file tmp (".txt");
+ const char *filename = tmp.get_filename ();
+ FILE *f = fopen (filename, "w");
+ ASSERT_NE (f, NULL);
+ for (int i = 1; i <= 1000; i++)
+ fprintf (f, "line %i\n", i);
+ fclose (f);
+
+ line_table_test ltt;
+ linemap_add (line_table, LC_ENTER, false, filename, 1);
+ linemap_position_for_column (line_table, 127);
+
+ edit_context edit;
+
+ /* A run of consecutive lines. */
+ change_line (edit, 2);
+ change_line (edit, 3);
+ change_line (edit, 4);
+
+ /* A run of nearby lines, within the contextual limit. */
+ change_line (edit, 150);
+ change_line (edit, 151);
+ change_line (edit, 153);
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,7 +1,7 @@\n"
+ " line 1\n"
+ "-line 2\n"
+ "-line 3\n"
+ "-line 4\n"
+ "+CHANGED: line 2\n"
+ "+CHANGED: line 3\n"
+ "+CHANGED: line 4\n"
+ " line 5\n"
+ " line 6\n"
+ " line 7\n"
+ "@@ -147,10 +147,10 @@\n"
+ " line 147\n"
+ " line 148\n"
+ " line 149\n"
+ "-line 150\n"
+ "-line 151\n"
+ "+CHANGED: line 150\n"
+ "+CHANGED: line 151\n"
+ " line 152\n"
+ "-line 153\n"
+ "+CHANGED: line 153\n"
+ " line 154\n"
+ " line 155\n"
+ " line 156\n", diff);
+ free (diff);
+
+ /* Ensure tmp stays alive until this point, so that the tempfile
+ persists until after the generate_diff call. */
+ tmp.get_filename ();
+}
+
+/* Test of converting an initializer for a named field from
+ the old GCC extension to C99 syntax.
+ Exercises a shrinking replacement followed by a growing
+ replacement on the same line. */
+
+static void
+test_applying_fixits_modernize_named_init ()
+{
+ /* Create a tempfile and write some text to it.
+ .....................00000000011111111.
+ .....................12345678901234567. */
+ const char *content = ("/* before */\n"
+ "bar : 1,\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt ();
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ location_t c1 = linemap_position_for_column (line_table, 1);
+ location_t c3 = linemap_position_for_column (line_table, 3);
+ location_t c8 = linemap_position_for_column (line_table, 8);
+
+ /* Replace "bar" with ".". */
+ rich_location r1 (line_table, c8);
+ r1.add_fixit_replace (source_range::from_locations (c1, c3),
+ ".");
+
+ /* Replace ":" with "bar =". */
+ rich_location r2 (line_table, c8);
+ r2.add_fixit_replace (source_range::from_locations (c8, c8),
+ "bar =");
+
+ /* The order should not matter. Do r1 then r2. */
+ {
+ edit_context edit;
+ edit.add_fixits (&r1);
+
+ /* Verify state after first replacement. */
+ {
+ /* We should now have:
+ ............00000000011.
+ ............12345678901. */
+ ASSERT_STREQ ("/* before */\n"
+ ". : 1,\n"
+ "/* after */\n",
+ edit.get_content (filename));
+ /* Location of the "1". */
+ ASSERT_EQ (6, edit.get_effective_column (filename, 2, 8));
+ /* Location of the ",". */
+ ASSERT_EQ (9, edit.get_effective_column (filename, 2, 11));
+ }
+
+ edit.add_fixits (&r2);
+
+ /* Verify state after second replacement.
+ ............00000000011111111.
+ ............12345678901234567. */
+ ASSERT_STREQ ("/* before */\n"
+ ". bar = 1,\n"
+ "/* after */\n",
+ edit.get_content (filename));
+ }
+
+ /* Try again, doing r2 then r1; the result should be the same. */
+ {
+ edit_context edit;
+ edit.add_fixits (&r2);
+ edit.add_fixits (&r1);
+ /*.............00000000011111111.
+ .............12345678901234567. */
+ ASSERT_STREQ ("/* before */\n"
+ ". bar = 1,\n"
+ "/* after */\n",
+ edit.get_content (filename));
+ }
+}
+
+/* Test of a fixit affecting a file that can't be read. */
+
+static void
+test_applying_fixits_unreadable_file ()
+{
+ const char *filename = "this-does-not-exist.txt";
+ line_table_test ltt ();
+ linemap_add (line_table, LC_ENTER, false, filename, 1);
+
+ location_t loc = linemap_position_for_column (line_table, 1);
+
+ rich_location insert (line_table, loc);
+ insert.add_fixit_insert (loc, "change 1");
+ insert.add_fixit_insert (loc, "change 2");
+
+ edit_context edit;
+ /* Attempting to add the fixits affecting the unreadable file
+ should transition the edit from valid to invalid. */
+ ASSERT_TRUE (edit.valid_p ());
+ edit.add_fixits (&insert);
+ ASSERT_FALSE (edit.valid_p ());
+ ASSERT_EQ (NULL, edit.get_content (filename));
+ ASSERT_EQ (NULL, edit.generate_diff (false));
+}
+
+/* Run all of the selftests within this file. */
+
+void
+edit_context_c_tests ()
+{
+ for_each_line_table_case (test_applying_fixits_insert);
+ for_each_line_table_case (test_applying_fixits_growing_replace);
+ for_each_line_table_case (test_applying_fixits_shrinking_replace);
+ for_each_line_table_case (test_applying_fixits_remove);
+ for_each_line_table_case (test_applying_fixits_multiple);
+ test_applying_fixits_multiple_lines ();
+ test_applying_fixits_modernize_named_init ();
+ test_applying_fixits_unreadable_file ();
+}
+
+} // namespace selftest
+
+#endif /* CHECKING_P */
diff --git a/gcc/edit-context.h b/gcc/edit-context.h
new file mode 100644
index 0000000..eb6c24b
--- /dev/null
+++ b/gcc/edit-context.h
@@ -0,0 +1,66 @@
+/* Applying fix-its on behalf of the user to their code.
+ 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/>. */
+
+#ifndef GCC_EDIT_CONTEXT_H
+#define GCC_EDIT_CONTEXT_H
+
+#include "typed-splay-tree.h"
+
+class edit_context;
+class edited_file;
+
+/* A set of changes to the source code.
+
+ The changes are "atomic" - if any changes can't be applied,
+ none of can be (tracked by the m_valid flag).
+
+ A complication here is that fix-its are expressed relative to coordinates
+ in the files when they were 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. */
+
+class edit_context
+{
+ public:
+ edit_context ();
+
+ bool valid_p () const { return m_valid; }
+
+ void add_fixits (rich_location *richloc);
+
+ const char *get_content (const char *filename);
+
+ int get_effective_column (const char *filename, int line, int column);
+
+ char *generate_diff (bool show_filenames);
+ void print_diff (pretty_printer *pp, bool show_filenames);
+ bool apply_changes ();
+
+ private:
+ bool apply_insert (const fixit_insert *insert);
+ bool apply_replace (const fixit_replace *replace);
+ edited_file *get_file (const char *filename);
+ edited_file &get_or_insert_file (const char *filename);
+
+ bool m_valid;
+ typed_splay_tree<const char *, edited_file *> m_files;
+};
+
+#endif /* GCC_EDIT_CONTEXT_H. */
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index e20bbd0..d9d3ea1 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -68,6 +68,7 @@ selftest::run_tests ()
rely on. */
diagnostic_show_locus_c_tests ();
diagnostic_c_tests ();
+ edit_context_c_tests ();
fold_const_c_tests ();
spellcheck_c_tests ();
spellcheck_tree_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 8e6c47a..41487af 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -150,6 +150,7 @@ for_each_line_table_case (void (*testcase) (const line_table_case &));
extern void bitmap_c_tests ();
extern void diagnostic_c_tests ();
extern void diagnostic_show_locus_c_tests ();
+extern void edit_context_c_tests ();
extern void et_forest_c_tests ();
extern void fold_const_c_tests ();
extern void fibonacci_heap_c_tests ();
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/4] Improvements to typed_splay_tree
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
2016-08-24 0:59 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
@ 2016-08-24 1:00 ` David Malcolm
2016-08-31 14:12 ` Bernd Schmidt
2016-08-24 1:00 ` [PATCH 4/4] Add -fdiagnostics-generate-patch and -fdiagnostics-apply-fixits David Malcolm
` (4 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-08-24 1:00 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch adds foreach, max and min methods to
class typed_splay_tree, along with the start of a selftest
suite.
gcc/ChangeLog:
* Makefile.in (OBJS): Add typed-splay-tree.o.
* selftest-run-tests.c (selftest::run_tests): Call
typed_splay_tree_c_tests.
* selftest.h (typed_splay_tree_c_tests): New decl.
* typed-splay-tree.c: New file.
* typed-splay-tree.h (typed_splay_tree::foreach_fn): New typedef.
(typed_splay_tree::max): New method.
(typed_splay_tree::min): New method.
(typed_splay_tree::foreach): New method.
(struct closure): New struct.
(inner_foreach_fn): New function.
---
gcc/Makefile.in | 1 +
gcc/selftest-run-tests.c | 1 +
gcc/selftest.h | 1 +
gcc/typed-splay-tree.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
gcc/typed-splay-tree.h | 67 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 149 insertions(+)
create mode 100644 gcc/typed-splay-tree.c
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 8d7cc51..f5f3339 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1542,6 +1542,7 @@ OBJS = \
tree-vectorizer.o \
tree-vrp.o \
tree.o \
+ typed-splay-tree.o \
valtrack.o \
value-prof.o \
var-tracking.o \
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 6453e31..e20bbd0 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -56,6 +56,7 @@ selftest::run_tests ()
ggc_tests_c_tests ();
sreal_c_tests ();
fibonacci_heap_c_tests ();
+ typed_splay_tree_c_tests ();
/* Mid-level data structures. */
input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index fd3c6b0..8e6c47a 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -165,6 +165,7 @@ extern void selftest_c_tests ();
extern void spellcheck_c_tests ();
extern void spellcheck_tree_c_tests ();
extern void sreal_c_tests ();
+extern void typed_splay_tree_c_tests ();
extern void tree_c_tests ();
extern void tree_cfg_c_tests ();
extern void vec_c_tests ();
diff --git a/gcc/typed-splay-tree.c b/gcc/typed-splay-tree.c
new file mode 100644
index 0000000..33992c1
--- /dev/null
+++ b/gcc/typed-splay-tree.c
@@ -0,0 +1,79 @@
+/* Selftests for typed-splay-tree.h.
+ 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 "typed-splay-tree.h"
+#include "selftest.h"
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Callback for use by test_str_to_int. */
+
+static int
+append_cb (const char *, int value, void *user_data)
+{
+ auto_vec <int> *vec = (auto_vec <int> *)user_data;
+ vec->safe_push (value);
+ return 0;
+}
+
+/* Test of typed_splay_tree <const char *, int>. */
+
+static void
+test_str_to_int ()
+{
+ typed_splay_tree <const char *, int> t (strcmp, NULL, NULL);
+
+ t.insert ("a", 1);
+ t.insert ("b", 2);
+ t.insert ("c", 3);
+
+ ASSERT_EQ (1, t.lookup ("a"));
+ ASSERT_EQ (2, t.lookup ("b"));
+ ASSERT_EQ (3, t.lookup ("c"));
+
+ ASSERT_EQ (2, t.predecessor ("c"));
+ ASSERT_EQ (3, t.successor ("b"));
+ ASSERT_EQ (1, t.min ());
+ ASSERT_EQ (3, t.max ());
+
+ /* Test foreach by appending to a vec, and verifying the vec. */
+ auto_vec <int> v;
+ t.foreach (append_cb, &v);
+ ASSERT_EQ (3, v.length ());
+ ASSERT_EQ (1, v[0]);
+ ASSERT_EQ (2, v[1]);
+ ASSERT_EQ (3, v[2]);
+}
+
+/* Run all of the selftests within this file. */
+
+void
+typed_splay_tree_c_tests ()
+{
+ test_str_to_int ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/typed-splay-tree.h b/gcc/typed-splay-tree.h
index 9dd96d6..b52fc20 100644
--- a/gcc/typed-splay-tree.h
+++ b/gcc/typed-splay-tree.h
@@ -33,6 +33,7 @@ class typed_splay_tree
typedef int (*compare_fn) (key_type, key_type);
typedef void (*delete_key_fn) (key_type);
typedef void (*delete_value_fn) (value_type);
+ typedef int (*foreach_fn) (key_type, value_type, void *);
typed_splay_tree (compare_fn,
delete_key_fn,
@@ -43,6 +44,9 @@ class typed_splay_tree
value_type predecessor (key_type k);
value_type successor (key_type k);
void insert (key_type k, value_type v);
+ value_type max ();
+ value_type min ();
+ int foreach (foreach_fn, void *);
private:
static value_type node_to_value (splay_tree_node node);
@@ -120,6 +124,69 @@ typed_splay_tree<KEY_TYPE, VALUE_TYPE>::insert (key_type key,
(splay_tree_value)value);
}
+/* Get the value with maximal key. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline VALUE_TYPE
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::max ()
+{
+ return node_to_value (splay_tree_max (m_inner));
+}
+
+/* Get the value with minimal key. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline VALUE_TYPE
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::min ()
+{
+ return node_to_value (splay_tree_min (m_inner));
+}
+
+/* Helper type for typed_splay_tree::foreach. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+struct closure
+{
+ typedef typename typed_splay_tree<KEY_TYPE, VALUE_TYPE>::foreach_fn foreach_fn;
+
+ closure (foreach_fn outer_cb,
+ void *outer_user_data)
+ : m_outer_cb (outer_cb), m_outer_user_data (outer_user_data) {}
+
+ foreach_fn m_outer_cb;
+ void *m_outer_user_data;
+};
+
+/* Helper function for typed_splay_tree::foreach. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+static int
+inner_foreach_fn (splay_tree_node node, void *user_data)
+{
+ closure <KEY_TYPE, VALUE_TYPE> *c
+ = (closure <KEY_TYPE, VALUE_TYPE> *)user_data;
+
+ return c->m_outer_cb ((KEY_TYPE)node->key, (VALUE_TYPE)node->value,
+ c->m_outer_user_data);
+}
+
+/* Call OUTER_CB, passing it the OUTER_USER_DATA, for every node,
+ following an in-order traversal. If OUTER_CB ever returns a non-zero
+ value, the iteration ceases immediately, and the value is returned.
+ Otherwise, this function returns 0. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline int
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::foreach (foreach_fn outer_cb,
+ void *outer_user_data)
+{
+ closure <KEY_TYPE, VALUE_TYPE> c (outer_cb, outer_user_data);
+
+ return splay_tree_foreach (m_inner,
+ inner_foreach_fn<KEY_TYPE, VALUE_TYPE>,
+ &c);
+}
+
/* Internal function for converting from splay_tree_node to
VALUE_TYPE. */
template <typename KEY_TYPE, typename VALUE_TYPE>
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 4/4] Add -fdiagnostics-generate-patch and -fdiagnostics-apply-fixits
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
2016-08-24 0:59 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
2016-08-24 1:00 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
@ 2016-08-24 1:00 ` David Malcolm
2016-08-24 1:00 ` [PATCH 3/4] Introduce class edit_context David Malcolm
` (3 subsequent siblings)
6 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-24 1:00 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch uses the edit_context machinery to provide two new options:
-fdiagnostics-generate-patch
-fdiagnostics-apply-fixits
If either is set, an edit_context is created for global_dc, and any
fix-it hints emitted by diagnostics are added to the edit_context.
-fdiagnostics-generate-patch writes out a patch to stderr in unified
diff format. This is potentially color-coded, following the same rules
as for diagnostics (and affected by -fdiagnostics-color). The color
scheme mimics that of git-diff.
-fdiagnostics-apply-fixits makes the changes to the filesystem, writing
back to the user's source code.
gcc/ChangeLog:
* common.opt (fdiagnostics-apply-fixits): New option.
(fdiagnostics-generate-patch): New option.
* diagnostic.c: Include "edit-context.h".
(diagnostic_initialize): Initialize context->edit_context_ptr.
(diagnostic_finish): Delete context->edit_context_ptr.
(diagnostic_report_diagnostic): Add fix-it hints from the
diagnostic to context->edit_context_ptr, if any.
* diagnostic.h (class edit_context): Add forward decl.
(struct diagnostic_context): Add field "edit_context_ptr".
* doc/invoke.texi (Diagnostic Message Formatting Options): Add
-fdiagnostics-apply-fixits and -fdiagnostics-generate-patch.
(-fdiagnostics-apply-fixits): New item.
(-fdiagnostics-generate-patch): New item.
* toplev.c: Include "edit-context.h".
(process_options): Set global_dc->edit_context_ptr to a new
edit_context if the options need one.
(toplev::main): Handle -fdiagnostics-apply-fixits and
-fdiagnostics-generate-patch by using global_dc->edit_context_ptr.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c: New
test case.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
diagnostic-test-show-locus-generate-patch.c to the sources
for diagnostic_plugin_test_show_locus.c.
---
gcc/common.opt | 8 +++
gcc/diagnostic.c | 11 ++++
gcc/diagnostic.h | 7 ++
gcc/doc/invoke.texi | 28 +++++++-
.../diagnostic-test-show-locus-generate-patch.c | 77 ++++++++++++++++++++++
gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 +-
gcc/toplev.c | 20 ++++++
7 files changed, 152 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
diff --git a/gcc/common.opt b/gcc/common.opt
index 65a9762..3c7f8bb 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1147,6 +1147,10 @@ fdevirtualize
Common Report Var(flag_devirtualize) Optimization
Try to convert virtual calls to direct ones.
+fdiagnostics-apply-fixits
+Common Var(flag_diagnostics_apply_fixits)
+Apply fix-it hints to source code, writing back to input files.
+
fdiagnostics-show-location=
Common Joined RejectNegative Enum(diagnostic_prefixing_rule)
-fdiagnostics-show-location=[once|every-line] How often to emit source location at the beginning of line-wrapped diagnostics.
@@ -1196,6 +1200,10 @@ fdiagnostics-parseable-fixits
Common Var(flag_diagnostics_parseable_fixits)
Print fixit hints in machine-readable form.
+fdiagnostics-generate-patch
+Common Var(flag_diagnostics_generate_patch)
+Print fix-it hints to stderr in unified diff format.
+
fdiagnostics-show-option
Common Var(flag_diagnostics_show_option) Init(1)
Amend appropriate diagnostic messages with the command line option that controls them.
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index b47da38..5a2c565 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see
#include "backtrace.h"
#include "diagnostic.h"
#include "diagnostic-color.h"
+#include "edit-context.h"
#include "selftest.h"
#ifdef HAVE_TERMIOS_H
@@ -174,6 +175,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
context->colorize_source_p = false;
context->show_ruler_p = false;
context->parseable_fixits_p = false;
+ context->edit_context_ptr = NULL;
}
/* Maybe initialize the color support. We require clients to do this
@@ -235,6 +237,12 @@ diagnostic_finish (diagnostic_context *context)
context->printer->~pretty_printer ();
XDELETE (context->printer);
context->printer = NULL;
+
+ if (context->edit_context_ptr)
+ {
+ delete context->edit_context_ptr;
+ context->edit_context_ptr = NULL;
+ }
}
/* Initialize DIAGNOSTIC, where the message MSG has already been
@@ -943,6 +951,9 @@ diagnostic_report_diagnostic (diagnostic_context *context,
diagnostic->message.format_spec = saved_format_spec;
diagnostic->x_data = NULL;
+ if (context->edit_context_ptr)
+ context->edit_context_ptr->add_fixits (diagnostic->richloc);
+
context->lock--;
return true;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 104e39c..5752563 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -62,6 +62,8 @@ typedef void (*diagnostic_start_span_fn) (diagnostic_context *,
typedef diagnostic_starter_fn diagnostic_finalizer_fn;
+class edit_context;
+
/* This data structure bundles altogether any information relevant to
the context of a diagnostic message. */
struct diagnostic_context
@@ -209,6 +211,11 @@ struct diagnostic_context
/* If true, print fixits in machine-parseable form after the
rest of the diagnostic. */
bool parseable_fixits_p;
+
+ /* If non-NULL, an edit_context to which fix-it hints should be
+ applied, for generating patches or writing back changes to
+ source code. */
+ edit_context *edit_context_ptr;
};
static inline void
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1f04501..bb65dae 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -248,7 +248,8 @@ Objective-C and Objective-C++ Dialects}.
-fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]} @gol
-fdiagnostics-color=@r{[}auto@r{|}never@r{|}always@r{]} @gol
-fno-diagnostics-show-option -fno-diagnostics-show-caret @gol
--fdiagnostics-parseable-fixits}
+-fdiagnostics-parseable-fixits @gol
+-fdiagnostics-apply-fixits -fdiagnostics-generate-patch}
@item Warning Options
@xref{Warning Options,,Options to Request or Suppress Warnings}.
@@ -3428,6 +3429,31 @@ An empty replacement string indicates that the given range is to be removed.
An empty range (e.g. ``45:3-45:3'') indicates that the string is to
be inserted at the given position.
+@item -fdiagnostics-apply-fixits
+@opindex fdiagnostics-apply-fixits
+Apply fix-it hints to source code, writing back to input files.
+
+@item -fdiagnostics-generate-patch
+@opindex fdiagnostics-generate-patch
+Print fix-it hints to stderr in unified diff format, after any diagnostics
+are printed. For example:
+
+@smallexample
+--- test.c
++++ test.c
+@@ -42,5 +42,5 @@
+
+ void show_cb(GtkDialog *dlg)
+ @{
+- gtk_widget_showall(dlg);
++ gtk_widget_show_all(dlg);
+ @}
+
+@end smallexample
+
+The diff may or may not be colorized, following the same rules
+as for diagnostics (see @option{-fdiagnostics-color}).
+
@end table
@node Warning Options
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
new file mode 100644
index 0000000..afbaf63
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
@@ -0,0 +1,77 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdiagnostics-generate-patch" } */
+
+/* This is a collection of unittests for diagnostic_show_locus;
+ see the overview in diagnostic_plugin_test_show_locus.c.
+
+ In particular, note the discussion of why we need a very long line here:
+01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
+ and that we can't use macros in this file. */
+
+/* Unit test for rendering of insertion fixit hints
+ (example taken from PR 62316). */
+
+void test_fixit_insert (void)
+{
+#if 0
+ int a[2][2] = { 0, 1 , 2, 3 }; /* { dg-warning "insertion hints" } */
+#endif
+}
+
+/* Unit test for rendering of "remove" fixit hints. */
+
+void test_fixit_remove (void)
+{
+#if 0
+ int a;; /* { dg-warning "example of a removal hint" } */
+#endif
+}
+
+/* Unit test for rendering of "replace" fixit hints. */
+
+void test_fixit_replace (void)
+{
+#if 0
+ gtk_widget_showall (dlg); /* { dg-warning "example of a replacement hint" } */
+#endif
+}
+
+
+
+/* Verify the output from -fdiagnostics-generate-patch.
+ We expect a header, containing the filename. This is the absolute path,
+ so we can only capture it via regexps. */
+
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+
+/* Next, we expect the diff itself. */
+/* { dg-begin-multiline-output "" }
+@@ -14,7 +14,7 @@
+ void test_fixit_insert (void)
+ {
+ #if 0
+- int a[2][2] = { 0, 1 , 2, 3 };
++ int a[2][2] = { {0, 1} , 2, 3 };
+ #endif
+ }
+
+@@ -23,7 +23,7 @@
+ void test_fixit_remove (void)
+ {
+ #if 0
+- int a;;
++ int a;
+ #endif
+ }
+
+@@ -32,7 +32,7 @@
+ void test_fixit_replace (void)
+ {
+ #if 0
+- gtk_widget_showall (dlg);
++ gtk_widget_show_all (dlg);
+ #endif
+ }
+
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 715038a..32ca748 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -65,7 +65,8 @@ set plugin_test_list [list \
{ diagnostic_plugin_test_show_locus.c \
diagnostic-test-show-locus-bw.c \
diagnostic-test-show-locus-color.c \
- diagnostic-test-show-locus-parseable-fixits.c } \
+ diagnostic-test-show-locus-parseable-fixits.c \
+ diagnostic-test-show-locus-generate-patch.c } \
{ diagnostic_plugin_test_tree_expression_range.c \
diagnostic-test-expressions-1.c } \
{ diagnostic_plugin_show_trees.c \
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 2607904..5a613b8 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -76,6 +76,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-chkp.h"
#include "omp-low.h"
#include "hsa.h"
+#include "edit-context.h"
#if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
#include "dbxout.h"
@@ -1221,6 +1222,9 @@ process_options (void)
/* Some machines may reject certain combinations of options. */
targetm.target_option.override ();
+ if (flag_diagnostics_generate_patch || flag_diagnostics_apply_fixits)
+ global_dc->edit_context_ptr = new edit_context ();
+
/* Avoid any informative notes in the second run of -fcompare-debug. */
if (flag_compare_debug)
diagnostic_inhibit_notes (global_dc);
@@ -2147,6 +2151,22 @@ toplev::main (int argc, char **argv)
emit some diagnostics here. */
invoke_plugin_callbacks (PLUGIN_FINISH, NULL);
+ if (flag_diagnostics_generate_patch)
+ {
+ gcc_assert (global_dc->edit_context_ptr);
+
+ pretty_printer (pp);
+ pp_show_color (&pp) = pp_show_color (global_dc->printer);
+ global_dc->edit_context_ptr->print_diff (&pp, true);
+ pp_flush (&pp);
+ }
+
+ if (flag_diagnostics_apply_fixits)
+ {
+ gcc_assert (global_dc->edit_context_ptr);
+ global_dc->edit_context_ptr->apply_changes ();
+ }
+
diagnostic_finish (global_dc);
finalize_plugins ();
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
` (3 preceding siblings ...)
2016-08-24 1:00 ` [PATCH 3/4] Introduce class edit_context David Malcolm
@ 2016-08-24 1:58 ` Eric Gallager
2016-08-24 13:15 ` David Malcolm
2016-08-24 7:59 ` Richard Biener
2016-08-24 13:45 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Bernd Schmidt
6 siblings, 1 reply; 31+ messages in thread
From: Eric Gallager @ 2016-08-24 1:58 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On 8/23/16, David Malcolm <dmalcolm@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.
>
> 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...
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
` (4 preceding siblings ...)
2016-08-24 1:58 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Eric Gallager
@ 2016-08-24 7:59 ` Richard Biener
2016-08-24 13:29 ` Trevor Saunders
` (2 more replies)
2016-08-24 13:45 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Bernd Schmidt
6 siblings, 3 replies; 31+ messages in thread
From: Richard Biener @ 2016-08-24 7:59 UTC (permalink / raw)
To: David Malcolm; +Cc: GCC Patches
On Wed, Aug 24, 2016 at 3:28 AM, David Malcolm <dmalcolm@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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 1:58 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Eric Gallager
@ 2016-08-24 13:15 ` David Malcolm
0 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-24 13:15 UTC (permalink / raw)
To: Eric Gallager; +Cc: gcc-patches
On Tue, 2016-08-23 at 21:57 -0400, Eric Gallager wrote:
> On 8/23/16, David Malcolm <dmalcolm@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.
> >
> > 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 7:59 ` Richard Biener
@ 2016-08-24 13:29 ` Trevor Saunders
2016-08-24 13:32 ` David Malcolm
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
2 siblings, 0 replies; 31+ messages in thread
From: Trevor Saunders @ 2016-08-24 13:29 UTC (permalink / raw)
To: Richard Biener; +Cc: David Malcolm, GCC Patches
On Wed, Aug 24, 2016 at 09:59:18AM +0200, Richard Biener wrote:
> On Wed, Aug 24, 2016 at 3:28 AM, David Malcolm <dmalcolm@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 ;)
Obviously your work flow needs to deal with things other than your
editor changing files for this to be useful, but it can only cause
trouble for you if you ask for it. I don't think it should be the
default, and I'm not sure if all fix it hints are good candidates.
However it does make my life better if I don't need to do busy work
like insert |typename| to make the compiler happy with my template
usage.
I think the even better use of this is for plugins that want to rewrite
the source code for a project to change some API's consumers in a
mechanical way. Currently you usually get to do that by manually
applying the same fix whever the compiler tells you there is an error,
and in some cases that really should be automatable. I believe
Libreoffice has used clang plugins to this to good effect, but I don't
know any details.
> 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.
That might be useful too.
Trev
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 7:59 ` Richard Biener
2016-08-24 13:29 ` Trevor Saunders
@ 2016-08-24 13:32 ` David Malcolm
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
2 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-24 13:32 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Wed, 2016-08-24 at 09:59 +0200, Richard Biener wrote:
> On Wed, Aug 24, 2016 at 3:28 AM, David Malcolm <dmalcolm@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 ;)
Yeah, the current implementation of (B) has issues with atomicity; I
wonder what would happen in a parallel build if a fix-it happened
affecting a header file...
Would you be open to a less ambitious version of the patch kit which
just implemented (A)?
> 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.
One approach here would be to provide a small binary that knows how to
parse the output of -fdiagnostics-parseable-fixits, and applies them,
something like ${bindir}/gcc-apply-fixits
It would read a stream of text on stdin (taken from stderr of cc1 and
friends), parse any lines emitted by -fdiagnostics-parseable-fixits
(ignoring the rest), put them all into an edit_context, and then apply
them (or generate a patch).
If nothing else, this might be useful for people integrating fix-its
into an IDE.
I was thinking that the driver could then have an option to attempt the
compile, and have it send stderr through the binary, and somehow run
the compiler twice. But presumably we'd want to have the 2nd
invocation of the compiler be run on temporarily-modified sources; what
happens if we need to issue a diagnostic on such sources? In
particular what filename does the user see? It seems that we'd need
some way of mapping filenames, so that the compiler and the user are
effectively seeing an "as-if" view of the filesystem (as if the changes
were applied).
Alternatively, maybe this is too implementation-focused: we should
start by thinking of the interaction model - what do we want the user's
experience to be?
I wonder if there's a way for a fix-it to actually change the token
stream seen by the compiler (doing it all in one invocation of the
compiler).
Dave
> > 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
> >
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
` (5 preceding siblings ...)
2016-08-24 7:59 ` Richard Biener
@ 2016-08-24 13:45 ` Bernd Schmidt
2016-08-24 13:56 ` Richard Biener
6 siblings, 1 reply; 31+ messages in thread
From: Bernd Schmidt @ 2016-08-24 13:45 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 08/24/2016 03:28 AM, David Malcolm wrote:
> (B) -fdiagnostics-apply-fixits, which writes back the changes
> to the input files.
That sounds really scary, there's no way I'd personally ever enable
something like this. Do we have any data about what percentage of fixits
actually correctly identify the problem?
We also better make really sure there's no way we'll get bug reports
along the lines of "gcc ate my program and now it's gone".
Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 13:45 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Bernd Schmidt
@ 2016-08-24 13:56 ` Richard Biener
2016-08-24 21:52 ` Manuel López-Ibáñez
0 siblings, 1 reply; 31+ messages in thread
From: Richard Biener @ 2016-08-24 13:56 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: David Malcolm, GCC Patches
On Wed, Aug 24, 2016 at 3:45 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 08/24/2016 03:28 AM, David Malcolm wrote:
>
>> (B) -fdiagnostics-apply-fixits, which writes back the changes
>> to the input files.
>
>
> That sounds really scary, there's no way I'd personally ever enable
> something like this. Do we have any data about what percentage of fixits
> actually correctly identify the problem?
>
> We also better make really sure there's no way we'll get bug reports along
> the lines of "gcc ate my program and now it's gone".
You never typoed
gcc t.c -o t.c
? ;) (I did ... :/)
But yes, exactly my feelings towards (B).
IMHO (B) is something for tight IDE - compiler integration, sth GCC is not very
good at at the moment. [hint: provide the patches in a form so emacs
can eat them]
Richard.
>
> Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 13:56 ` Richard Biener
@ 2016-08-24 21:52 ` Manuel López-Ibáñez
2016-08-25 9:20 ` Richard Biener
0 siblings, 1 reply; 31+ messages in thread
From: Manuel López-Ibáñez @ 2016-08-24 21:52 UTC (permalink / raw)
To: Richard Biener, Bernd Schmidt; +Cc: David Malcolm, GCC Patches
On 24/08/16 14:56, Richard Biener wrote:
> You never typoed
>
> gcc t.c -o t.c
>
> ? ;) (I did ... :/)
With GCC >=5
$ gcc t.c -o t.c
gcc: fatal error: input file ‘t.c’ is the same as output file
compilation terminated.
You are welcome ;-)
Manuel.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/4] selftest: split out named_temp_file from temp_source_file
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
@ 2016-08-25 0:44 ` David Malcolm
2016-08-29 21:53 ` Bernd Schmidt
2016-08-25 0:45 ` [PATCH 3/4] (v2) Introduce class edit_context David Malcolm
` (3 subsequent siblings)
4 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-08-25 0:44 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, David Malcolm
Split out a new base class for temp_source_file, named_temp_file,
moving the deletion to the base class dtor, so that we can write
out temporary files in other ways in selftests.
gcc/ChangeLog:
* selftest.c (selftest::named_temp_file::named_temp_file): New
ctor.
(selftest::temp_source_file::~temp_source_file): Move to...
(selftest::named_temp_file::~named_temp_file): ...here.
(selftest::test_named_temp_file): New function.
(selftest::selftest_c_tests): Call test_named_temp_file.
* selftest.h (class named_temp_file): New class.
(class temp_source_file): Convert to a subclass of named_temp_file.
---
gcc/selftest.c | 49 +++++++++++++++++++++++++++++++++++--------------
gcc/selftest.h | 24 +++++++++++++++++-------
2 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/gcc/selftest.c b/gcc/selftest.c
index 629db98..e6c9510 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -120,34 +120,40 @@ selftest::assert_str_contains (const location &loc,
desc_haystack, desc_needle, val_haystack, val_needle);
}
-/* Constructor. Create a tempfile using SUFFIX, and write CONTENT to
- it. Abort if anything goes wrong, using LOC as the effective
- location in the problem report. */
+/* Constructor. Generate a name for the file. */
-selftest::temp_source_file::temp_source_file (const location &loc,
- const char *suffix,
- const char *content)
+selftest::named_temp_file::named_temp_file (const char *suffix)
{
m_filename = make_temp_file (suffix);
ASSERT_NE (m_filename, NULL);
-
- FILE *out = fopen (m_filename, "w");
- if (!out)
- ::selftest::fail_formatted (loc, "unable to open tempfile: %s",
- m_filename);
- fprintf (out, "%s", content);
- fclose (out);
}
/* Destructor. Delete the tempfile. */
-selftest::temp_source_file::~temp_source_file ()
+selftest::named_temp_file::~named_temp_file ()
{
unlink (m_filename);
diagnostics_file_cache_forcibly_evict_file (m_filename);
free (m_filename);
}
+/* Constructor. Create a tempfile using SUFFIX, and write CONTENT to
+ it. Abort if anything goes wrong, using LOC as the effective
+ location in the problem report. */
+
+selftest::temp_source_file::temp_source_file (const location &loc,
+ const char *suffix,
+ const char *content)
+: named_temp_file (suffix)
+{
+ FILE *out = fopen (get_filename (), "w");
+ if (!out)
+ ::selftest::fail_formatted (loc, "unable to open tempfile: %s",
+ get_filename ());
+ fprintf (out, "%s", content);
+ fclose (out);
+}
+
/* Selftests for the selftest system itself. */
namespace selftest {
@@ -167,12 +173,27 @@ test_assertions ()
ASSERT_STR_CONTAINS ("foo bar baz", "bar");
}
+/* Verify named_temp_file. */
+
+static void
+test_named_temp_file ()
+{
+ named_temp_file t (".txt");
+ FILE *f = fopen (t.get_filename (), "w");
+ if (!f)
+ selftest::fail_formatted (SELFTEST_LOCATION,
+ "unable to open %s for writing",
+ t.get_filename ());
+ fclose (f);
+}
+
/* Run all of the selftests within this file. */
void
selftest_c_tests ()
{
test_assertions ();
+ test_named_temp_file ();
}
} // namespace selftest
diff --git a/gcc/selftest.h b/gcc/selftest.h
index b073ed6..fd3c6b0 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -77,22 +77,32 @@ extern void assert_str_contains (const location &loc,
const char *val_haystack,
const char *val_needle);
-/* A class for writing out a temporary sourcefile for use in selftests
- of input handling. */
+/* A named temporary file for use in selftests.
+ Usable for writing out files, and as the base class for
+ temp_source_file.
+ The file is unlinked in the destructor. */
-class temp_source_file
+class named_temp_file
{
public:
- temp_source_file (const location &loc, const char *suffix,
- const char *content);
- ~temp_source_file ();
-
+ named_temp_file (const char *suffix);
+ ~named_temp_file ();
const char *get_filename () const { return m_filename; }
private:
char *m_filename;
};
+/* A class for writing out a temporary sourcefile for use in selftests
+ of input handling. */
+
+class temp_source_file : public named_temp_file
+{
+ public:
+ temp_source_file (const location &loc, const char *suffix,
+ const char *content);
+};
+
/* Various selftests involving location-handling require constructing a
line table and one or more line maps within it.
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
` (2 preceding siblings ...)
2016-08-25 0:45 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
@ 2016-08-25 0:45 ` David Malcolm
2016-09-02 19:53 ` David Malcolm
2016-09-09 23:01 ` [PATCH 0/4] (v2) Generating patches from fix-it hints Jeff Law
4 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-08-25 0:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, David Malcolm
Changed in v2: I dropped -fdiagnostics-apply-fixits
This patch uses the edit_context machinery to provide a new
-fdiagnostics-generate-patch option.
If set an edit_context is created for global_dc, and any
fix-it hints emitted by diagnostics are added to the edit_context.
-fdiagnostics-generate-patch writes out a patch to stderr in unified
diff format. This is potentially color-coded, following the same rules
as for diagnostics (and affected by -fdiagnostics-color). The color
scheme mimics that of git-diff.
gcc/ChangeLog:
* common.opt (fdiagnostics-generate-patch): New option.
* diagnostic.c: Include "edit-context.h".
(diagnostic_initialize): Initialize context->edit_context_ptr.
(diagnostic_finish): Delete context->edit_context_ptr.
(diagnostic_report_diagnostic): Add fix-it hints from the
diagnostic to context->edit_context_ptr, if any.
* diagnostic.h (class edit_context): Add forward decl.
(struct diagnostic_context): Add field "edit_context_ptr".
* doc/invoke.texi (Diagnostic Message Formatting Options): Add
-fdiagnostics-generate-patch.
(-fdiagnostics-generate-patch): New item.
* toplev.c: Include "edit-context.h".
(process_options): Set global_dc->edit_context_ptr to a new
edit_context if the options need one.
(toplev::main): Handle -fdiagnostics-generate-patch by using
global_dc->edit_context_ptr.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c: New
test case.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
diagnostic-test-show-locus-generate-patch.c to the sources
for diagnostic_plugin_test_show_locus.c.
---
gcc/common.opt | 4 ++
gcc/diagnostic.c | 11 ++++
gcc/diagnostic.h | 6 ++
gcc/doc/invoke.texi | 23 ++++++-
.../diagnostic-test-show-locus-generate-patch.c | 77 ++++++++++++++++++++++
gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 +-
gcc/toplev.c | 14 ++++
7 files changed, 136 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
diff --git a/gcc/common.opt b/gcc/common.opt
index 65a9762..0c40d0a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1196,6 +1196,10 @@ fdiagnostics-parseable-fixits
Common Var(flag_diagnostics_parseable_fixits)
Print fixit hints in machine-readable form.
+fdiagnostics-generate-patch
+Common Var(flag_diagnostics_generate_patch)
+Print fix-it hints to stderr in unified diff format.
+
fdiagnostics-show-option
Common Var(flag_diagnostics_show_option) Init(1)
Amend appropriate diagnostic messages with the command line option that controls them.
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index b47da38..5a2c565 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see
#include "backtrace.h"
#include "diagnostic.h"
#include "diagnostic-color.h"
+#include "edit-context.h"
#include "selftest.h"
#ifdef HAVE_TERMIOS_H
@@ -174,6 +175,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
context->colorize_source_p = false;
context->show_ruler_p = false;
context->parseable_fixits_p = false;
+ context->edit_context_ptr = NULL;
}
/* Maybe initialize the color support. We require clients to do this
@@ -235,6 +237,12 @@ diagnostic_finish (diagnostic_context *context)
context->printer->~pretty_printer ();
XDELETE (context->printer);
context->printer = NULL;
+
+ if (context->edit_context_ptr)
+ {
+ delete context->edit_context_ptr;
+ context->edit_context_ptr = NULL;
+ }
}
/* Initialize DIAGNOSTIC, where the message MSG has already been
@@ -943,6 +951,9 @@ diagnostic_report_diagnostic (diagnostic_context *context,
diagnostic->message.format_spec = saved_format_spec;
diagnostic->x_data = NULL;
+ if (context->edit_context_ptr)
+ context->edit_context_ptr->add_fixits (diagnostic->richloc);
+
context->lock--;
return true;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 104e39c..27e6007 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -62,6 +62,8 @@ typedef void (*diagnostic_start_span_fn) (diagnostic_context *,
typedef diagnostic_starter_fn diagnostic_finalizer_fn;
+class edit_context;
+
/* This data structure bundles altogether any information relevant to
the context of a diagnostic message. */
struct diagnostic_context
@@ -209,6 +211,10 @@ struct diagnostic_context
/* If true, print fixits in machine-parseable form after the
rest of the diagnostic. */
bool parseable_fixits_p;
+
+ /* If non-NULL, an edit_context to which fix-it hints should be
+ applied, for generating patches. */
+ edit_context *edit_context_ptr;
};
static inline void
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1f04501..db2f400 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -248,7 +248,7 @@ Objective-C and Objective-C++ Dialects}.
-fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]} @gol
-fdiagnostics-color=@r{[}auto@r{|}never@r{|}always@r{]} @gol
-fno-diagnostics-show-option -fno-diagnostics-show-caret @gol
--fdiagnostics-parseable-fixits}
+-fdiagnostics-parseable-fixits -fdiagnostics-generate-patch}
@item Warning Options
@xref{Warning Options,,Options to Request or Suppress Warnings}.
@@ -3428,6 +3428,27 @@ An empty replacement string indicates that the given range is to be removed.
An empty range (e.g. ``45:3-45:3'') indicates that the string is to
be inserted at the given position.
+@item -fdiagnostics-generate-patch
+@opindex fdiagnostics-generate-patch
+Print fix-it hints to stderr in unified diff format, after any diagnostics
+are printed. For example:
+
+@smallexample
+--- test.c
++++ test.c
+@@ -42,5 +42,5 @@
+
+ void show_cb(GtkDialog *dlg)
+ @{
+- gtk_widget_showall(dlg);
++ gtk_widget_show_all(dlg);
+ @}
+
+@end smallexample
+
+The diff may or may not be colorized, following the same rules
+as for diagnostics (see @option{-fdiagnostics-color}).
+
@end table
@node Warning Options
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
new file mode 100644
index 0000000..afbaf63
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c
@@ -0,0 +1,77 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdiagnostics-generate-patch" } */
+
+/* This is a collection of unittests for diagnostic_show_locus;
+ see the overview in diagnostic_plugin_test_show_locus.c.
+
+ In particular, note the discussion of why we need a very long line here:
+01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
+ and that we can't use macros in this file. */
+
+/* Unit test for rendering of insertion fixit hints
+ (example taken from PR 62316). */
+
+void test_fixit_insert (void)
+{
+#if 0
+ int a[2][2] = { 0, 1 , 2, 3 }; /* { dg-warning "insertion hints" } */
+#endif
+}
+
+/* Unit test for rendering of "remove" fixit hints. */
+
+void test_fixit_remove (void)
+{
+#if 0
+ int a;; /* { dg-warning "example of a removal hint" } */
+#endif
+}
+
+/* Unit test for rendering of "replace" fixit hints. */
+
+void test_fixit_replace (void)
+{
+#if 0
+ gtk_widget_showall (dlg); /* { dg-warning "example of a replacement hint" } */
+#endif
+}
+
+
+
+/* Verify the output from -fdiagnostics-generate-patch.
+ We expect a header, containing the filename. This is the absolute path,
+ so we can only capture it via regexps. */
+
+/* { dg-regexp "\\-\\-\\- .*" } */
+/* { dg-regexp "\\+\\+\\+ .*" } */
+
+/* Next, we expect the diff itself. */
+/* { dg-begin-multiline-output "" }
+@@ -14,7 +14,7 @@
+ void test_fixit_insert (void)
+ {
+ #if 0
+- int a[2][2] = { 0, 1 , 2, 3 };
++ int a[2][2] = { {0, 1} , 2, 3 };
+ #endif
+ }
+
+@@ -23,7 +23,7 @@
+ void test_fixit_remove (void)
+ {
+ #if 0
+- int a;;
++ int a;
+ #endif
+ }
+
+@@ -32,7 +32,7 @@
+ void test_fixit_replace (void)
+ {
+ #if 0
+- gtk_widget_showall (dlg);
++ gtk_widget_show_all (dlg);
+ #endif
+ }
+
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 715038a..32ca748 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -65,7 +65,8 @@ set plugin_test_list [list \
{ diagnostic_plugin_test_show_locus.c \
diagnostic-test-show-locus-bw.c \
diagnostic-test-show-locus-color.c \
- diagnostic-test-show-locus-parseable-fixits.c } \
+ diagnostic-test-show-locus-parseable-fixits.c \
+ diagnostic-test-show-locus-generate-patch.c } \
{ diagnostic_plugin_test_tree_expression_range.c \
diagnostic-test-expressions-1.c } \
{ diagnostic_plugin_show_trees.c \
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 2607904..4da5627 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -76,6 +76,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-chkp.h"
#include "omp-low.h"
#include "hsa.h"
+#include "edit-context.h"
#if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
#include "dbxout.h"
@@ -1221,6 +1222,9 @@ process_options (void)
/* Some machines may reject certain combinations of options. */
targetm.target_option.override ();
+ if (flag_diagnostics_generate_patch)
+ global_dc->edit_context_ptr = new edit_context ();
+
/* Avoid any informative notes in the second run of -fcompare-debug. */
if (flag_compare_debug)
diagnostic_inhibit_notes (global_dc);
@@ -2147,6 +2151,16 @@ toplev::main (int argc, char **argv)
emit some diagnostics here. */
invoke_plugin_callbacks (PLUGIN_FINISH, NULL);
+ if (flag_diagnostics_generate_patch)
+ {
+ gcc_assert (global_dc->edit_context_ptr);
+
+ pretty_printer (pp);
+ pp_show_color (&pp) = pp_show_color (global_dc->printer);
+ global_dc->edit_context_ptr->print_diff (&pp, true);
+ pp_flush (&pp);
+ }
+
diagnostic_finish (global_dc);
finalize_plugins ();
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] (v2) Introduce class edit_context
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
2016-08-25 0:44 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
@ 2016-08-25 0:45 ` David Malcolm
2016-08-28 15:05 ` Trevor Saunders
2016-08-25 0:45 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
` (2 subsequent siblings)
4 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-08-25 0:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, David Malcolm
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 ();
+ 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;
+};
+
+/* 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;
+ }
+
+ private:
+ int m_start;
+ int m_finish;
+ int m_delta;
+};
+
+/* Implementation of class edit_context. */
+
+/* edit_context's ctor. */
+
+edit_context::edit_context ()
+: m_valid (true),
+ m_files (strcmp, NULL, edited_file::delete_cb)
+{}
+
+/* Add any fixits within RICHLOC to this context, recording the
+ changes that they make. */
+
+void
+edit_context::add_fixits (rich_location *richloc)
+{
+ if (!m_valid)
+ return;
+ for (unsigned i = 0; i < richloc->get_num_fixit_hints (); i++)
+ {
+ const fixit_hint *hint = richloc->get_fixit_hint (i);
+ switch (hint->get_kind ())
+ {
+ case fixit_hint::INSERT:
+ if (!apply_insert ((const fixit_insert *)hint))
+ {
+ /* Failure. */
+ m_valid = false;
+ return;
+ }
+ break;
+ case fixit_hint::REPLACE:
+ if (!apply_replace ((const fixit_replace *)hint))
+ {
+ /* Failure. */
+ m_valid = false;
+ return;
+ }
+ break;
+ default:
+ gcc_unreachable ();
+ }
+ }
+}
+
+/* Get the content of the given file, with fix-its applied.
+ If any errors occurred in this edit_context, return NULL. */
+
+const char *
+edit_context::get_content (const char *filename)
+{
+ if (!m_valid)
+ return NULL;
+ edited_file &file = get_or_insert_file (filename);
+ return file.get_content ();
+}
+
+/* Map a location before the edits to a column number after the edits.
+ This method is for the selftests. */
+
+int
+edit_context::get_effective_column (const char *filename, int line,
+ int column)
+{
+ edited_file *file = get_file (filename);
+ if (!file)
+ return column;
+ return file->get_effective_column (line, column);
+}
+
+/* Generate a unified diff. The resulting string should be freed by the
+ caller. Primarily for selftests.
+ If any errors occurred in this edit_context, return NULL. */
+
+char *
+edit_context::generate_diff (bool show_filenames)
+{
+ if (!m_valid)
+ return NULL;
+
+ pretty_printer pp;
+ print_diff (&pp, show_filenames);
+ return xstrdup (pp_formatted_text (&pp));
+}
+
+/* Print a unified diff to PP, showing the changes made within the
+ context. */
+
+void
+edit_context::print_diff (pretty_printer *pp, bool show_filenames)
+{
+ if (!m_valid)
+ return;
+ diff d (pp, show_filenames);
+ m_files.foreach (edited_file::call_print_diff, &d);
+}
+
+/* Attempt to apply the given fixit. Return true if it can be
+ applied, or false otherwise. */
+
+bool
+edit_context::apply_insert (const fixit_insert *insert)
+{
+ expanded_location exploc = expand_location (insert->get_location ());
+
+ if (exploc.column == 0)
+ return false;
+
+ edited_file &file = get_or_insert_file (exploc.file);
+ if (!m_valid)
+ return false;
+ return file.apply_insert (exploc.line, exploc.column, insert->get_string (),
+ insert->get_length ());
+}
+
+/* Attempt to apply the given fixit. Return true if it can be
+ applied, or false otherwise. */
+
+bool
+edit_context::apply_replace (const fixit_replace *replace)
+{
+ source_range range = replace->get_range ();
+
+ expanded_location start = expand_location (range.m_start);
+ expanded_location finish = expand_location (range.m_finish);
+ if (start.file != finish.file)
+ return false;
+ if (start.line != finish.line)
+ return false;
+ if (start.column == 0)
+ return false;
+ if (finish.column == 0)
+ return false;
+
+ edited_file &file = get_or_insert_file (start.file);
+ if (!m_valid)
+ return false;
+ return file.apply_replace (start.line, start.column, finish.column,
+ replace->get_string (),
+ replace->get_length ());
+}
+
+/* Locate the edited_file * for FILENAME, if any
+ Return NULL if there isn't one. */
+
+edited_file *
+edit_context::get_file (const char *filename)
+{
+ gcc_assert (filename);
+ return m_files.lookup (filename);
+}
+
+/* Locate the edited_file for FILENAME, adding one if there isn't one. */
+
+edited_file &
+edit_context::get_or_insert_file (const char *filename)
+{
+ gcc_assert (filename);
+
+ edited_file *file = get_file (filename);
+ if (file)
+ return *file;
+
+ /* Not found. */
+ file = new edited_file (filename);
+ if (!file->read_from_file ())
+ m_valid = false;
+ m_files.insert (filename, file);
+ return *file;
+}
+
+/* Implementation of class edited_file. */
+
+/* Callback for m_edited_lines, for comparing line numbers. */
+
+static int line_comparator (int a, int b)
+{
+ return a - b;
+}
+
+/* edited_file's constructor. */
+
+edited_file::edited_file (const char *filename)
+: m_filename (filename),
+ m_content (NULL), m_len (0), m_alloc_sz (0),
+ m_edited_lines (line_comparator, NULL, line_state::delete_cb)
+{
+}
+
+/* Read the contents of the file from the filesystem into memory.
+ Return true if successful; false if any problems occurred. */
+
+bool
+edited_file::read_from_file ()
+{
+ FILE *f_in = fopen (m_filename, "r");
+ if (!f_in)
+ return false;
+
+ char buf[4096];
+ size_t iter_sz_in;
+
+ while ( (iter_sz_in = fread (buf, 1, sizeof (buf), f_in)) )
+ {
+ gcc_assert (m_alloc_sz >= m_len);
+ size_t old_len = m_len;
+ m_len += iter_sz_in;
+ ensure_capacity (m_len);
+ memcpy (m_content + old_len, buf, iter_sz_in);
+ }
+
+ if (!feof (f_in))
+ return false;
+
+ fclose (f_in);
+
+ ensure_terminated ();
+
+ return true;
+}
+
+/* edited_file's dtor. */
+
+edited_file::~edited_file ()
+{
+ free (m_content);
+}
+
+/* A callback for deleting edited_file *, for use as a
+ delete_value_fn for edit_context::m_files. */
+
+void
+edited_file::delete_cb (edited_file *file)
+{
+ delete file;
+}
+
+/* Insert the string INSERT_STR with length INSERT_LEN at LINE and COLUMN,
+ updating the in-memory copy of the file, and the record of edits to
+ the line. */
+
+bool
+edited_file::apply_insert (int line, int column,
+ const char *insert_str,
+ int insert_len)
+{
+ column = get_effective_column (line, column);
+
+ int start_offset = column - 1;
+ gcc_assert (start_offset >= 0);
+
+ /* Ensure buffer is big enough. */
+ size_t new_len = m_len + insert_len;
+ ensure_capacity (new_len);
+
+ char *line_start = get_line_start (line);
+ if (!line_start)
+ return false;
+
+ char *suffix = line_start + start_offset;
+ size_t len_suffix = (m_content + m_len) - suffix;
+
+ /* Move successor content into position. They overlap, so use memmove. */
+ memmove (line_start + start_offset + insert_len,
+ suffix, len_suffix);
+
+ /* Replace target content. They don't overlap, so use memcpy. */
+ memcpy (line_start + start_offset,
+ insert_str,
+ insert_len);
+
+ m_len = new_len;
+
+ ensure_terminated ();
+ evict_line_start_index ();
+
+ /* Record the insertion, so that followup changes to this line
+ can have their columns adjusted as appropriate. */
+ line_state &ls = get_or_insert_line (line);
+ ls.apply_insert (column, insert_len);
+
+ return true;
+}
+
+/* Replace columns START_COLUMN through FINISH_COLUMN of LINE
+ with the string REPLACEMENT_STR of length REPLACEMENT_LEN,
+ updating the in-memory copy of the file, and the record of edits to
+ the line. */
+
+bool
+edited_file::apply_replace (int line, int start_column,
+ int finish_column,
+ const char *replacement_str,
+ int replacement_len)
+{
+ start_column = get_effective_column (line, start_column);
+ finish_column = get_effective_column (line, finish_column);
+
+ int start_offset = start_column - 1;
+ int end_offset = finish_column - 1;
+
+ gcc_assert (start_offset >= 0);
+ gcc_assert (end_offset >= 0);
+
+ gcc_assert (start_column <= finish_column);
+
+ size_t victim_len = end_offset - start_offset + 1;
+
+ /* Ensure buffer is big enough. */
+ size_t new_len = m_len + replacement_len - victim_len;
+ ensure_capacity (new_len);
+
+ char *line_start = get_line_start (line);
+ if (!line_start)
+ return false;
+
+ char *suffix = line_start + end_offset + 1;
+ size_t len_suffix = (m_content + m_len) - suffix;
+
+ /* Move successor content into position. They overlap, so use memmove. */
+ memmove (line_start + start_offset + replacement_len,
+ suffix, len_suffix);
+
+ /* Replace target content. They don't overlap, so use memcpy. */
+ memcpy (line_start + start_offset,
+ replacement_str,
+ replacement_len);
+
+ m_len = new_len;
+
+ ensure_terminated ();
+ evict_line_start_index ();
+
+ /* Record the replacement, so that followup changes to this line
+ can have their columns adjusted as appropriate. */
+ line_state &ls = get_or_insert_line (line);
+ ls.apply_replace (start_column, finish_column, replacement_len);
+
+ return true;
+}
+
+/* Given line LINE, map from COLUMN in the input file to its current
+ column after edits have been applied. */
+
+int
+edited_file::get_effective_column (int line, int column)
+{
+ const line_state *ls = get_line (line);
+ if (!ls)
+ return column;
+ return ls->get_effective_column (column);
+}
+
+/* Print a unified diff to PP, showing any changes that have occurred
+ to this file. */
+
+void
+edited_file::print_diff (pretty_printer *pp, bool show_filenames)
+{
+ if (show_filenames)
+ {
+ pp_string (pp, colorize_start (pp_show_color (pp), "diff-filename"));
+ pp_printf (pp, "--- %s\n", m_filename);
+ pp_printf (pp, "+++ %s\n", m_filename);
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+ }
+
+ line_state *ls = m_edited_lines.min ();
+
+ const int context_lines = 3;
+
+ while (ls)
+ {
+ int start_of_hunk = ls->get_line_num ();
+ start_of_hunk -= context_lines;
+ if (start_of_hunk < 1)
+ start_of_hunk = 1;
+
+ /* Locate end of hunk, merging in changed lines
+ that are sufficiently close. */
+ while (true)
+ {
+ line_state *next_ls
+ = m_edited_lines.successor (ls->get_line_num ());
+ if (!next_ls)
+ break;
+ if (ls->get_line_num () + context_lines
+ >= next_ls->get_line_num () - context_lines)
+ ls = next_ls;
+ else
+ break;
+ }
+ int end_of_hunk = ls->get_line_num ();
+ end_of_hunk += context_lines;
+ if (end_of_hunk > get_num_lines ())
+ end_of_hunk = get_num_lines ();
+
+ int num_lines = end_of_hunk - start_of_hunk + 1;
+
+ pp_string (pp, colorize_start (pp_show_color (pp), "diff-hunk"));
+ pp_printf (pp, "@@ -%i,%i +%i,%i @@\n", start_of_hunk, num_lines,
+ start_of_hunk, num_lines);
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+ int line_num = start_of_hunk;
+ while (line_num <= end_of_hunk)
+ {
+ line_state *ls = get_line (line_num);
+ if (ls)
+ {
+ /* We have an edited line.
+ Consolidate into runs of changed lines. */
+ const int first_changed_line_in_run = line_num;
+ while (get_line (line_num))
+ line_num++;
+ const int last_changed_line_in_run = line_num - 1;
+
+ pp_string (pp, colorize_start (pp_show_color (pp),
+ "diff-delete"));
+
+ /* Show old version of lines. */
+ for (line_num = first_changed_line_in_run;
+ line_num <= last_changed_line_in_run;
+ line_num++)
+ {
+ int line_size;
+ const char *old_line
+ = location_get_source_line (m_filename, line_num,
+ &line_size);
+ pp_character (pp, '-');
+ for (int i = 0; i < line_size; i++)
+ pp_character (pp, old_line[i]);
+ pp_character (pp, '\n');
+ }
+
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+ pp_string (pp, colorize_start (pp_show_color (pp),
+ "diff-insert"));
+
+ /* Show new version of lines. */
+ for (line_num = first_changed_line_in_run;
+ line_num <= last_changed_line_in_run;
+ line_num++)
+ {
+ pp_character (pp, '+');
+ print_line_in_diff (pp, line_num);
+ }
+
+ pp_string (pp, colorize_stop (pp_show_color (pp)));
+
+ }
+ else
+ {
+ /* Unchanged line. */
+ pp_character (pp, ' ');
+ print_line_in_diff (pp, line_num);
+ line_num++;
+ }
+ }
+
+ ls = m_edited_lines.successor (ls->get_line_num ());
+ }
+}
+
+/* Subroutine of edited_file::print_diff. Print the given line
+ (after edits) to PP (with a trailing newline). */
+
+void
+edited_file::print_line_in_diff (pretty_printer *pp, int line_num)
+{
+ const char *line = get_line_start (line_num);
+ gcc_assert (line);
+ while (char ch = *line++)
+ {
+ pp_character (pp, ch);
+ if (ch == '\n')
+ break;
+ }
+}
+
+/* Get the state of LINE within the file, or NULL if it is untouched. */
+
+line_state *
+edited_file::get_line (int line)
+{
+ return m_edited_lines.lookup (line);
+}
+
+/* Get the state of LINE within the file, creating a state for it
+ if necessary. */
+
+line_state &
+edited_file::get_or_insert_line (int line)
+{
+ line_state *ls = get_line (line);
+ if (ls)
+ return *ls;
+ ls = new line_state (line);
+ m_edited_lines.insert (line, ls);
+ return *ls;
+}
+
+/* Ensure that the buffer for m_content is at least large enough to hold
+ a string of length LEN and its 0-terminator, doubling on repeated
+ allocations. */
+
+void
+edited_file::ensure_capacity (size_t len)
+{
+ /* Allow 1 extra byte for 0-termination. */
+ if (m_alloc_sz < (len + 1))
+ {
+ size_t new_alloc_sz = (len + 1) * 2;
+ m_content = (char *)xrealloc (m_content, new_alloc_sz);
+ m_alloc_sz = new_alloc_sz;
+ }
+}
+
+/* Ensure that m_content is 0-terminated. */
+
+void
+edited_file::ensure_terminated ()
+{
+ /* 0-terminate the buffer. */
+ gcc_assert (m_len < m_alloc_sz);
+ m_content[m_len] = '\0';
+}
+
+/* Return a pointer to the start of the given line within the
+ m_content buffer, or NULL if line_num is out of range. */
+
+char *
+edited_file::get_line_start (int line_num)
+{
+ if (line_num < 1)
+ return NULL;
+ ensure_line_start_index ();
+ if (line_num > (int)m_line_start_index.length ())
+ return NULL;
+ return &m_content[m_line_start_index[line_num - 1]];
+}
+
+/* If necessary, regenerate the index of line-start offsets
+ based on the current state of m_content. */
+
+void
+edited_file::ensure_line_start_index ()
+{
+ if (m_line_start_index.length () == 0)
+ {
+ int offset = 0;
+ m_line_start_index.safe_push (0);
+ while (char ch = m_content[offset++])
+ {
+ if (ch == '\n')
+ if (m_content[offset])
+ m_line_start_index.safe_push (offset);
+ if (ch == '\0')
+ break;
+ }
+ }
+}
+
+/* Evict the index of line-start offsets. */
+
+void
+edited_file::evict_line_start_index ()
+{
+ m_line_start_index.truncate (0);
+}
+
+/* Get the total number of lines in m_content. */
+
+int
+edited_file::get_num_lines ()
+{
+ ensure_line_start_index ();
+ return m_line_start_index.length ();
+}
+
+/* Implementation of class line_state. */
+
+/* A callback for deleting line_state *, for use as a
+ delete_value_fn for edited_file::m_edited_lines. */
+
+void
+line_state::delete_cb (line_state *ls)
+{
+ delete ls;
+}
+
+/* line_state's dtor. */
+
+line_state::~line_state ()
+{
+ int i;
+ line_event *event;
+ FOR_EACH_VEC_ELT (m_line_events, i, event)
+ delete event;
+}
+
+/* Map a location before the edits to a column number after the edits,
+ within a specific line. */
+
+int
+line_state::get_effective_column (int orig_column) const
+{
+ int i;
+ line_event *event;
+ FOR_EACH_VEC_ELT (m_line_events, i, event)
+ orig_column = event->get_effective_column (orig_column);
+ return orig_column;
+}
+
+/* Record that a string of length LEN was inserted at COLUMN within
+ this line, so that future changes to the line can have their
+ column information adjusted accordingly. */
+
+void
+line_state::apply_insert (int column, int len)
+{
+ m_line_events.safe_push (new insert_event (column, len));
+}
+
+/* Record that the string between columns START and FINISH
+ of this line was replaced with a string of length LEN,
+ so that future changes to the line can have their
+ column information adjusted accordingly. */
+
+void
+line_state::apply_replace (int start, int finish, int len)
+{
+ m_line_events.safe_push (new replace_event (start, finish, len));
+}
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Selftests of code-editing. */
+
+/* Test applying an "insert" fixit. */
+
+static void
+test_applying_fixits_insert (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................0000000001111111.
+ .....................1234567890123456. */
+ const char *content = ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, tmp.get_filename (), 2);
+
+ /* Add a comment in front of "bar.field". */
+ location_t start = linemap_position_for_column (line_table, 7);
+ rich_location richloc (line_table, start);
+ richloc.add_fixit_insert (start, "/* inserted */");
+
+ if (start > LINE_MAP_MAX_LOCATION_WITH_COLS)
+ return;
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (start <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ ASSERT_STREQ ("/* before */\n"
+ "foo = /* inserted */bar.field;\n"
+ "/* after */\n", result);
+
+ /* Verify that locations on other lines aren't affected by the change. */
+ ASSERT_EQ (100, edit.get_effective_column (filename, 1, 100));
+ ASSERT_EQ (100, edit.get_effective_column (filename, 3, 100));
+
+ /* Verify locations on the line before the change. */
+ ASSERT_EQ (1, edit.get_effective_column (filename, 2, 1));
+ ASSERT_EQ (6, edit.get_effective_column (filename, 2, 6));
+
+ /* Verify locations on the line at and after the change. */
+ ASSERT_EQ (21, edit.get_effective_column (filename, 2, 7));
+ ASSERT_EQ (22, edit.get_effective_column (filename, 2, 8));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.field;\n"
+ "+foo = /* inserted */bar.field;\n"
+ " /* after */\n", diff);
+ free (diff);
+}
+
+/* Test applying a "replace" fixit that grows the affected line. */
+
+static void
+test_applying_fixits_growing_replace (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................0000000001111111.
+ .....................1234567890123456. */
+ const char *content = ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Replace "field" with "m_field". */
+ location_t start = linemap_position_for_column (line_table, 11);
+ location_t finish = linemap_position_for_column (line_table, 15);
+ location_t field = make_location (start, start, finish);
+ rich_location richloc (line_table, field);
+ source_range range;
+ range.m_start = start;
+ range.m_finish = finish;
+ richloc.add_fixit_replace (range, "m_field");
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (finish <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = bar.m_field;\n"
+ "/* after */\n", result);
+
+ /* Verify location of ";" after the change. */
+ ASSERT_EQ (18, edit.get_effective_column (filename, 2, 16));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.field;\n"
+ "+foo = bar.m_field;\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Test applying a "replace" fixit that shrinks the affected line. */
+
+static void
+test_applying_fixits_shrinking_replace (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................000000000111111111.
+ .....................123456789012345678. */
+ const char *content = ("/* before */\n"
+ "foo = bar.m_field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Replace "field" with "m_field". */
+ location_t start = linemap_position_for_column (line_table, 11);
+ location_t finish = linemap_position_for_column (line_table, 17);
+ location_t m_field = make_location (start, start, finish);
+ rich_location richloc (line_table, m_field);
+ source_range range;
+ range.m_start = start;
+ range.m_finish = finish;
+ richloc.add_fixit_replace (range, "field");
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (finish <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n", result);
+
+ /* Verify location of ";" after the change. */
+ ASSERT_EQ (16, edit.get_effective_column (filename, 2, 18));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.m_field;\n"
+ "+foo = bar.field;\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Test applying a "remove" fixit. */
+
+static void
+test_applying_fixits_remove (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................000000000111111111.
+ .....................123456789012345678. */
+ const char *content = ("/* before */\n"
+ "foo = bar.m_field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Remove ".m_field". */
+ location_t start = linemap_position_for_column (line_table, 10);
+ location_t finish = linemap_position_for_column (line_table, 17);
+ rich_location richloc (line_table, start);
+ source_range range;
+ range.m_start = start;
+ range.m_finish = finish;
+ richloc.add_fixit_remove (range);
+
+ edit_context edit;
+ edit.add_fixits (&richloc);
+ const char *result = edit.get_content (filename);
+ if (finish <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = bar;\n"
+ "/* after */\n", result);
+
+ /* Verify location of ";" after the change. */
+ ASSERT_EQ (10, edit.get_effective_column (filename, 2, 18));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.m_field;\n"
+ "+foo = bar;\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Test applying multiple fixits to one line. */
+
+static void
+test_applying_fixits_multiple (const line_table_case &case_)
+{
+ /* Create a tempfile and write some text to it.
+ .....................00000000011111111.
+ .....................12345678901234567. */
+ const char *content = ("/* before */\n"
+ "foo = bar.field;\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt (case_);
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ location_t c7 = linemap_position_for_column (line_table, 7);
+ location_t c9 = linemap_position_for_column (line_table, 9);
+ location_t c11 = linemap_position_for_column (line_table, 11);
+ location_t c15 = linemap_position_for_column (line_table, 15);
+ location_t c17 = linemap_position_for_column (line_table, 17);
+
+ if (c17 > LINE_MAP_MAX_LOCATION_WITH_COLS)
+ return;
+
+ /* Add a comment in front of "bar.field". */
+ rich_location insert_a (line_table, c7);
+ insert_a.add_fixit_insert (c7, "/* alpha */");
+
+ /* Add a comment after "bar.field;". */
+ rich_location insert_b (line_table, c17);
+ insert_b.add_fixit_insert (c17, "/* beta */");
+
+ /* Replace "bar" with "pub". */
+ rich_location replace_a (line_table, c7);
+ replace_a.add_fixit_replace (source_range::from_locations (c7, c9),
+ "pub");
+
+ /* Replace "field" with "meadow". */
+ rich_location replace_b (line_table, c7);
+ replace_b.add_fixit_replace (source_range::from_locations (c11, c15),
+ "meadow");
+
+ edit_context edit;
+ edit.add_fixits (&insert_a);
+ ASSERT_EQ (100, edit.get_effective_column (filename, 1, 100));
+ ASSERT_EQ (1, edit.get_effective_column (filename, 2, 1));
+ ASSERT_EQ (6, edit.get_effective_column (filename, 2, 6));
+ ASSERT_EQ (18, edit.get_effective_column (filename, 2, 7));
+ ASSERT_EQ (27, edit.get_effective_column (filename, 2, 16));
+ ASSERT_EQ (100, edit.get_effective_column (filename, 3, 100));
+
+ edit.add_fixits (&insert_b);
+ edit.add_fixits (&replace_a);
+ edit.add_fixits (&replace_b);
+
+ if (c17 <= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ {
+ ASSERT_STREQ ("/* before */\n"
+ "foo = /* alpha */pub.meadow;/* beta */\n"
+ "/* after */\n",
+ edit.get_content (filename));
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,3 +1,3 @@\n"
+ " /* before */\n"
+ "-foo = bar.field;\n"
+ "+foo = /* alpha */pub.meadow;/* beta */\n"
+ " /* after */\n", diff);
+ free (diff);
+ }
+}
+
+/* Subroutine of test_applying_fixits_multiple_lines.
+ Add the text "CHANGED: " to the front of the given line. */
+
+static void
+change_line (edit_context &edit, int line_num)
+{
+ const line_map_ordinary *ord_map
+ = LINEMAPS_LAST_ORDINARY_MAP (line_table);
+ const int column = 1;
+ location_t loc =
+ linemap_position_for_line_and_column (line_table, ord_map,
+ line_num, column);
+
+ expanded_location exploc = expand_location (loc);
+ ASSERT_EQ (line_num, exploc.line);
+ ASSERT_EQ (column, exploc.column);
+
+ rich_location insert (line_table, loc);
+ insert.add_fixit_insert (loc, "CHANGED: ");
+ edit.add_fixits (&insert);
+}
+
+/* Test of editing multiple lines within a long file,
+ to ensure that diffs are generated as expected. */
+
+static void
+test_applying_fixits_multiple_lines ()
+{
+ /* Create a tempfile and write many lines of text to it. */
+ named_temp_file tmp (".txt");
+ const char *filename = tmp.get_filename ();
+ FILE *f = fopen (filename, "w");
+ ASSERT_NE (f, NULL);
+ for (int i = 1; i <= 1000; i++)
+ fprintf (f, "line %i\n", i);
+ fclose (f);
+
+ line_table_test ltt;
+ linemap_add (line_table, LC_ENTER, false, filename, 1);
+ linemap_position_for_column (line_table, 127);
+
+ edit_context edit;
+
+ /* A run of consecutive lines. */
+ change_line (edit, 2);
+ change_line (edit, 3);
+ change_line (edit, 4);
+
+ /* A run of nearby lines, within the contextual limit. */
+ change_line (edit, 150);
+ change_line (edit, 151);
+ change_line (edit, 153);
+
+ /* Verify diff. */
+ char *diff = edit.generate_diff (false);
+ ASSERT_STREQ ("@@ -1,7 +1,7 @@\n"
+ " line 1\n"
+ "-line 2\n"
+ "-line 3\n"
+ "-line 4\n"
+ "+CHANGED: line 2\n"
+ "+CHANGED: line 3\n"
+ "+CHANGED: line 4\n"
+ " line 5\n"
+ " line 6\n"
+ " line 7\n"
+ "@@ -147,10 +147,10 @@\n"
+ " line 147\n"
+ " line 148\n"
+ " line 149\n"
+ "-line 150\n"
+ "-line 151\n"
+ "+CHANGED: line 150\n"
+ "+CHANGED: line 151\n"
+ " line 152\n"
+ "-line 153\n"
+ "+CHANGED: line 153\n"
+ " line 154\n"
+ " line 155\n"
+ " line 156\n", diff);
+ free (diff);
+
+ /* Ensure tmp stays alive until this point, so that the tempfile
+ persists until after the generate_diff call. */
+ tmp.get_filename ();
+}
+
+/* Test of converting an initializer for a named field from
+ the old GCC extension to C99 syntax.
+ Exercises a shrinking replacement followed by a growing
+ replacement on the same line. */
+
+static void
+test_applying_fixits_modernize_named_init ()
+{
+ /* Create a tempfile and write some text to it.
+ .....................00000000011111111.
+ .....................12345678901234567. */
+ const char *content = ("/* before */\n"
+ "bar : 1,\n"
+ "/* after */\n");
+ temp_source_file tmp (SELFTEST_LOCATION, ".c", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt ();
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ location_t c1 = linemap_position_for_column (line_table, 1);
+ location_t c3 = linemap_position_for_column (line_table, 3);
+ location_t c8 = linemap_position_for_column (line_table, 8);
+
+ /* Replace "bar" with ".". */
+ rich_location r1 (line_table, c8);
+ r1.add_fixit_replace (source_range::from_locations (c1, c3),
+ ".");
+
+ /* Replace ":" with "bar =". */
+ rich_location r2 (line_table, c8);
+ r2.add_fixit_replace (source_range::from_locations (c8, c8),
+ "bar =");
+
+ /* The order should not matter. Do r1 then r2. */
+ {
+ edit_context edit;
+ edit.add_fixits (&r1);
+
+ /* Verify state after first replacement. */
+ {
+ /* We should now have:
+ ............00000000011.
+ ............12345678901. */
+ ASSERT_STREQ ("/* before */\n"
+ ". : 1,\n"
+ "/* after */\n",
+ edit.get_content (filename));
+ /* Location of the "1". */
+ ASSERT_EQ (6, edit.get_effective_column (filename, 2, 8));
+ /* Location of the ",". */
+ ASSERT_EQ (9, edit.get_effective_column (filename, 2, 11));
+ }
+
+ edit.add_fixits (&r2);
+
+ /* Verify state after second replacement.
+ ............00000000011111111.
+ ............12345678901234567. */
+ ASSERT_STREQ ("/* before */\n"
+ ". bar = 1,\n"
+ "/* after */\n",
+ edit.get_content (filename));
+ }
+
+ /* Try again, doing r2 then r1; the result should be the same. */
+ {
+ edit_context edit;
+ edit.add_fixits (&r2);
+ edit.add_fixits (&r1);
+ /*.............00000000011111111.
+ .............12345678901234567. */
+ ASSERT_STREQ ("/* before */\n"
+ ". bar = 1,\n"
+ "/* after */\n",
+ edit.get_content (filename));
+ }
+}
+
+/* Test of a fixit affecting a file that can't be read. */
+
+static void
+test_applying_fixits_unreadable_file ()
+{
+ const char *filename = "this-does-not-exist.txt";
+ line_table_test ltt ();
+ linemap_add (line_table, LC_ENTER, false, filename, 1);
+
+ location_t loc = linemap_position_for_column (line_table, 1);
+
+ rich_location insert (line_table, loc);
+ insert.add_fixit_insert (loc, "change 1");
+ insert.add_fixit_insert (loc, "change 2");
+
+ edit_context edit;
+ /* Attempting to add the fixits affecting the unreadable file
+ should transition the edit from valid to invalid. */
+ ASSERT_TRUE (edit.valid_p ());
+ edit.add_fixits (&insert);
+ ASSERT_FALSE (edit.valid_p ());
+ ASSERT_EQ (NULL, edit.get_content (filename));
+ ASSERT_EQ (NULL, edit.generate_diff (false));
+}
+
+/* Verify that we gracefully handle an attempt to edit a line
+ that's beyond the end of the file. */
+
+static void
+test_applying_fixits_line_out_of_range ()
+{
+ /* Create a tempfile and write some text to it.
+ .....................00000000011111111.
+ .....................12345678901234567. */
+ const char *content = "One-liner file\n";
+ temp_source_file tmp (SELFTEST_LOCATION, ".txt", content);
+ const char *filename = tmp.get_filename ();
+ line_table_test ltt ();
+ linemap_add (line_table, LC_ENTER, false, filename, 2);
+
+ /* Try to insert a string in line 2. */
+ location_t loc = linemap_position_for_column (line_table, 1);
+
+ rich_location insert (line_table, loc);
+ insert.add_fixit_insert (loc, "change");
+
+ /* Verify that attempting the insertion puts an edit_context
+ into an invalid state. */
+ edit_context edit;
+ ASSERT_TRUE (edit.valid_p ());
+ edit.add_fixits (&insert);
+ ASSERT_FALSE (edit.valid_p ());
+ ASSERT_EQ (NULL, edit.get_content (filename));
+ ASSERT_EQ (NULL, edit.generate_diff (false));
+}
+
+/* Run all of the selftests within this file. */
+
+void
+edit_context_c_tests ()
+{
+ for_each_line_table_case (test_applying_fixits_insert);
+ for_each_line_table_case (test_applying_fixits_growing_replace);
+ for_each_line_table_case (test_applying_fixits_shrinking_replace);
+ for_each_line_table_case (test_applying_fixits_remove);
+ for_each_line_table_case (test_applying_fixits_multiple);
+ test_applying_fixits_multiple_lines ();
+ test_applying_fixits_modernize_named_init ();
+ test_applying_fixits_unreadable_file ();
+ test_applying_fixits_line_out_of_range ();
+}
+
+} // namespace selftest
+
+#endif /* CHECKING_P */
diff --git a/gcc/edit-context.h b/gcc/edit-context.h
new file mode 100644
index 0000000..54e576b
--- /dev/null
+++ b/gcc/edit-context.h
@@ -0,0 +1,65 @@
+/* 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/>. */
+
+#ifndef GCC_EDIT_CONTEXT_H
+#define GCC_EDIT_CONTEXT_H
+
+#include "typed-splay-tree.h"
+
+class edit_context;
+class edited_file;
+
+/* A set of changes to the source code.
+
+ The changes are "atomic" - if any changes can't be applied,
+ none of can be (tracked by the m_valid flag).
+
+ A complication here is that fix-its are expressed relative to coordinates
+ in the files when they were 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. */
+
+class edit_context
+{
+ public:
+ edit_context ();
+
+ bool valid_p () const { return m_valid; }
+
+ void add_fixits (rich_location *richloc);
+
+ const char *get_content (const char *filename);
+
+ int get_effective_column (const char *filename, int line, int column);
+
+ char *generate_diff (bool show_filenames);
+ void print_diff (pretty_printer *pp, bool show_filenames);
+
+ private:
+ bool apply_insert (const fixit_insert *insert);
+ bool apply_replace (const fixit_replace *replace);
+ edited_file *get_file (const char *filename);
+ edited_file &get_or_insert_file (const char *filename);
+
+ bool m_valid;
+ typed_splay_tree<const char *, edited_file *> m_files;
+};
+
+#endif /* GCC_EDIT_CONTEXT_H. */
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index e20bbd0..d9d3ea1 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -68,6 +68,7 @@ selftest::run_tests ()
rely on. */
diagnostic_show_locus_c_tests ();
diagnostic_c_tests ();
+ edit_context_c_tests ();
fold_const_c_tests ();
spellcheck_c_tests ();
spellcheck_tree_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 8e6c47a..41487af 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -150,6 +150,7 @@ for_each_line_table_case (void (*testcase) (const line_table_case &));
extern void bitmap_c_tests ();
extern void diagnostic_c_tests ();
extern void diagnostic_show_locus_c_tests ();
+extern void edit_context_c_tests ();
extern void et_forest_c_tests ();
extern void fold_const_c_tests ();
extern void fibonacci_heap_c_tests ();
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/4] Improvements to typed_splay_tree
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
2016-08-25 0:44 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
2016-08-25 0:45 ` [PATCH 3/4] (v2) Introduce class edit_context David Malcolm
@ 2016-08-25 0:45 ` David Malcolm
2016-08-25 0:45 ` [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch David Malcolm
2016-09-09 23:01 ` [PATCH 0/4] (v2) Generating patches from fix-it hints Jeff Law
4 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-25 0:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, David Malcolm
This patch adds foreach, max and min methods to
class typed_splay_tree, along with the start of a selftest
suite.
gcc/ChangeLog:
* Makefile.in (OBJS): Add typed-splay-tree.o.
* selftest-run-tests.c (selftest::run_tests): Call
typed_splay_tree_c_tests.
* selftest.h (typed_splay_tree_c_tests): New decl.
* typed-splay-tree.c: New file.
* typed-splay-tree.h (typed_splay_tree::foreach_fn): New typedef.
(typed_splay_tree::max): New method.
(typed_splay_tree::min): New method.
(typed_splay_tree::foreach): New method.
(struct closure): New struct.
(inner_foreach_fn): New function.
---
gcc/Makefile.in | 1 +
gcc/selftest-run-tests.c | 1 +
gcc/selftest.h | 1 +
gcc/typed-splay-tree.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
gcc/typed-splay-tree.h | 67 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 149 insertions(+)
create mode 100644 gcc/typed-splay-tree.c
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 8d7cc51..f5f3339 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1542,6 +1542,7 @@ OBJS = \
tree-vectorizer.o \
tree-vrp.o \
tree.o \
+ typed-splay-tree.o \
valtrack.o \
value-prof.o \
var-tracking.o \
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 6453e31..e20bbd0 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -56,6 +56,7 @@ selftest::run_tests ()
ggc_tests_c_tests ();
sreal_c_tests ();
fibonacci_heap_c_tests ();
+ typed_splay_tree_c_tests ();
/* Mid-level data structures. */
input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index fd3c6b0..8e6c47a 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -165,6 +165,7 @@ extern void selftest_c_tests ();
extern void spellcheck_c_tests ();
extern void spellcheck_tree_c_tests ();
extern void sreal_c_tests ();
+extern void typed_splay_tree_c_tests ();
extern void tree_c_tests ();
extern void tree_cfg_c_tests ();
extern void vec_c_tests ();
diff --git a/gcc/typed-splay-tree.c b/gcc/typed-splay-tree.c
new file mode 100644
index 0000000..33992c1
--- /dev/null
+++ b/gcc/typed-splay-tree.c
@@ -0,0 +1,79 @@
+/* Selftests for typed-splay-tree.h.
+ 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 "typed-splay-tree.h"
+#include "selftest.h"
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Callback for use by test_str_to_int. */
+
+static int
+append_cb (const char *, int value, void *user_data)
+{
+ auto_vec <int> *vec = (auto_vec <int> *)user_data;
+ vec->safe_push (value);
+ return 0;
+}
+
+/* Test of typed_splay_tree <const char *, int>. */
+
+static void
+test_str_to_int ()
+{
+ typed_splay_tree <const char *, int> t (strcmp, NULL, NULL);
+
+ t.insert ("a", 1);
+ t.insert ("b", 2);
+ t.insert ("c", 3);
+
+ ASSERT_EQ (1, t.lookup ("a"));
+ ASSERT_EQ (2, t.lookup ("b"));
+ ASSERT_EQ (3, t.lookup ("c"));
+
+ ASSERT_EQ (2, t.predecessor ("c"));
+ ASSERT_EQ (3, t.successor ("b"));
+ ASSERT_EQ (1, t.min ());
+ ASSERT_EQ (3, t.max ());
+
+ /* Test foreach by appending to a vec, and verifying the vec. */
+ auto_vec <int> v;
+ t.foreach (append_cb, &v);
+ ASSERT_EQ (3, v.length ());
+ ASSERT_EQ (1, v[0]);
+ ASSERT_EQ (2, v[1]);
+ ASSERT_EQ (3, v[2]);
+}
+
+/* Run all of the selftests within this file. */
+
+void
+typed_splay_tree_c_tests ()
+{
+ test_str_to_int ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/typed-splay-tree.h b/gcc/typed-splay-tree.h
index 9dd96d6..b52fc20 100644
--- a/gcc/typed-splay-tree.h
+++ b/gcc/typed-splay-tree.h
@@ -33,6 +33,7 @@ class typed_splay_tree
typedef int (*compare_fn) (key_type, key_type);
typedef void (*delete_key_fn) (key_type);
typedef void (*delete_value_fn) (value_type);
+ typedef int (*foreach_fn) (key_type, value_type, void *);
typed_splay_tree (compare_fn,
delete_key_fn,
@@ -43,6 +44,9 @@ class typed_splay_tree
value_type predecessor (key_type k);
value_type successor (key_type k);
void insert (key_type k, value_type v);
+ value_type max ();
+ value_type min ();
+ int foreach (foreach_fn, void *);
private:
static value_type node_to_value (splay_tree_node node);
@@ -120,6 +124,69 @@ typed_splay_tree<KEY_TYPE, VALUE_TYPE>::insert (key_type key,
(splay_tree_value)value);
}
+/* Get the value with maximal key. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline VALUE_TYPE
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::max ()
+{
+ return node_to_value (splay_tree_max (m_inner));
+}
+
+/* Get the value with minimal key. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline VALUE_TYPE
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::min ()
+{
+ return node_to_value (splay_tree_min (m_inner));
+}
+
+/* Helper type for typed_splay_tree::foreach. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+struct closure
+{
+ typedef typename typed_splay_tree<KEY_TYPE, VALUE_TYPE>::foreach_fn foreach_fn;
+
+ closure (foreach_fn outer_cb,
+ void *outer_user_data)
+ : m_outer_cb (outer_cb), m_outer_user_data (outer_user_data) {}
+
+ foreach_fn m_outer_cb;
+ void *m_outer_user_data;
+};
+
+/* Helper function for typed_splay_tree::foreach. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+static int
+inner_foreach_fn (splay_tree_node node, void *user_data)
+{
+ closure <KEY_TYPE, VALUE_TYPE> *c
+ = (closure <KEY_TYPE, VALUE_TYPE> *)user_data;
+
+ return c->m_outer_cb ((KEY_TYPE)node->key, (VALUE_TYPE)node->value,
+ c->m_outer_user_data);
+}
+
+/* Call OUTER_CB, passing it the OUTER_USER_DATA, for every node,
+ following an in-order traversal. If OUTER_CB ever returns a non-zero
+ value, the iteration ceases immediately, and the value is returned.
+ Otherwise, this function returns 0. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline int
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::foreach (foreach_fn outer_cb,
+ void *outer_user_data)
+{
+ closure <KEY_TYPE, VALUE_TYPE> c (outer_cb, outer_user_data);
+
+ return splay_tree_foreach (m_inner,
+ inner_foreach_fn<KEY_TYPE, VALUE_TYPE>,
+ &c);
+}
+
/* Internal function for converting from splay_tree_node to
VALUE_TYPE. */
template <typename KEY_TYPE, typename VALUE_TYPE>
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 0/4] (v2) Generating patches from fix-it hints
2016-08-24 7:59 ` Richard Biener
2016-08-24 13:29 ` Trevor Saunders
2016-08-24 13:32 ` David Malcolm
@ 2016-08-25 0:45 ` David Malcolm
2016-08-25 0:44 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
` (4 more replies)
2 siblings, 5 replies; 31+ messages in thread
From: David Malcolm @ 2016-08-25 0:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, David Malcolm
Here's a much less ambitious version of the patch kit, which
eliminates any attempt to write to the user's source
code (getting rid of edit_context::apply_changes and
-fdiagnostics-apply-fixits).
It implements -fdiagnostics-generate-patch. In so doing, it
tightens up the exact semantics of fix-its; see [1] for an
example of where that's useful.
I need review of at least patches 1 and 2 (which are unchanged
from v1 of the kit). I believe I can self-approve patches 3
and 4 as part of my "diagnostics maintainer" role; are they
acceptable to those who objected to the earlier kit? (now
that there's no attempt to write to source files).
Successfully bootstrapped®rtested the combination of the patches
on x86_64-pc-linux-gnu.
OK for trunk? (assuming individual bootstraps®rtesting)
[1] https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01751.html
David Malcolm (4):
selftest: split out named_temp_file from temp_source_file
Improvements to typed_splay_tree
Introduce class edit_context (v2)
Add -fdiagnostics-generate-patch (v2)
gcc/Makefile.in | 2 +
gcc/common.opt | 4 +
gcc/diagnostic-color.c | 8 +-
gcc/diagnostic.c | 11 +
gcc/diagnostic.h | 6 +
gcc/doc/invoke.texi | 23 +-
gcc/edit-context.c | 1306 ++++++++++++++++++++
gcc/edit-context.h | 65 +
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 | 14 +
gcc/typed-splay-tree.c | 79 ++
gcc/typed-splay-tree.h | 67 +
16 files changed, 1718 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] Applying fix-its on behalf of the user to their code
2016-08-24 21:52 ` Manuel López-Ibáñez
@ 2016-08-25 9:20 ` Richard Biener
0 siblings, 0 replies; 31+ messages in thread
From: Richard Biener @ 2016-08-25 9:20 UTC (permalink / raw)
To: Manuel López-Ibáñez
Cc: Bernd Schmidt, David Malcolm, GCC Patches
On Wed, Aug 24, 2016 at 11:52 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 24/08/16 14:56, Richard Biener wrote:
>>
>> You never typoed
>>
>> gcc t.c -o t.c
>>
>> ? ;) (I did ... :/)
>
>
> With GCC >=5
>
> $ gcc t.c -o t.c
> gcc: fatal error: input file ‘t.c’ is the same as output file
> compilation terminated.
>
> You are welcome ;-)
Heh. Need to update my host compiler then...
Similar "interesting" things happen with
> gcc t.c t.C
works ok (using tmpfiles for intermediate assembler)
> gcc t.c t.C -save-temps
and you get everthing from t.C twice ... (they share the same
intermediate assembler file name, t.s!)
Richard.
>
> Manuel.
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/4] (v2) Introduce class edit_context
2016-08-25 0:45 ` [PATCH 3/4] (v2) Introduce class edit_context David Malcolm
@ 2016-08-28 15:05 ` Trevor Saunders
2016-08-29 18:53 ` David Malcolm
0 siblings, 1 reply; 31+ messages in thread
From: Trevor Saunders @ 2016-08-28 15:05 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches, Richard Biener
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/4] (v2) Introduce class edit_context
2016-08-28 15:05 ` Trevor Saunders
@ 2016-08-29 18:53 ` David Malcolm
2016-08-30 2:51 ` Trevor Saunders
0 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-08-29 18:53 UTC (permalink / raw)
To: Trevor Saunders; +Cc: gcc-patches, Richard Biener
On Sun, 2016-08-28 at 11:13 -0400, Trevor Saunders wrote:
> 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.
(nods), and I think read_from_file could have been made private. But I
will probably eliminate it as...
> > + 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.
...yes, though I'd put the content into class edited_line. Unedited
lines come from input.c's source cache; we can look up edited lines by
number using the splay tree in class edited_file. That ought to be
much more efficient that my current "store the whole file in class
edited_file" implementation, and this efficiency would help if we want
to use class edit_context in diagnostic-show-locus.c for printing fix
-it hints, to show the region of the line that's been touched by a fix
-it
(see https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01816.html )
> > +};
> > +
> > +/* 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?
I think I want to rule that as invalid: that it's not valid to have
overlapping "replace" fixits, and that (ideally) attempts to do so
ought to be rejected within rich_location (they aren't yet rejected at
the moment).
> > + }
> > +
> > + 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.
That occurred to me. IIRC clang does this, but their source ranges are
half-open, whereas ours are closed (I don't remember why, only that it
made sense at the time...). Maybe we could just put a bool into the
combined class, and if an insert, then ignore the range's finish
location.
> > +change_line (edit_context &edit, int line_num)
>
> I guess this is test only, put I'm not a fan of non const references.
Out of interest, how would you have written it?
> Trev
Thank
Dave
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/4] selftest: split out named_temp_file from temp_source_file
2016-08-25 0:44 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
@ 2016-08-29 21:53 ` Bernd Schmidt
2016-08-31 0:02 ` [PATCH] selftest.c: avoid explicit "selftest::" qualifiers David Malcolm
0 siblings, 1 reply; 31+ messages in thread
From: Bernd Schmidt @ 2016-08-29 21:53 UTC (permalink / raw)
To: David Malcolm, gcc-patches; +Cc: Richard Biener
On 08/25/2016 03:13 AM, David Malcolm wrote:
> Split out a new base class for temp_source_file, named_temp_file,
> moving the deletion to the base class dtor, so that we can write
> out temporary files in other ways in selftests.
>
> gcc/ChangeLog:
> * selftest.c (selftest::named_temp_file::named_temp_file): New
> ctor.
> (selftest::temp_source_file::~temp_source_file): Move to...
> (selftest::named_temp_file::~named_temp_file): ...here.
> (selftest::test_named_temp_file): New function.
> (selftest::selftest_c_tests): Call test_named_temp_file.
> * selftest.h (class named_temp_file): New class.
> (class temp_source_file): Convert to a subclass of named_temp_file.
Ok.
> +selftest::named_temp_file::named_temp_file (const char *suffix)
Any reason these aren't inside namespace selftest to shorten these
declarations?
Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/4] (v2) Introduce class edit_context
2016-08-29 18:53 ` David Malcolm
@ 2016-08-30 2:51 ` Trevor Saunders
0 siblings, 0 replies; 31+ messages in thread
From: Trevor Saunders @ 2016-08-30 2:51 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches, Richard Biener
On Mon, Aug 29, 2016 at 02:53:43PM -0400, David Malcolm wrote:
> On Sun, 2016-08-28 at 11:13 -0400, Trevor Saunders wrote:
> > On Wed, Aug 24, 2016 at 09:13:51PM -0400, David Malcolm wrote:
> > > +
> > > + 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.
>
> ...yes, though I'd put the content into class edited_line. Unedited
> lines come from input.c's source cache; we can look up edited lines by
> number using the splay tree in class edited_file. That ought to be
> much more efficient that my current "store the whole file in class
> edited_file" implementation, and this efficiency would help if we want
> to use class edit_context in diagnostic-show-locus.c for printing fix
> -it hints, to show the region of the line that's been touched by a fix
> -it
> (see https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01816.html )
yeah, that does sound even better.
> > > +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?
>
> I think I want to rule that as invalid: that it's not valid to have
> overlapping "replace" fixits, and that (ideally) attempts to do so
> ought to be rejected within rich_location (they aren't yet rejected at
> the moment).
yeah, that is the simple solution ;)
>
> > > + }
> > > +
> > > + 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.
>
> That occurred to me. IIRC clang does this, but their source ranges are
> half-open, whereas ours are closed (I don't remember why, only that it
> made sense at the time...). Maybe we could just put a bool into the
> combined class, and if an insert, then ignore the range's finish
> location.
yeah, that is a complication to be thought about.
>
> > > +change_line (edit_context &edit, int line_num)
> >
> > I guess this is test only, put I'm not a fan of non const references.
>
> Out of interest, how would you have written it?
probably just using a pointer. the nonnull template in the GSL would be
nice, but I'm not sure if I care enough to backport it to C++98.
Trev
>
> > Trev
>
> Thank
> Dave
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] selftest.c: avoid explicit "selftest::" qualifiers
2016-08-29 21:53 ` Bernd Schmidt
@ 2016-08-31 0:02 ` David Malcolm
2016-08-31 8:36 ` Bernd Schmidt
0 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-08-31 0:02 UTC (permalink / raw)
To: Bernd Schmidt, gcc-patches; +Cc: Richard Biener, David Malcolm
On Mon, 2016-08-29 at 23:53 +0200, Bernd Schmidt wrote:
> On 08/25/2016 03:13 AM, David Malcolm wrote:
> > Split out a new base class for temp_source_file, named_temp_file,
> > moving the deletion to the base class dtor, so that we can write
> > out temporary files in other ways in selftests.
> >
> > gcc/ChangeLog:
> > * selftest.c (selftest::named_temp_file::named_temp_file): New
> > ctor.
> > (selftest::temp_source_file::~temp_source_file): Move to...
> > (selftest::named_temp_file::~named_temp_file): ...here.
> > (selftest::test_named_temp_file): New function.
> > (selftest::selftest_c_tests): Call test_named_temp_file.
> > * selftest.h (class named_temp_file): New class.
> > (class temp_source_file): Convert to a subclass of
> > named_temp_file.
>
> Ok.
>
> > +selftest::named_temp_file::named_temp_file (const char *suffix)
>
> Any reason these aren't inside namespace selftest to shorten these
> declarations?
I was being consistent with the rest of the file, but I no longer
remember why I used explicit namespace selftest:: prefixes there.
The following follow-up patch removes them, moving the
"namespace selftest {" to the top of the file so it covers
everything.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/ChangeLog:
* selftest.c: Move "namespace selftest {" to top of file,
removing explicit "selftest::" qualifiers throughout.
---
gcc/selftest.c | 78 +++++++++++++++++++++++++++-------------------------------
1 file changed, 36 insertions(+), 42 deletions(-)
diff --git a/gcc/selftest.c b/gcc/selftest.c
index e6c9510..69d9931 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -24,12 +24,14 @@ along with GCC; see the file COPYING3. If not see
#if CHECKING_P
-int selftest::num_passes;
+namespace selftest {
+
+int num_passes;
/* Record the successful outcome of some aspect of a test. */
void
-selftest::pass (const location &/*loc*/, const char */*msg*/)
+pass (const location &/*loc*/, const char */*msg*/)
{
num_passes++;
}
@@ -37,7 +39,7 @@ selftest::pass (const location &/*loc*/, const char */*msg*/)
/* Report the failed outcome of some aspect of a test and abort. */
void
-selftest::fail (const location &loc, const char *msg)
+fail (const location &loc, const char *msg)
{
fprintf (stderr,"%s:%i: %s: FAIL: %s\n", loc.m_file, loc.m_line,
loc.m_function, msg);
@@ -47,7 +49,7 @@ selftest::fail (const location &loc, const char *msg)
/* As "fail", but using printf-style formatted output. */
void
-selftest::fail_formatted (const location &loc, const char *fmt, ...)
+fail_formatted (const location &loc, const char *fmt, ...)
{
va_list ap;
@@ -65,26 +67,23 @@ selftest::fail_formatted (const location &loc, const char *fmt, ...)
to be non-NULL; fail gracefully if either are NULL. */
void
-selftest::assert_streq (const location &loc,
- const char *desc_expected, const char *desc_actual,
- const char *val_expected, const char *val_actual)
+assert_streq (const location &loc,
+ const char *desc_expected, const char *desc_actual,
+ const char *val_expected, const char *val_actual)
{
/* If val_expected is NULL, the test is buggy. Fail gracefully. */
if (val_expected == NULL)
- ::selftest::fail_formatted
- (loc, "ASSERT_STREQ (%s, %s) expected=NULL",
- desc_expected, desc_actual);
+ fail_formatted (loc, "ASSERT_STREQ (%s, %s) expected=NULL",
+ desc_expected, desc_actual);
/* If val_actual is NULL, fail with a custom error message. */
if (val_actual == NULL)
- ::selftest::fail_formatted
- (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=NULL",
- desc_expected, desc_actual, val_expected);
+ fail_formatted (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=NULL",
+ desc_expected, desc_actual, val_expected);
if (0 == strcmp (val_expected, val_actual))
- ::selftest::pass (loc, "ASSERT_STREQ");
+ pass (loc, "ASSERT_STREQ");
else
- ::selftest::fail_formatted
- (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
- desc_expected, desc_actual, val_expected, val_actual);
+ fail_formatted (loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
+ desc_expected, desc_actual, val_expected, val_actual);
}
/* Implementation detail of ASSERT_STR_CONTAINS.
@@ -93,36 +92,35 @@ selftest::assert_streq (const location &loc,
::selftest::fail if it is not found. */
void
-selftest::assert_str_contains (const location &loc,
- const char *desc_haystack,
- const char *desc_needle,
- const char *val_haystack,
- const char *val_needle)
+assert_str_contains (const location &loc,
+ const char *desc_haystack,
+ const char *desc_needle,
+ const char *val_haystack,
+ const char *val_needle)
{
/* If val_haystack is NULL, fail with a custom error message. */
if (val_haystack == NULL)
- ::selftest::fail_formatted
- (loc, "ASSERT_STR_CONTAINS (%s, %s) haystack=NULL",
- desc_haystack, desc_needle);
+ fail_formatted (loc, "ASSERT_STR_CONTAINS (%s, %s) haystack=NULL",
+ desc_haystack, desc_needle);
/* If val_needle is NULL, fail with a custom error message. */
if (val_needle == NULL)
- ::selftest::fail_formatted
- (loc, "ASSERT_STR_CONTAINS (%s, %s) haystack=\"%s\" needle=NULL",
- desc_haystack, desc_needle, val_haystack);
+ fail_formatted (loc,
+ "ASSERT_STR_CONTAINS (%s, %s) haystack=\"%s\" needle=NULL",
+ desc_haystack, desc_needle, val_haystack);
const char *test = strstr (val_haystack, val_needle);
if (test)
- ::selftest::pass (loc, "ASSERT_STR_CONTAINS");
+ pass (loc, "ASSERT_STR_CONTAINS");
else
- ::selftest::fail_formatted
+ fail_formatted
(loc, "ASSERT_STR_CONTAINS (%s, %s) haystack=\"%s\" needle=\"%s\"",
desc_haystack, desc_needle, val_haystack, val_needle);
}
/* Constructor. Generate a name for the file. */
-selftest::named_temp_file::named_temp_file (const char *suffix)
+named_temp_file::named_temp_file (const char *suffix)
{
m_filename = make_temp_file (suffix);
ASSERT_NE (m_filename, NULL);
@@ -130,7 +128,7 @@ selftest::named_temp_file::named_temp_file (const char *suffix)
/* Destructor. Delete the tempfile. */
-selftest::named_temp_file::~named_temp_file ()
+named_temp_file::~named_temp_file ()
{
unlink (m_filename);
diagnostics_file_cache_forcibly_evict_file (m_filename);
@@ -141,23 +139,20 @@ selftest::named_temp_file::~named_temp_file ()
it. Abort if anything goes wrong, using LOC as the effective
location in the problem report. */
-selftest::temp_source_file::temp_source_file (const location &loc,
- const char *suffix,
- const char *content)
+temp_source_file::temp_source_file (const location &loc,
+ const char *suffix,
+ const char *content)
: named_temp_file (suffix)
{
FILE *out = fopen (get_filename (), "w");
if (!out)
- ::selftest::fail_formatted (loc, "unable to open tempfile: %s",
- get_filename ());
+ fail_formatted (loc, "unable to open tempfile: %s", get_filename ());
fprintf (out, "%s", content);
fclose (out);
}
/* Selftests for the selftest system itself. */
-namespace selftest {
-
/* Sanity-check the ASSERT_ macros with various passing cases. */
static void
@@ -181,9 +176,8 @@ test_named_temp_file ()
named_temp_file t (".txt");
FILE *f = fopen (t.get_filename (), "w");
if (!f)
- selftest::fail_formatted (SELFTEST_LOCATION,
- "unable to open %s for writing",
- t.get_filename ());
+ fail_formatted (SELFTEST_LOCATION,
+ "unable to open %s for writing", t.get_filename ());
fclose (f);
}
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] selftest.c: avoid explicit "selftest::" qualifiers
2016-08-31 0:02 ` [PATCH] selftest.c: avoid explicit "selftest::" qualifiers David Malcolm
@ 2016-08-31 8:36 ` Bernd Schmidt
0 siblings, 0 replies; 31+ messages in thread
From: Bernd Schmidt @ 2016-08-31 8:36 UTC (permalink / raw)
To: David Malcolm, gcc-patches; +Cc: Richard Biener
On 08/31/2016 02:31 AM, David Malcolm wrote:
> The following follow-up patch removes them, moving the
> "namespace selftest {" to the top of the file so it covers
> everything.
LGTM.
Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/4] Improvements to typed_splay_tree
2016-08-24 1:00 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
@ 2016-08-31 14:12 ` Bernd Schmidt
2016-09-01 0:33 ` [PATCH] Improvements to typed_splay_tree (v2) David Malcolm
0 siblings, 1 reply; 31+ messages in thread
From: Bernd Schmidt @ 2016-08-31 14:12 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 08/24/2016 03:28 AM, David Malcolm wrote:
> +/* Helper type for typed_splay_tree::foreach. */
> +
> +template <typename KEY_TYPE, typename VALUE_TYPE>
> +struct closure
Is this in the global namespace? If so, something more specific than
"closure" might be more appropriate. Or move the struct into the
typed_splay_tree class definition.
Looks ok otherwise.
Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] Improvements to typed_splay_tree (v2)
2016-08-31 14:12 ` Bernd Schmidt
@ 2016-09-01 0:33 ` David Malcolm
2016-09-01 8:57 ` Bernd Schmidt
0 siblings, 1 reply; 31+ messages in thread
From: David Malcolm @ 2016-09-01 0:33 UTC (permalink / raw)
To: Bernd Schmidt, gcc-patches; +Cc: David Malcolm
On Wed, 2016-08-31 at 16:12 +0200, Bernd Schmidt wrote:
> On 08/24/2016 03:28 AM, David Malcolm wrote:
> > +/* Helper type for typed_splay_tree::foreach. */
> > +
> > +template <typename KEY_TYPE, typename VALUE_TYPE>
> > +struct closure
>
> Is this in the global namespace? If so, something more specific than
> "closure" might be more appropriate. Or move the struct into the
> typed_splay_tree class definition.
>
> Looks ok otherwise.
Thanks. Here's a revised version; I moved the "struct closure" into
the class itself.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/ChangeLog:
* Makefile.in (OBJS): Add typed-splay-tree.o.
* selftest-run-tests.c (selftest::run_tests): Call
typed_splay_tree_c_tests.
* selftest.h (typed_splay_tree_c_tests): New decl.
* typed-splay-tree.c: New file.
* typed-splay-tree.h (typed_splay_tree::foreach_fn): New typedef.
(typed_splay_tree::max): New method.
(typed_splay_tree::min): New method.
(typed_splay_tree::foreach): New method.
(typed_splay_tree::closure): New struct.
(typed_splay_tree::inner_foreach_fn): New function.
---
gcc/Makefile.in | 1 +
gcc/selftest-run-tests.c | 1 +
gcc/selftest.h | 1 +
gcc/typed-splay-tree.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
gcc/typed-splay-tree.h | 62 +++++++++++++++++++++++++++++++++++++
5 files changed, 144 insertions(+)
create mode 100644 gcc/typed-splay-tree.c
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index eb5ab61..b38a0c1 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1542,6 +1542,7 @@ OBJS = \
tree-vectorizer.o \
tree-vrp.o \
tree.o \
+ typed-splay-tree.o \
valtrack.o \
value-prof.o \
var-tracking.o \
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 6453e31..e20bbd0 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -56,6 +56,7 @@ selftest::run_tests ()
ggc_tests_c_tests ();
sreal_c_tests ();
fibonacci_heap_c_tests ();
+ typed_splay_tree_c_tests ();
/* Mid-level data structures. */
input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 47d7350..54a33f9 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -166,6 +166,7 @@ extern void selftest_c_tests ();
extern void spellcheck_c_tests ();
extern void spellcheck_tree_c_tests ();
extern void sreal_c_tests ();
+extern void typed_splay_tree_c_tests ();
extern void tree_c_tests ();
extern void tree_cfg_c_tests ();
extern void vec_c_tests ();
diff --git a/gcc/typed-splay-tree.c b/gcc/typed-splay-tree.c
new file mode 100644
index 0000000..33992c1
--- /dev/null
+++ b/gcc/typed-splay-tree.c
@@ -0,0 +1,79 @@
+/* Selftests for typed-splay-tree.h.
+ 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 "typed-splay-tree.h"
+#include "selftest.h"
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Callback for use by test_str_to_int. */
+
+static int
+append_cb (const char *, int value, void *user_data)
+{
+ auto_vec <int> *vec = (auto_vec <int> *)user_data;
+ vec->safe_push (value);
+ return 0;
+}
+
+/* Test of typed_splay_tree <const char *, int>. */
+
+static void
+test_str_to_int ()
+{
+ typed_splay_tree <const char *, int> t (strcmp, NULL, NULL);
+
+ t.insert ("a", 1);
+ t.insert ("b", 2);
+ t.insert ("c", 3);
+
+ ASSERT_EQ (1, t.lookup ("a"));
+ ASSERT_EQ (2, t.lookup ("b"));
+ ASSERT_EQ (3, t.lookup ("c"));
+
+ ASSERT_EQ (2, t.predecessor ("c"));
+ ASSERT_EQ (3, t.successor ("b"));
+ ASSERT_EQ (1, t.min ());
+ ASSERT_EQ (3, t.max ());
+
+ /* Test foreach by appending to a vec, and verifying the vec. */
+ auto_vec <int> v;
+ t.foreach (append_cb, &v);
+ ASSERT_EQ (3, v.length ());
+ ASSERT_EQ (1, v[0]);
+ ASSERT_EQ (2, v[1]);
+ ASSERT_EQ (3, v[2]);
+}
+
+/* Run all of the selftests within this file. */
+
+void
+typed_splay_tree_c_tests ()
+{
+ test_str_to_int ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/typed-splay-tree.h b/gcc/typed-splay-tree.h
index 9dd96d6..7b8afef 100644
--- a/gcc/typed-splay-tree.h
+++ b/gcc/typed-splay-tree.h
@@ -33,6 +33,7 @@ class typed_splay_tree
typedef int (*compare_fn) (key_type, key_type);
typedef void (*delete_key_fn) (key_type);
typedef void (*delete_value_fn) (value_type);
+ typedef int (*foreach_fn) (key_type, value_type, void *);
typed_splay_tree (compare_fn,
delete_key_fn,
@@ -43,8 +44,23 @@ class typed_splay_tree
value_type predecessor (key_type k);
value_type successor (key_type k);
void insert (key_type k, value_type v);
+ value_type max ();
+ value_type min ();
+ int foreach (foreach_fn, void *);
private:
+ /* Helper type for typed_splay_tree::foreach. */
+ struct closure
+ {
+ closure (foreach_fn outer_cb, void *outer_user_data)
+ : m_outer_cb (outer_cb), m_outer_user_data (outer_user_data) {}
+
+ foreach_fn m_outer_cb;
+ void *m_outer_user_data;
+ };
+
+ static int inner_foreach_fn (splay_tree_node node, void *user_data);
+
static value_type node_to_value (splay_tree_node node);
private:
@@ -120,6 +136,52 @@ typed_splay_tree<KEY_TYPE, VALUE_TYPE>::insert (key_type key,
(splay_tree_value)value);
}
+/* Get the value with maximal key. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline VALUE_TYPE
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::max ()
+{
+ return node_to_value (splay_tree_max (m_inner));
+}
+
+/* Get the value with minimal key. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline VALUE_TYPE
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::min ()
+{
+ return node_to_value (splay_tree_min (m_inner));
+}
+
+/* Call OUTER_CB, passing it the OUTER_USER_DATA, for every node,
+ following an in-order traversal. If OUTER_CB ever returns a non-zero
+ value, the iteration ceases immediately, and the value is returned.
+ Otherwise, this function returns 0. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+inline int
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::foreach (foreach_fn outer_cb,
+ void *outer_user_data)
+{
+ closure c (outer_cb, outer_user_data);
+
+ return splay_tree_foreach (m_inner, inner_foreach_fn, &c);
+}
+
+/* Helper function for typed_splay_tree::foreach. */
+
+template <typename KEY_TYPE, typename VALUE_TYPE>
+int
+typed_splay_tree<KEY_TYPE, VALUE_TYPE>::inner_foreach_fn (splay_tree_node node,
+ void *user_data)
+{
+ closure *c = (closure *)user_data;
+
+ return c->m_outer_cb ((KEY_TYPE)node->key, (VALUE_TYPE)node->value,
+ c->m_outer_user_data);
+}
+
/* Internal function for converting from splay_tree_node to
VALUE_TYPE. */
template <typename KEY_TYPE, typename VALUE_TYPE>
--
1.8.5.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Improvements to typed_splay_tree (v2)
2016-09-01 0:33 ` [PATCH] Improvements to typed_splay_tree (v2) David Malcolm
@ 2016-09-01 8:57 ` Bernd Schmidt
0 siblings, 0 replies; 31+ messages in thread
From: Bernd Schmidt @ 2016-09-01 8:57 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 09/01/2016 03:01 AM, David Malcolm wrote:
> On Wed, 2016-08-31 at 16:12 +0200, Bernd Schmidt wrote:
>> Looks ok otherwise.
>
> Thanks. Here's a revised version; I moved the "struct closure" into
> the class itself.
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
Sure.
Bernd
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch
2016-08-25 0:45 ` [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch David Malcolm
@ 2016-09-02 19:53 ` David Malcolm
0 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-09-02 19:53 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener
On Wed, 2016-08-24 at 21:13 -0400, David Malcolm wrote:
> Changed in v2: I dropped -fdiagnostics-apply-fixits
>
> This patch uses the edit_context machinery to provide a new
> -fdiagnostics-generate-patch option.
>
> If set an edit_context is created for global_dc, and any
> fix-it hints emitted by diagnostics are added to the edit_context.
>
> -fdiagnostics-generate-patch writes out a patch to stderr in unified
> diff format. This is potentially color-coded, following the same
> rules
> as for diagnostics (and affected by -fdiagnostics-color). The color
> scheme mimics that of git-diff.
> gcc/ChangeLog:
> * common.opt (fdiagnostics-generate-patch): New option.
> * diagnostic.c: Include "edit-context.h".
> (diagnostic_initialize): Initialize context->edit_context_ptr.
> (diagnostic_finish): Delete context->edit_context_ptr.
> (diagnostic_report_diagnostic): Add fix-it hints from the
> diagnostic to context->edit_context_ptr, if any.
> * diagnostic.h (class edit_context): Add forward decl.
> (struct diagnostic_context): Add field "edit_context_ptr".
> * doc/invoke.texi (Diagnostic Message Formatting Options): Add
> -fdiagnostics-generate-patch.
> (-fdiagnostics-generate-patch): New item.
> * toplev.c: Include "edit-context.h".
> (process_options): Set global_dc->edit_context_ptr to a new
> edit_context if the options need one.
> (toplev::main): Handle -fdiagnostics-generate-patch by using
> global_dc->edit_context_ptr.
>
> gcc/testsuite/ChangeLog:
> * gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c:
> New
> test case.
> * gcc.dg/plugin/plugin.exp (plugin_test_list): Add
> diagnostic-test-show-locus-generate-patch.c to the sources
> for diagnostic_plugin_test_show_locus.c.
The prerequisites are in, so I've committed this to trunk (as r239965), having verified bootstrap®rtest on x86_64-pc-linux-gnu.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] (v2) Generating patches from fix-it hints
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
` (3 preceding siblings ...)
2016-08-25 0:45 ` [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch David Malcolm
@ 2016-09-09 23:01 ` Jeff Law
2016-09-12 13:44 ` David Malcolm
4 siblings, 1 reply; 31+ messages in thread
From: Jeff Law @ 2016-09-09 23:01 UTC (permalink / raw)
To: David Malcolm, gcc-patches; +Cc: Richard Biener
On 08/24/2016 07:13 PM, David Malcolm wrote:
> Here's a much less ambitious version of the patch kit, which
> eliminates any attempt to write to the user's source
> code (getting rid of edit_context::apply_changes and
> -fdiagnostics-apply-fixits).
>
> It implements -fdiagnostics-generate-patch. In so doing, it
> tightens up the exact semantics of fix-its; see [1] for an
> example of where that's useful.
>
> I need review of at least patches 1 and 2 (which are unchanged
> from v1 of the kit). I believe I can self-approve patches 3
> and 4 as part of my "diagnostics maintainer" role; are they
> acceptable to those who objected to the earlier kit? (now
> that there's no attempt to write to source files).
>
> Successfully bootstrapped®rtested the combination of the patches
> on x86_64-pc-linux-gnu.
>
> OK for trunk? (assuming individual bootstraps®rtesting)
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01751.html
My understanding was #1 was approved by Bernd. I think the update to #1
which removes the unnecessary explicit namespaces is fine.
Have you addressed the question/concern for #2?
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02125.html
Jeff
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/4] (v2) Generating patches from fix-it hints
2016-09-09 23:01 ` [PATCH 0/4] (v2) Generating patches from fix-it hints Jeff Law
@ 2016-09-12 13:44 ` David Malcolm
0 siblings, 0 replies; 31+ messages in thread
From: David Malcolm @ 2016-09-12 13:44 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: Richard Biener
On Fri, 2016-09-09 at 16:56 -0600, Jeff Law wrote:
> On 08/24/2016 07:13 PM, David Malcolm wrote:
> > Here's a much less ambitious version of the patch kit, which
> > eliminates any attempt to write to the user's source
> > code (getting rid of edit_context::apply_changes and
> > -fdiagnostics-apply-fixits).
> >
> > It implements -fdiagnostics-generate-patch. In so doing, it
> > tightens up the exact semantics of fix-its; see [1] for an
> > example of where that's useful.
> >
> > I need review of at least patches 1 and 2 (which are unchanged
> > from v1 of the kit). I believe I can self-approve patches 3
> > and 4 as part of my "diagnostics maintainer" role; are they
> > acceptable to those who objected to the earlier kit? (now
> > that there's no attempt to write to source files).
> >
> > Successfully bootstrapped®rtested the combination of the patches
> > on x86_64-pc-linux-gnu.
> >
> > OK for trunk? (assuming individual bootstraps®rtesting)
> >
> > [1] https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01751.html
> My understanding was #1 was approved by Bernd. I think the update to
> #1
> which removes the unnecessary explicit namespaces is fine.
Thanks (Bernd also approved it as
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02109.html ;
I committed it as r239892.
> Have you addressed the question/concern for #2?
>
> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02125.html
Yes, in:
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00001.html
which Bernd approved in:
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00023.html
I committed it as r239958.
FWIW I've already committed the rest of the less-ambitious patch kit:
- patch 3 (edit_context) as r239963, and
- patch 4 (-fdiagnostics-generate-patch) as r239965
I'm experimenting with using edit_context to provide more readable
printing of fix-it hints in diagnostic-show-locus.c, and with the
support needed to add "break;\n" etc fix-its for -Wfallthrough (i.e.
how to cope with newlines and indentation).
Dave
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2016-09-12 13:24 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 0:59 [PATCH 0/4] Applying fix-its on behalf of the user to their code David Malcolm
2016-08-24 0:59 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
2016-08-24 1:00 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
2016-08-31 14:12 ` Bernd Schmidt
2016-09-01 0:33 ` [PATCH] Improvements to typed_splay_tree (v2) David Malcolm
2016-09-01 8:57 ` Bernd Schmidt
2016-08-24 1:00 ` [PATCH 4/4] Add -fdiagnostics-generate-patch and -fdiagnostics-apply-fixits David Malcolm
2016-08-24 1:00 ` [PATCH 3/4] Introduce class edit_context David Malcolm
2016-08-24 1:58 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Eric Gallager
2016-08-24 13:15 ` David Malcolm
2016-08-24 7:59 ` Richard Biener
2016-08-24 13:29 ` Trevor Saunders
2016-08-24 13:32 ` David Malcolm
2016-08-25 0:45 ` [PATCH 0/4] (v2) Generating patches from fix-it hints David Malcolm
2016-08-25 0:44 ` [PATCH 1/4] selftest: split out named_temp_file from temp_source_file David Malcolm
2016-08-29 21:53 ` Bernd Schmidt
2016-08-31 0:02 ` [PATCH] selftest.c: avoid explicit "selftest::" qualifiers David Malcolm
2016-08-31 8:36 ` Bernd Schmidt
2016-08-25 0:45 ` [PATCH 3/4] (v2) Introduce class edit_context David Malcolm
2016-08-28 15:05 ` Trevor Saunders
2016-08-29 18:53 ` David Malcolm
2016-08-30 2:51 ` Trevor Saunders
2016-08-25 0:45 ` [PATCH 2/4] Improvements to typed_splay_tree David Malcolm
2016-08-25 0:45 ` [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch David Malcolm
2016-09-02 19:53 ` David Malcolm
2016-09-09 23:01 ` [PATCH 0/4] (v2) Generating patches from fix-it hints Jeff Law
2016-09-12 13:44 ` David Malcolm
2016-08-24 13:45 ` [PATCH 0/4] Applying fix-its on behalf of the user to their code Bernd Schmidt
2016-08-24 13:56 ` Richard Biener
2016-08-24 21:52 ` Manuel López-Ibáñez
2016-08-25 9:20 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).