From 5a05b737e1b626f92ee968f6e32715bf955dae54 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 30 Sep 2019 20:03:55 +0000 Subject: [PATCH] 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 --- gcc/ChangeLog | 16 ++++++++ gcc/diagnostic-show-locus.c | 75 ++++++++++++++++++++++++++++++------- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index f665a5c684c..5c3e3e1a0c7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,19 @@ +2019-09-30 David Malcolm + + * 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 * 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 4d563dda8f4..6612cbb6a93 100644 --- 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)); } -- 2.30.2