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&regrtested 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

Reply via email to