On 11/10/2015 09:35 AM, David Malcolm wrote:
This patch adds the ability to add "fix-it hints" to a rich_location,
which will be displayed when the corresponding diagnostic is printed.
It does not actually add any fix-it hints (that comes in the second
patch), but it adds test coverage of the machinery and printing,
by using the existing diagnostic_plugin_test_show_locus to inject
some meaningless fixit hints, and to verify the output.
For now, add a nasty linker kludge in layout::print_any_fixits for
the sake of diagnostic_plugin_test_show_locus.
Successfully bootstrapped®rtested the pair of patches on
x86_64-pc-linux-gnu (on top of the 10-patch diagnostics kit).
OK for trunk?
gcc/ChangeLog:
PR/62314
* diagnostic-show-locus.c (colorizer::set_fixit_hint): New.
(class layout): Update comment
(layout::print_any_fixits): New method.
(layout::move_to_column): New method.
(diagnostic_show_locus): Add call to layout.print_any_fixits.
gcc/testsuite/ChangeLog:
PR/62314
* gcc.dg/plugin/diagnostic-test-show-locus-ascii-bw.c
(test_fixit_insert): New.
(test_fixit_remove): New.
(test_fixit_replace): New.
* gcc.dg/plugin/diagnostic-test-show-locus-ascii-color.c
(test_fixit_insert): New.
(test_fixit_remove): New.
(test_fixit_replace): New.
* gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
(test_show_locus): Add tests of rendering fixit hints.
libcpp/ChangeLog:
PR/62314
* include/line-map.h (source_range::intersects_line_p): New
method.
(rich_location::~rich_location): New.
(rich_location::add_fixit_insert): New method.
(rich_location::add_fixit_remove): New method.
(rich_location::add_fixit_replace): New method.
(rich_location::get_num_fixit_hints): New accessor.
(rich_location::get_fixit_hint): New accessor.
(rich_location::MAX_FIXIT_HINTS): New constant.
(rich_location::m_num_fixit_hints): New field.
(rich_location::m_fixit_hints): New field.
(class fixit_hint): New class.
(class fixit_insert): New class.
(class fixit_remove): New class.
(class fixit_replace): New class.
* line-map.c (source_range::intersects_line_p): New method.
(rich_location::rich_location): Add initialization of
m_num_fixit_hints to both ctors.
(rich_location::~rich_location): New.
(rich_location::add_fixit_insert): New method.
(rich_location::add_fixit_remove): New method.
(rich_location::add_fixit_replace): New method.
(fixit_insert::fixit_insert): New.
(fixit_insert::~fixit_insert): New.
(fixit_insert::affects_line_p): New.
(fixit_remove::fixit_remove): New.
(fixit_remove::affects_line_p): New.
(fixit_replace::fixit_replace): New.
(fixit_replace::~fixit_replace): New.
(fixit_replace::affects_line_p): New.
+
+ /* Nasty workaround to convince the linker to add
+ rich_location::add_fixit_insert
+ rich_location::add_fixit_remove
+ rich_location::add_fixit_replace
+ to cc1 for use by diagnostic_plugin_test_show_locus,
+ before anything in cc1 is using them.
+
+ This conditional should never hold, but hopefully the compiler can't
+ figure that out. */
+ if ('.' == global_dc->caret_chars[0])
+ {
+ rich_location dummy (line_table, UNKNOWN_LOCATION);
+ dummy.add_fixit_insert (UNKNOWN_LOCATION, "");
+ dummy.add_fixit_remove
+ (source_range::from_location (UNKNOWN_LOCATION));
+ dummy.add_fixit_replace
+ (source_range::from_location (UNKNOWN_LOCATION), "");
+ }
+}
So "the kludge" is presumably going to be removed based on later
comments in this patch's thread.
What is the purpose of the #if 0 code in the various tests? Did you
mean to leave those in?
I think this is probably OK. Though I am concerned that with blobs of
the tests commented out that it's not being tested as thoroughly as you
think it has :-)
Jeff