diagnostics: support compact printing of secondary locations
authorDavid Malcolm <dmalcolm@redhat.com>
Tue, 11 Jul 2017 13:43:31 +0000 (13:43 +0000)
committerDavid Malcolm <dmalcolm@gcc.gnu.org>
Tue, 11 Jul 2017 13:43:31 +0000 (13:43 +0000)
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
gcc/diagnostic-show-locus.c
gcc/gcc-rich-location.h

index c960cd76098e43d47b9786bff974e7d1e749b2b8..c971e8abccb0f1bfb370d2fbefcaa158d5aca299 100644 (file)
@@ -1,3 +1,18 @@
+2017-07-11  David Malcolm  <dmalcolm@redhat.com>
+
+       * 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  <tom@codesourcery.com>
 
        * config/nvptx/nvptx.c (WORKAROUND_PTXJIT_BUG): New macro.
index 8a4fd5f17dc8a4e41c91f85d4ebb6333faae23dc..5227400b716a691937e3ca728ffa38cd3d6c7eba 100644 (file)
@@ -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);
index 49708cabf17cc8dae831501e3f78cfc53f368e79..2720f38d0ebe534c13fced434660183bab58ad22 100644 (file)
@@ -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 */