diagnostic-show-locus.c: rework handling of multiple labels

This patch improves the handling of large numbers of labels within a
rich_location: previously, overlapping labels could lead to an assertion
failure within layout::print_any_labels.  Also, the labels were printed
in reverse order of insertion into the rich_location.

This patch moves the determination of whether a vertical bar should
be printed for a line_label into the
  'Figure out how many "label lines" we need, and which
   one each label is printed in.'
step of layout::print_any_labels, rather than doing it as the lines
are printed.  It also flips the sort order, so that labels at the
same line/column are printed in order of insertion into the
rich_location.

I haven't run into these issues with our existing diagnostics, but it
affects a patch kit I'm working on that makes more extensive use of
labels.

gcc/ChangeLog:
	* diagnostic-show-locus.c (line_label::line_label): Initialize
	m_has_vbar.
	(line_label::comparator): Reverse the sort order by m_state_idx,
	so that when the list is walked backwards the labels appear in
	order of insertion into the rich_location.
	(line_label::m_has_vbar): New field.
	(layout::print_any_labels): When dealing with multiple labels at
	the same line and column, only print vertical bars for the one
	with the highest label_line.
	(selftest::test_one_liner_labels): Update test for multiple labels
	to expect the labels to be in the order of insertion into the
	rich_location.  Add a test for many such labels, where the column
	numbers are out-of-order relative to the insertion order.

From-SVN: r276371

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f665a5c684c6a73a0d651a225b916706c2b54f09..5c3e3e1a0c7283dee32f5554a62ba016c1a15481 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,19 @@
+2019-09-30  David Malcolm  <dmalcolm@redhat.com>
+
+       * diagnostic-show-locus.c (line_label::line_label): Initialize
+       m_has_vbar.
+       (line_label::comparator): Reverse the sort order by m_state_idx,
+       so that when the list is walked backwards the labels appear in
+       order of insertion into the rich_location.
+       (line_label::m_has_vbar): New field.
+       (layout::print_any_labels): When dealing with multiple labels at
+       the same line and column, only print vertical bars for the one
+       with the highest label_line.
+       (selftest::test_one_liner_labels): Update test for multiple labels
+       to expect the labels to be in the order of insertion into the
+       rich_location.  Add a test for many such labels, where the column
+       numbers are out-of-order relative to the insertion order.
+
 2019-09-30  Richard Sandiford  <richard.sandiford@arm.com>
 
        * config/i386/i386.h (ix86_frame::expensive_p): New field.

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 4d563dda8f420d22e97836a95be6b3709d0e90dc..6612cbb6a9311520db4b246f41d6212b00ccdb9e 100644 (file)
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -1416,7 +1416,7 @@ public:
   line_label (int state_idx, int column, label_text text)
   : m_state_idx (state_idx), m_column (column),
     m_text (text), m_length (strlen (text.m_buffer)),
-    m_label_line (0)
+    m_label_line (0), m_has_vbar (true)
   {}
 
   /* Sorting is primarily by column, then by state index.  */
@@ -1427,7 +1427,10 @@ public:
     int column_cmp = compare (ll1->m_column, ll2->m_column);
     if (column_cmp)
       return column_cmp;
-    return compare (ll1->m_state_idx, ll2->m_state_idx);
+    /* Order by reverse state index, so that labels are printed
+       in order of insertion into the rich_location when the
+       sorted list is walked backwards.  */
+    return -compare (ll1->m_state_idx, ll2->m_state_idx);
   }
 
   int m_state_idx;
@@ -1435,6 +1438,7 @@ public:
   label_text m_text;
   size_t m_length;
   int m_label_line;
+  bool m_has_vbar;
 };
 
 /* Print any labels in this row.  */
@@ -1511,8 +1515,8 @@ layout::print_any_labels (linenum_type row)
        foo + bar
            ^               : label line 0
            |               : label line 1
-           label 1         : label line 2
-           label 0         : label line 3.  */
+           label 0         : label line 2
+           label 1         : label line 3.  */
 
   int max_label_line = 1;
   {
@@ -1522,7 +1526,15 @@ layout::print_any_labels (linenum_type row)
       {
        /* Would this label "touch" or overlap the next label?  */
        if (label->m_column + label->m_length >= (size_t)next_column)
-         max_label_line++;
+         {
+           max_label_line++;
+
+           /* If we've already seen labels with the same column, suppress the
+              vertical bar for subsequent ones in this backwards iteration;
+              hence only the one with the highest label_line has m_has_vbar set.  */
+           if (label->m_column == next_column)
+             label->m_has_vbar = false;
+         }
 
        label->m_label_line = max_label_line;
        next_column = label->m_column;
@@ -1533,10 +1545,6 @@ layout::print_any_labels (linenum_type row)
      either a vertical bar ('|') for the labels that are lower down, or the
      labels themselves once we've reached their line.  */
   {
-    /* Keep track of in which column we last printed a vertical bar.
-       This allows us to suppress duplicate vertical bars for the case
-       where multiple labels are on one column.  */
-    int last_vbar = 0;
     for (int label_line = 0; label_line <= max_label_line; label_line++)
       {
        start_annotation_line ();
@@ -1558,14 +1566,13 @@ layout::print_any_labels (linenum_type row)
                m_colorizer.set_normal_text ();
                column += label->m_length;
              }
-           else if (label->m_column != last_vbar)
+           else if (label->m_has_vbar)
              {
                gcc_assert (column <= label->m_column);
                move_to_column (&column, label->m_column, true);
                m_colorizer.set_range (label->m_state_idx);
                pp_character (m_pp, '|');
                m_colorizer.set_normal_text ();
-               last_vbar = column;
                column++;
              }
          }
@@ -2783,9 +2790,51 @@ test_one_liner_labels ()
                  " foo = bar.field;\n"
                  "       ^~~\n"
                  "       |\n"
-                 "       label 2\n"
+                 "       label 0\n"
                  "       label 1\n"
-                 "       label 0\n",
+                 "       label 2\n",
+                 pp_formatted_text (dc.printer));
+  }
+
+  /* Example of out-of-order ranges (thus requiring a sort), where
+     they overlap, and there are multiple ranges on the same point.  */
+  {
+    text_range_label label_0a ("label 0a");
+    text_range_label label_1a ("label 1a");
+    text_range_label label_2a ("label 2a");
+    text_range_label label_0b ("label 0b");
+    text_range_label label_1b ("label 1b");
+    text_range_label label_2b ("label 2b");
+    text_range_label label_0c ("label 0c");
+    text_range_label label_1c ("label 1c");
+    text_range_label label_2c ("label 2c");
+    gcc_rich_location richloc (field, &label_0a);
+    richloc.add_range (bar, SHOW_RANGE_WITHOUT_CARET, &label_1a);
+    richloc.add_range (foo, SHOW_RANGE_WITHOUT_CARET, &label_2a);
+
+    richloc.add_range (field, SHOW_RANGE_WITHOUT_CARET, &label_0b);
+    richloc.add_range (bar, SHOW_RANGE_WITHOUT_CARET, &label_1b);
+    richloc.add_range (foo, SHOW_RANGE_WITHOUT_CARET, &label_2b);
+
+    richloc.add_range (field, SHOW_RANGE_WITHOUT_CARET, &label_0c);
+    richloc.add_range (bar, SHOW_RANGE_WITHOUT_CARET, &label_1c);
+    richloc.add_range (foo, SHOW_RANGE_WITHOUT_CARET, &label_2c);
+
+    test_diagnostic_context dc;
+    diagnostic_show_locus (&dc, &richloc, DK_ERROR);
+    ASSERT_STREQ ("\n"
+                 " foo = bar.field;\n"
+                 " ~~~   ~~~ ^~~~~\n"
+                 " |     |   |\n"
+                 " |     |   label 0a\n"
+                 " |     |   label 0b\n"
+                 " |     |   label 0c\n"
+                 " |     label 1a\n"
+                 " |     label 1b\n"
+                 " |     label 1c\n"
+                 " label 2a\n"
+                 " label 2b\n"
+                 " label 2c\n",
                  pp_formatted_text (dc.printer));
   }