From a10631530f1719c92d70117455e2a207975b350b Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Tue, 11 Jul 2017 13:43:31 +0000 Subject: [PATCH] diagnostics: support compact printing of secondary locations gcc/ChangeLog: * diagnostic-show-locus.c: Include "gcc-rich-location.h". (layout::m_primary_loc): New field. (layout::layout): Initialize new field. Move location filtering logic from here to... (layout::maybe_add_location_range): ...this new method. Add support for filtering to just the lines already specified by other locations. (layout::will_show_line_p): New method. (selftest::test_add_location_if_nearby): New test function. (selftest::diagnostic_show_locus_c_tests): Call it. * gcc-rich-location.h (gcc_rich_location::add_location_if_nearby): New method. From-SVN: r250133 --- gcc/ChangeLog | 15 ++ gcc/diagnostic-show-locus.c | 273 +++++++++++++++++++++++++++--------- gcc/gcc-rich-location.h | 21 +++ 3 files changed, 243 insertions(+), 66 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c960cd76098..c971e8abccb 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,18 @@ +2017-07-11 David Malcolm + + * diagnostic-show-locus.c: Include "gcc-rich-location.h". + (layout::m_primary_loc): New field. + (layout::layout): Initialize new field. Move location filtering + logic from here to... + (layout::maybe_add_location_range): ...this new method. Add + support for filtering to just the lines already specified by other + locations. + (layout::will_show_line_p): New method. + (selftest::test_add_location_if_nearby): New test function. + (selftest::diagnostic_show_locus_c_tests): Call it. + * gcc-rich-location.h (gcc_rich_location::add_location_if_nearby): + New method. + 2017-07-11 Tom de Vries * config/nvptx/nvptx.c (WORKAROUND_PTXJIT_BUG): New macro. diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index 8a4fd5f17dc..5227400b716 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -27,6 +27,7 @@ along with GCC; see the file COPYING3. If not see #include "backtrace.h" #include "diagnostic.h" #include "diagnostic-color.h" +#include "gcc-rich-location.h" #include "selftest.h" #ifdef HAVE_TERMIOS_H @@ -196,6 +197,9 @@ class layout rich_location *richloc, diagnostic_t diagnostic_kind); + bool maybe_add_location_range (const location_range *loc_range, + bool restrict_to_current_line_spans); + int get_num_line_spans () const { return m_line_spans.length (); } const line_span *get_line_span (int idx) const { return &m_line_spans[idx]; } @@ -206,6 +210,7 @@ class layout void print_line (int row); private: + bool will_show_line_p (int row) const; void print_leading_fixits (int row); void print_source_line (int row, const char *line, int line_width, line_bounds *lbounds_out); @@ -241,6 +246,7 @@ class layout diagnostic_context *m_context; pretty_printer *m_pp; diagnostic_t m_diagnostic_kind; + location_t m_primary_loc; expanded_location m_exploc; colorizer m_colorizer; bool m_colorize_source_p; @@ -767,6 +773,7 @@ layout::layout (diagnostic_context * context, : m_context (context), m_pp (context->printer), m_diagnostic_kind (diagnostic_kind), + m_primary_loc (richloc->get_range (0)->m_loc), m_exploc (richloc->get_expanded_location (0)), m_colorizer (context, diagnostic_kind), m_colorize_source_p (context->colorize_source_p), @@ -775,77 +782,12 @@ layout::layout (diagnostic_context * context, m_line_spans (1 + richloc->get_num_locations ()), m_x_offset (0) { - source_location primary_loc = richloc->get_range (0)->m_loc; - for (unsigned int idx = 0; idx < richloc->get_num_locations (); idx++) { /* This diagnostic printer can only cope with "sufficiently sane" ranges. Ignore any ranges that are awkward to handle. */ const location_range *loc_range = richloc->get_range (idx); - - /* Split the "range" into caret and range information. */ - source_range src_range = get_range_from_loc (line_table, loc_range->m_loc); - - /* Expand the various locations. */ - expanded_location start - = linemap_client_expand_location_to_spelling_point - (src_range.m_start, LOCATION_ASPECT_START); - expanded_location finish - = linemap_client_expand_location_to_spelling_point - (src_range.m_finish, LOCATION_ASPECT_FINISH); - expanded_location caret - = linemap_client_expand_location_to_spelling_point - (loc_range->m_loc, LOCATION_ASPECT_CARET); - - /* If any part of the range isn't in the same file as the primary - location of this diagnostic, ignore the range. */ - if (start.file != m_exploc.file) - continue; - if (finish.file != m_exploc.file) - continue; - if (loc_range->m_show_caret_p) - if (caret.file != m_exploc.file) - continue; - - /* Sanitize the caret location for non-primary ranges. */ - if (m_layout_ranges.length () > 0) - if (loc_range->m_show_caret_p) - if (!compatible_locations_p (loc_range->m_loc, primary_loc)) - /* Discard any non-primary ranges that can't be printed - sanely relative to the primary location. */ - continue; - - /* Everything is now known to be in the correct source file, - but it may require further sanitization. */ - layout_range ri (&start, &finish, loc_range->m_show_caret_p, &caret); - - /* If we have a range that finishes before it starts (perhaps - from something built via macro expansion), printing the - range is likely to be nonsensical. Also, attempting to do so - breaks assumptions within the printing code (PR c/68473). - Similarly, don't attempt to print ranges if one or both ends - of the range aren't sane to print relative to the - primary location (PR c++/70105). */ - if (start.line > finish.line - || !compatible_locations_p (src_range.m_start, primary_loc) - || !compatible_locations_p (src_range.m_finish, primary_loc)) - { - /* Is this the primary location? */ - if (m_layout_ranges.length () == 0) - { - /* We want to print the caret for the primary location, but - we must sanitize away m_start and m_finish. */ - ri.m_start = ri.m_caret; - ri.m_finish = ri.m_caret; - } - else - /* This is a non-primary range; ignore it. */ - continue; - } - - /* Passed all the tests; add the range to m_layout_ranges so that - it will be printed. */ - m_layout_ranges.safe_push (ri); + maybe_add_location_range (loc_range, false); } /* Populate m_fixit_hints, filtering to only those that are in the @@ -882,6 +824,118 @@ layout::layout (diagnostic_context * context, show_ruler (m_x_offset + max_width); } +/* Attempt to add LOC_RANGE to m_layout_ranges, filtering them to + those that we can sanely print. + + If RESTRICT_TO_CURRENT_LINE_SPANS is true, then LOC_RANGE is also + filtered against this layout instance's current line spans: it + will only be added if the location is fully within the lines + already specified by other locations. + + Return true iff LOC_RANGE was added. */ + +bool +layout::maybe_add_location_range (const location_range *loc_range, + bool restrict_to_current_line_spans) +{ + gcc_assert (loc_range); + + /* Split the "range" into caret and range information. */ + source_range src_range = get_range_from_loc (line_table, loc_range->m_loc); + + /* Expand the various locations. */ + expanded_location start + = linemap_client_expand_location_to_spelling_point + (src_range.m_start, LOCATION_ASPECT_START); + expanded_location finish + = linemap_client_expand_location_to_spelling_point + (src_range.m_finish, LOCATION_ASPECT_FINISH); + expanded_location caret + = linemap_client_expand_location_to_spelling_point + (loc_range->m_loc, LOCATION_ASPECT_CARET); + + /* If any part of the range isn't in the same file as the primary + location of this diagnostic, ignore the range. */ + if (start.file != m_exploc.file) + return false; + if (finish.file != m_exploc.file) + return false; + if (loc_range->m_show_caret_p) + if (caret.file != m_exploc.file) + return false; + + /* Sanitize the caret location for non-primary ranges. */ + if (m_layout_ranges.length () > 0) + if (loc_range->m_show_caret_p) + if (!compatible_locations_p (loc_range->m_loc, m_primary_loc)) + /* Discard any non-primary ranges that can't be printed + sanely relative to the primary location. */ + return false; + + /* Everything is now known to be in the correct source file, + but it may require further sanitization. */ + layout_range ri (&start, &finish, loc_range->m_show_caret_p, &caret); + + /* If we have a range that finishes before it starts (perhaps + from something built via macro expansion), printing the + range is likely to be nonsensical. Also, attempting to do so + breaks assumptions within the printing code (PR c/68473). + Similarly, don't attempt to print ranges if one or both ends + of the range aren't sane to print relative to the + primary location (PR c++/70105). */ + if (start.line > finish.line + || !compatible_locations_p (src_range.m_start, m_primary_loc) + || !compatible_locations_p (src_range.m_finish, m_primary_loc)) + { + /* Is this the primary location? */ + if (m_layout_ranges.length () == 0) + { + /* We want to print the caret for the primary location, but + we must sanitize away m_start and m_finish. */ + ri.m_start = ri.m_caret; + ri.m_finish = ri.m_caret; + } + else + /* This is a non-primary range; ignore it. */ + return false; + } + + /* Potentially filter to just the lines already specified by other + locations. This is for use by gcc_rich_location::add_location_if_nearby. + The layout ctor doesn't use it, and can't because m_line_spans + hasn't been set up at that point. */ + if (restrict_to_current_line_spans) + { + if (!will_show_line_p (start.line)) + return false; + if (!will_show_line_p (finish.line)) + return false; + if (loc_range->m_show_caret_p) + if (!will_show_line_p (caret.line)) + return false; + } + + /* Passed all the tests; add the range to m_layout_ranges so that + it will be printed. */ + m_layout_ranges.safe_push (ri); + return true; +} + +/* Return true iff ROW is within one of the line spans for this layout. */ + +bool +layout::will_show_line_p (int row) const +{ + for (int line_span_idx = 0; line_span_idx < get_num_line_spans (); + line_span_idx++) + { + const line_span *line_span = get_line_span (line_span_idx); + if (line_span->contains_line_p (row)) + return true; + } + return false; +} + /* Return true iff we should print a heading when starting the line span with the given index. */ @@ -1782,6 +1836,28 @@ layout::print_line (int row) } /* End of anonymous namespace. */ +/* If LOC is within the spans of lines that will already be printed for + this gcc_rich_location, then add it as a secondary location and return true. + + Otherwise return false. */ + +bool +gcc_rich_location::add_location_if_nearby (location_t loc) +{ + /* Use the layout location-handling logic to sanitize LOC, + filtering it to the current line spans within a temporary + layout instance. */ + layout layout (global_dc, this, DK_ERROR); + location_range loc_range; + loc_range.m_loc = loc; + loc_range.m_show_caret_p = false; + if (!layout.maybe_add_location_range (&loc_range, true)) + return false; + + add_range (loc, false); + return true; +} + /* Print the physical source code corresponding to the location of this diagnostic, with additional annotations. */ @@ -2226,6 +2302,70 @@ test_diagnostic_show_locus_one_liner (const line_table_case &case_) test_one_liner_many_fixits_2 (); } +/* Verify that gcc_rich_location::add_location_if_nearby works. */ + +static void +test_add_location_if_nearby (const line_table_case &case_) +{ + /* Create a tempfile and write some text to it. + ...000000000111111111122222222223333333333. + ...123456789012345678901234567890123456789. */ + const char *content + = ("struct same_line { double x; double y; ;\n" /* line 1. */ + "struct different_line\n" /* line 2. */ + "{\n" /* line 3. */ + " double x;\n" /* line 4. */ + " double y;\n" /* line 5. */ + ";\n"); /* line 6. */ + temp_source_file tmp (SELFTEST_LOCATION, ".c", content); + line_table_test ltt (case_); + + const line_map_ordinary *ord_map + = linemap_check_ordinary (linemap_add (line_table, LC_ENTER, false, + tmp.get_filename (), 0)); + + linemap_line_start (line_table, 1, 100); + + const location_t final_line_end + = linemap_position_for_line_and_column (line_table, ord_map, 6, 7); + + /* Don't attempt to run the tests if column data might be unavailable. */ + if (final_line_end > LINE_MAP_MAX_LOCATION_WITH_COLS) + return; + + /* Test of add_location_if_nearby on the same line as the + primary location. */ + { + const location_t missing_close_brace_1_39 + = linemap_position_for_line_and_column (line_table, ord_map, 1, 39); + const location_t matching_open_brace_1_18 + = linemap_position_for_line_and_column (line_table, ord_map, 1, 18); + gcc_rich_location richloc (missing_close_brace_1_39); + bool added = richloc.add_location_if_nearby (matching_open_brace_1_18); + ASSERT_TRUE (added); + ASSERT_EQ (2, richloc.get_num_locations ()); + test_diagnostic_context dc; + diagnostic_show_locus (&dc, &richloc, DK_ERROR); + ASSERT_STREQ ("\n" + " struct same_line { double x; double y; ;\n" + " ~ ^\n", + pp_formatted_text (dc.printer)); + } + + /* Test of add_location_if_nearby on a different line to the + primary location. */ + { + const location_t missing_close_brace_6_1 + = linemap_position_for_line_and_column (line_table, ord_map, 6, 1); + const location_t matching_open_brace_3_1 + = linemap_position_for_line_and_column (line_table, ord_map, 3, 1); + gcc_rich_location richloc (missing_close_brace_6_1); + bool added = richloc.add_location_if_nearby (matching_open_brace_3_1); + ASSERT_FALSE (added); + ASSERT_EQ (1, richloc.get_num_locations ()); + } +} + /* Verify that we print fixits even if they only affect lines outside those covered by the ranges in the rich_location. */ @@ -2857,6 +2997,7 @@ diagnostic_show_locus_c_tests () test_diagnostic_show_locus_unknown_location (); for_each_line_table_case (test_diagnostic_show_locus_one_liner); + for_each_line_table_case (test_add_location_if_nearby); for_each_line_table_case (test_diagnostic_show_locus_fixit_lines); for_each_line_table_case (test_fixit_consolidation); for_each_line_table_case (test_overlapped_fixit_printing); diff --git a/gcc/gcc-rich-location.h b/gcc/gcc-rich-location.h index 49708cabf17..2720f38d0eb 100644 --- a/gcc/gcc-rich-location.h +++ b/gcc/gcc-rich-location.h @@ -40,6 +40,27 @@ class gcc_rich_location : public rich_location void add_fixit_misspelled_id (location_t misspelled_token_loc, tree hint_id); + + /* If LOC is within the spans of lines that will already be printed for + this gcc_rich_location, then add it as a secondary location + and return true. + + Otherwise return false. + + This allows for a diagnostic to compactly print secondary locations + in one diagnostic when these are near enough the primary locations for + diagnostics-show-locus.c to cope with them, and to fall back to + printing them via a note otherwise e.g.: + + gcc_rich_location richloc (primary_loc); + bool added secondary = richloc.add_location_if_nearby (secondary_loc); + error_at_rich_loc (&richloc, "main message"); + if (!added secondary) + inform (secondary_loc, "message for secondary"); + + Implemented in diagnostic-show-locus.c. */ + + bool add_location_if_nearby (location_t loc); }; #endif /* GCC_RICH_LOCATION_H */ -- 2.30.2