From ee908516796887afcaa1d9fabac80eae5a16c047 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 26 Aug 2016 21:25:41 +0000 Subject: Add validation and consolidation of fix-it hints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first aspect of this patch is to add some checking of fix-it hints. The idea is to put this checking within the rich_location machinery, rather than requiring every diagnostic to implement it for itself. The fixits within a rich_location are "atomic": all must be valid for any to be applicable. We reject any fixits involving locations above LINE_MAP_MAX_LOCATION_WITH_COLS. There's no guarantee that it's sane to modify a macro, so we reject any fix-its that touch them. For example, note the attempt to provide a fix-it for the definition of the macro FIELD: spellcheck-fields-2.c: In function ‘test_macro’: spellcheck-fields-2.c:26:15: error: ‘union u’ has no member named ‘colour’; did you mean ‘color’? #define FIELD colour ^ color spellcheck-fields-2.c:27:15: note: in expansion of macro ‘FIELD’ return ptr->FIELD; ^~~~~ After this patch, the fixit is not displayed: spellcheck-fields-2.c: In function ‘test_macro’: spellcheck-fields-2.c:26:15: error: ‘union u’ has no member named ‘colour’; did you mean ‘color’? #define FIELD colour ^ spellcheck-fields-2.c:27:15: note: in expansion of macro ‘FIELD’ return ptr->FIELD; ^~~~~ We might want some way for a diagnostic to opt-in to fix-its that affect macros, but for now it's simplest to reject them. The other aspect of this patch is fix-it consolidation: in some cases neighboring fix-its can be merged. For example, in a diagnostic to modernize old-style struct initializers from: struct s example = { - foo: 1, + .foo = 1, }; one approach would be to replace the "foo" with ".foo" and the ":" with " =". This would give two "replace" fix-its: foo: 1, --- FIXIT 1 .foo - FIXIT 2 = This patch allows them to be consolidated into a single "replace" fix-it: foo: 1, ---- .foo = gcc/ChangeLog: * diagnostic-show-locus.c (selftest::test_fixit_consolidation): New function. (selftest::diagnostic_show_locus_c_tests): Call it. * gcc-rich-location.h (gcc_rich_location): Eliminate unused constructor based on source_range. gcc/testsuite/ChangeLog: * gcc.dg/spellcheck-fields-2.c (test): Move dg-begin/end-multiline-output within function body. (test_macro): New function. libcpp/ChangeLog: * include/line-map.h (rich_location): Eliminate unimplemented constructor based on source_range. (rich_location::get_last_fixit_hint): New method. (rich_location::reject_impossible_fixit): New method. (rich_location): Add fields m_line_table and m_seen_impossible_fixit. (fixit_hint::maybe_append_replace): New pure virtual function. (fixit_insert::maybe_append_replace): New function. (fixit_replace::maybe_append_replace): New function. * line-map.c (rich_location::rich_location): Initialize m_line_table and m_seen_impossible_fixit. (rich_location::add_fixit_insert): Call reject_impossible_fixit and bail out if true. (column_before_p): New function. (rich_location::add_fixit_replace): Call reject_impossible_fixit and bail out if true. Attempt to consolidate with neighboring fixits. (rich_location::get_last_fixit_hint): New method. (rich_location::reject_impossible_fixit): New method. (fixit_insert::maybe_append_replace): New method. (fixit_replace::maybe_append_replace): New method. From-SVN: r239789 --- libcpp/include/line-map.h | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'libcpp/include/line-map.h') diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index a2ed008..0fc4848 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -1367,9 +1367,6 @@ class rich_location /* Constructing from a location. */ rich_location (line_maps *set, source_location loc); - /* Constructing from a source_range. */ - rich_location (source_range src_range); - /* Destructor. */ ~rich_location (); @@ -1411,12 +1408,17 @@ class rich_location unsigned int get_num_fixit_hints () const { return m_num_fixit_hints; } fixit_hint *get_fixit_hint (int idx) const { return m_fixit_hints[idx]; } + fixit_hint *get_last_fixit_hint () const; + +private: + bool reject_impossible_fixit (source_location where); public: static const int MAX_RANGES = 3; static const int MAX_FIXIT_HINTS = 2; protected: + line_maps *m_line_table; unsigned int m_num_ranges; location_range m_ranges[MAX_RANGES]; @@ -1427,6 +1429,7 @@ protected: unsigned int m_num_fixit_hints; fixit_hint *m_fixit_hints[MAX_FIXIT_HINTS]; + bool m_seen_impossible_fixit; }; class fixit_hint @@ -1440,6 +1443,10 @@ public: virtual bool affects_line_p (const char *file, int line) = 0; virtual source_location get_start_loc () const = 0; virtual bool maybe_get_end_loc (source_location *out) const = 0; + /* Vfunc for consolidating successor fixits. */ + virtual bool maybe_append_replace (line_maps *set, + source_range src_range, + const char *new_content) = 0; }; class fixit_insert : public fixit_hint @@ -1452,6 +1459,9 @@ class fixit_insert : public fixit_hint bool affects_line_p (const char *file, int line); source_location get_start_loc () const { return m_where; } bool maybe_get_end_loc (source_location *) const { return false; } + bool maybe_append_replace (line_maps *set, + source_range src_range, + const char *new_content); source_location get_location () const { return m_where; } const char *get_string () const { return m_bytes; } @@ -1478,6 +1488,9 @@ class fixit_replace : public fixit_hint *out = m_src_range.m_finish; return true; } + bool maybe_append_replace (line_maps *set, + source_range src_range, + const char *new_content); source_range get_range () const { return m_src_range; } const char *get_string () const { return m_bytes; } -- cgit v1.1