From 17bdbbdf5b00a5e340ba8902a371c8f42cc0a4ed Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 9 Aug 2018 12:55:59 -0400
Subject: [PATCH 05/18] FIXME: WIP on eliding location prefixes for notes (PR
 other/84889)

FIXME: but what about i18n in selftests???
---
 gcc/common.opt              |   5 +-
 gcc/diagnostic-show-locus.c | 135 +++++++++++++++++++++++++++++++++++++++++++-
 gcc/diagnostic.c            |  70 ++++++++++++++++++++++-
 gcc/diagnostic.h            |   8 +++
 gcc/doc/invoke.texi         |  10 +++-
 gcc/pretty-print.c          |   7 ++-
 gcc/pretty-print.h          |  22 +++++---
 gcc/selftest-diagnostic.c   |  21 +++++++
 gcc/selftest-diagnostic.h   |  16 ++++++
 gcc/testsuite/lib/prune.exp |   1 +
 10 files changed, 279 insertions(+), 16 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index a08555f..486f69a7 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1214,7 +1214,7 @@ Try to convert virtual calls to direct ones.
 
 fdiagnostics-show-location=
 Common Joined RejectNegative Enum(diagnostic_prefixing_rule)
--fdiagnostics-show-location=[once|every-line]	How often to emit source location at the beginning of line-wrapped diagnostics.
+-fdiagnostics-show-location=[elide|once|every-line]	How often to emit source location at the beginning of diagnostics.
 
 ; Required for these enum values.
 SourceInclude
@@ -1229,6 +1229,9 @@ Enum(diagnostic_prefixing_rule) String(once) Value(DIAGNOSTICS_SHOW_PREFIX_ONCE)
 EnumValue
 Enum(diagnostic_prefixing_rule) String(every-line) Value(DIAGNOSTICS_SHOW_PREFIX_EVERY_LINE)
 
+EnumValue
+Enum(diagnostic_prefixing_rule) String(elide) Value(DIAGNOSTICS_SHOW_PREFIX_ELIDE)
+
 fdiagnostics-show-caret
 Common Var(flag_diagnostics_show_caret) Init(1)
 Show the source line with a caret indicating the column.
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index c9edaab..79b76d2 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -1262,6 +1262,7 @@ layout::print_source_line (linenum_type row, const char *line, int line_width,
       for (int i = 0; i < m_linenum_width - width; i++)
 	pp_space (m_pp);
       pp_printf (m_pp, "%i | ", row);
+      m_context->last_margin = width + 1;
     }
   else
     pp_space (m_pp);
@@ -2244,6 +2245,11 @@ diagnostic_show_locus (diagnostic_context * context,
 {
   pp_newline (context->printer);
 
+  int saved_last_margin = context->last_margin;
+
+  /* Reset last_margin.  */
+  context->last_margin = -1;
+
   location_t loc = richloc->get_loc ();
   /* Do nothing if source-printing has been disabled.  */
   if (!context->show_caret)
@@ -2257,7 +2263,11 @@ diagnostic_show_locus (diagnostic_context * context,
      fix-it hints.  */
   if (loc == context->last_location
       && richloc->get_num_fixit_hints () == 0)
-    return;
+    {
+      /* Bail out, retaining the value of context->last_margin.  */
+      context->last_margin = saved_last_margin;
+      return;
+    }
 
   context->last_location = loc;
 
@@ -2764,6 +2774,128 @@ test_one_liner_labels ()
      gcc-rich-location.c due to Makefile.in issues).  */
 }
 
+/* Ensure that DIAGNOSTICS_SHOW_PREFIX_ELIDE works as expected.  */
+
+static void
+test_one_liner_elide_location ()
+{
+  location_t start = linemap_position_for_column (line_table, 11);
+  location_t finish = linemap_position_for_column (line_table, 15);
+  location_t field = make_location (start, start, finish);
+
+  /* Test the various combinations of
+     - with/without line numbers
+     - with/without elision.  */
+
+  /* With elision.  */
+  {
+    test_diagnostic_context dc;
+    global_dc_test gdt (&dc);
+    rich_location richloc (line_table, field);
+    richloc.add_fixit_replace ("m_field");
+    error_at (&richloc, "the error message");
+    inform (field, "the inform message");
+    ASSERT_STREQ ("FILENAME:1:11: error: the error message\n"
+		  " foo = bar.field;\n"
+		  "           ^~~~~\n"
+		  "           m_field\n"
+		  "   note: the inform message\n",
+		  pp_formatted_text (dc.printer));
+  }
+  {
+    test_diagnostic_context dc;
+    dc.show_line_numbers_p = true;
+    global_dc_test gdt (&dc);
+    rich_location richloc (line_table, field);
+    richloc.add_fixit_replace ("m_field");
+    error_at (&richloc, "the error message");
+    inform (field, "the 1st inform message");
+    inform (field, "the 2nd inform message");
+    ASSERT_STREQ ("FILENAME:1:11: error: the error message\n"
+		  "1 | foo = bar.field;\n"
+		  "  |           ^~~~~\n"
+		  "  |           m_field\n"
+		  "  = note: the 1st inform message\n"
+		  "  = note: the 2nd inform message\n",
+		  pp_formatted_text (dc.printer));
+  }
+
+  {
+    /* Verify handling of UNKNOWN_LOCATION.  */
+    const char *old_progname = progname;
+    progname = "PROGNAME";
+    test_diagnostic_context dc;
+    dc.show_line_numbers_p = true;
+    global_dc_test gdt (&dc);
+    error_at (UNKNOWN_LOCATION, "the error message");
+    ASSERT_STREQ ("PROGNAME: error: the error message\n",
+		  pp_formatted_text (dc.printer));
+    progname = old_progname;
+  }
+  {
+    /* Verify handling of BUILTINS_LOCATION.  */
+    test_diagnostic_context dc;
+    dc.show_line_numbers_p = true;
+    global_dc_test gdt (&dc);
+    error_at (BUILTINS_LOCATION, "the error message");
+    inform (BUILTINS_LOCATION, "the inform message");
+    ASSERT_STREQ ("<built-in>: error: the error message\n"
+		  "<built-in>: note: the inform message\n",
+		  pp_formatted_text (dc.printer));
+  }
+  {
+    /* With a fix-it hint on the followup.  */
+    test_diagnostic_context dc;
+    dc.show_line_numbers_p = true;
+    global_dc_test gdt (&dc);
+    error_at (field, "the error message");
+    rich_location richloc (line_table, field);
+    richloc.add_fixit_replace ("m_field");
+    inform (&richloc, "the inform message");
+    ASSERT_STREQ ("FILENAME:1:11: error: the error message\n"
+		  "1 | foo = bar.field;\n"
+		  "  |           ^~~~~\n"
+		  "  = note: the inform message\n"
+		  "1 | foo = bar.field;\n"
+		  "  |           ^~~~~\n"
+		  "  |           m_field\n",
+		  pp_formatted_text (dc.printer));
+  }
+
+  /* Without elision.  */
+  {
+    test_diagnostic_context dc;
+    pp_prefixing_rule (dc.printer) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
+    global_dc_test gdt (&dc);
+    rich_location richloc (line_table, field);
+    richloc.add_fixit_replace ("m_field");
+    error_at (&richloc, "the error message");
+    inform (field, "the inform message");
+    ASSERT_STREQ ("FILENAME:1:11: error: the error message\n"
+		  " foo = bar.field;\n"
+		  "           ^~~~~\n"
+		  "           m_field\n"
+		  "FILENAME:1:11: note: the inform message\n",
+		  pp_formatted_text (dc.printer));
+  }
+  {
+    test_diagnostic_context dc;
+    pp_prefixing_rule (dc.printer) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
+    dc.show_line_numbers_p = true;
+    global_dc_test gdt (&dc);
+    rich_location richloc (line_table, field);
+    richloc.add_fixit_replace ("m_field");
+    error_at (&richloc, "the error message");
+    inform (field, "the inform message");
+    ASSERT_STREQ ("FILENAME:1:11: error: the error message\n"
+		  "1 | foo = bar.field;\n"
+		  "  |           ^~~~~\n"
+		  "  |           m_field\n"
+		  "FILENAME:1:11: note: the inform message\n",
+		  pp_formatted_text (dc.printer));
+  }
+}
+
 /* Run the various one-liner tests.  */
 
 static void
@@ -2801,6 +2933,7 @@ test_diagnostic_show_locus_one_liner (const line_table_case &case_)
   test_one_liner_many_fixits_1 ();
   test_one_liner_many_fixits_2 ();
   test_one_liner_labels ();
+  test_one_liner_elide_location ();
 }
 
 /* Verify that gcc_rich_location::add_location_if_nearby works.  */
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 36af1e8..063fd38 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -179,7 +179,9 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
   context->show_line_numbers_p = false;
   context->show_ruler_p = false;
   context->parseable_fixits_p = false;
+  context->last_margin = -1;
   context->edit_context_ptr = NULL;
+  context->override_file = NULL;
 }
 
 /* Maybe initialize the color support. We require clients to do this
@@ -322,8 +324,25 @@ diagnostic_get_location_text (diagnostic_context *context,
   pretty_printer *pp = context->printer;
   const char *locus_cs = colorize_start (pp_show_color (pp), "locus");
   const char *locus_ce = colorize_stop (pp_show_color (pp));
-  const char *file = s.file ? s.file : progname;
-  int line = strcmp (file, N_("<built-in>")) ? s.line : 0;
+  const char *file;
+  int line;
+  if (s.file)
+    {
+      file = s.file;
+      if (strcmp (file, N_("<built-in>")))
+	{
+	  if (context->override_file)
+	    file = context->override_file;
+	  line = s.line;
+	}
+      else
+	line = 0;
+    }
+  else
+    {
+      file = progname;
+      line = s.line;
+    }
   int col = context->show_column ? s.column : 0;
 
   const char *line_col = maybe_line_and_column (line, col);
@@ -357,6 +376,52 @@ diagnostic_build_prefix (diagnostic_context *context,
       text_ce = colorize_stop (pp_show_color (pp));
     }
 
+  /* Don't bother printing the location in the prefix if we have a diagnostic
+     with the same location as the previous diagnostic.  This makes the
+     diagnostics less "dense" visually, so that e.g.:
+
+       PATH_WHICH_COULD_BE_VERY_LONG/SOME_FILENAME:LINE:COLUMN: error: MESSAGE_1
+        SOURCE VIEW
+       PATH_WHICH_COULD_BE_VERY_LONG/SOME_FILENAME:LINE:COLUMN: note: MESSAGE_2
+       PATH_WHICH_COULD_BE_VERY_LONG/SOME_FILENAME:LINE:COLUMN: note: MESSAGE_3
+
+     can be instead printed as:
+
+       PATH_WHICH_COULD_BE_VERY_LONG/SOME_FILENAME:LINE:COLUMN: error: MESSAGE_1
+        SOURCE VIEW
+          note: MESSAGE_2
+          note: MESSAGE_3
+
+     or, with line numbers:
+
+       PATH_WHICH_COULD_BE_VERY_LONG/SOME_FILENAME:LINE:COLUMN: error: MESSAGE_1
+       nn | SOURCE VIEW
+          = note: MESSAGE_2
+          = note: MESSAGE_3
+
+     This should also make it easier for the user to see the boundaries between
+     logically-related diagnostics.  */
+  if (pp_prefixing_rule (pp) == DIAGNOSTICS_SHOW_PREFIX_ELIDE)
+    {
+      location_t loc = diagnostic->richloc->get_loc ();
+      // FIXME: but not if a builtin location?
+      if (loc == context->last_location
+	  && loc != UNKNOWN_LOCATION)
+	{
+	  /* last_margin is set when printing the left margin.  */
+	  if (context->last_margin >= 0)
+	    {
+	      pretty_printer pp;
+	      for (int i = 0; i < context->last_margin; i++)
+		pp_character (&pp, ' ');
+	      pp_printf (&pp, "= %s%s%s", text_cs, text, text_ce);
+	      return xstrdup (pp_formatted_text (&pp));
+	    }
+	  else
+	    return build_message_string ("   %s%s%s", text_cs, text, text_ce);
+	}
+    }
+
   expanded_location s = diagnostic_expand_location (diagnostic);
   char *location_text = diagnostic_get_location_text (context, s);
 
@@ -1666,6 +1731,7 @@ assert_location_text (const char *expected_loc_text,
 {
   test_diagnostic_context dc;
   dc.show_column = show_column;
+  dc.override_file = NULL;
 
   expanded_location xloc;
   xloc.file = filename;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index fe3130b..8d78cf3 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -219,9 +219,17 @@ struct diagnostic_context
      rest of the diagnostic.  */
   bool parseable_fixits_p;
 
+  /* For use with DIAGNOSTICS_SHOW_PREFIX_ELIDE: the width of the last
+     margin, or -1 if there was no margin.  */
+  int last_margin;
+
   /* If non-NULL, an edit_context to which fix-it hints should be
      applied, for generating patches.  */
   edit_context *edit_context_ptr;
+
+  /* For use in selftests: if non-NULL, then use this string when
+     printing filenames.  */
+  const char *override_file;
 };
 
 static inline void
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 760f6e3..baa8e09 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3564,14 +3564,20 @@ Note - this option also affects the display of the @samp{#error} and
 function/type/variable attribute.  It does not however affect the
 @samp{pragma GCC warning} and @samp{pragma GCC error} pragmas.
 
+@item -fdiagnostics-show-location=elide
+@opindex fdiagnostics-show-location
+Instructs the diagnostic messages reporter to only emit source location
+information once within a given diagnostic, and to elide such locations
+for followup diagnostics that have the same location.
+This is the default behavior.
+
 @item -fdiagnostics-show-location=once
 @opindex fdiagnostics-show-location
 Only meaningful in line-wrapping mode.  Instructs the diagnostic messages
 reporter to emit source location information @emph{once}; that is, in
 case the message is too long to fit on a single physical line and has to
 be wrapped, the source location won't be emitted (as prefix) again,
-over and over, in subsequent continuation lines.  This is the default
-behavior.
+over and over, in subsequent continuation lines.
 
 @item -fdiagnostics-show-location=every-line
 Only meaningful in line-wrapping mode.  Instructs the diagnostic
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 736af8f..d30a300 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -773,6 +773,7 @@ pp_set_real_maximum_length (pretty_printer *pp)
      not to increase unnecessarily the line-length cut-off.  */
   if (!pp_is_wrapping_line (pp)
       || pp_prefixing_rule (pp) == DIAGNOSTICS_SHOW_PREFIX_ONCE
+      || pp_prefixing_rule (pp) == DIAGNOSTICS_SHOW_PREFIX_ELIDE
       || pp_prefixing_rule (pp) == DIAGNOSTICS_SHOW_PREFIX_NEVER)
     pp->maximum_length = pp_line_cutoff (pp);
   else
@@ -1531,6 +1532,7 @@ pp_emit_prefix (pretty_printer *pp)
 	  break;
 
 	case DIAGNOSTICS_SHOW_PREFIX_ONCE:
+	case DIAGNOSTICS_SHOW_PREFIX_ELIDE:
 	  if (pp->emitted_prefix)
 	    {
 	      pp_indent (pp);
@@ -1567,8 +1569,9 @@ pretty_printer::pretty_printer (int maximum_length)
     show_color ()
 {
   pp_line_cutoff (this) = maximum_length;
-  /* By default, we emit prefixes once per message.  */
-  pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
+  /* Emit only once within a given diagnostic, and elide locations
+     for followup diagnostics with equal location.  */
+  pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ELIDE;
   pp_set_prefix (this, NULL);
 }
 
diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h
index 0d67e30..ad28730 100644
--- a/gcc/pretty-print.h
+++ b/gcc/pretty-print.h
@@ -40,16 +40,22 @@ struct text_info
   location_t get_location (unsigned int index_of_location) const;
 };
 
-/* How often diagnostics are prefixed by their locations:
-   o DIAGNOSTICS_SHOW_PREFIX_NEVER: never - not yet supported;
-   o DIAGNOSTICS_SHOW_PREFIX_ONCE: emit only once;
-   o DIAGNOSTICS_SHOW_PREFIX_EVERY_LINE: emit each time a physical
-   line is started.  */
+/* How often diagnostics are prefixed by their locations.  */
 enum diagnostic_prefixing_rule_t
 {
-  DIAGNOSTICS_SHOW_PREFIX_ONCE       = 0x0,
-  DIAGNOSTICS_SHOW_PREFIX_NEVER      = 0x1,
-  DIAGNOSTICS_SHOW_PREFIX_EVERY_LINE = 0x2
+  /* Emit only once within a given diagnostic.  */
+  DIAGNOSTICS_SHOW_PREFIX_ONCE,
+
+  /* Emit only once within a given diagnostic, and elide locations
+     for followup diagnostics with equal location.  */
+  DIAGNOSTICS_SHOW_PREFIX_ELIDE,
+
+  /* Never - not yet supported.  */
+  DIAGNOSTICS_SHOW_PREFIX_NEVER,
+
+  /* Emit each time a physical line is started within a diagnostic
+     message.  */
+  DIAGNOSTICS_SHOW_PREFIX_EVERY_LINE
 };
 
 /* The chunk_info data structure forms a stack of the results from the
diff --git a/gcc/selftest-diagnostic.c b/gcc/selftest-diagnostic.c
index f3c255e..1356d71 100644
--- a/gcc/selftest-diagnostic.c
+++ b/gcc/selftest-diagnostic.c
@@ -36,10 +36,14 @@ namespace selftest {
 test_diagnostic_context::test_diagnostic_context ()
 {
   diagnostic_initialize (this, 0);
+  /* Don't print to stderr.  */
+  printer->buffer->stream = NULL;
+  printer->buffer->flush_p = false;
   show_caret = true;
   show_labels_p = true;
   show_column = true;
   start_span = start_span_cb;
+  override_file = "FILENAME";
 }
 
 test_diagnostic_context::~test_diagnostic_context ()
@@ -58,6 +62,23 @@ test_diagnostic_context::start_span_cb (diagnostic_context *context,
   default_diagnostic_start_span_fn (context, exploc);
 }
 
+/* Implementation of class selftest::global_dc_test.  */
+
+/* Constructor.  Override "global_dc" with DC.  */
+
+global_dc_test::global_dc_test (diagnostic_context *dc)
+: m_saved_global_dc (global_dc)
+{
+  global_dc = dc;
+}
+
+/* Destructor.  Restore the saved global_dc.  */
+
+global_dc_test::~global_dc_test ()
+{
+  global_dc = m_saved_global_dc;
+}
+
 } // namespace selftest
 
 #endif /* #if CHECKING_P */
diff --git a/gcc/selftest-diagnostic.h b/gcc/selftest-diagnostic.h
index b1d6ace..90ff2d1 100644
--- a/gcc/selftest-diagnostic.h
+++ b/gcc/selftest-diagnostic.h
@@ -42,6 +42,22 @@ class test_diagnostic_context : public diagnostic_context
   start_span_cb (diagnostic_context *context, expanded_location exploc);
 };
 
+/* A class for overriding the global "global_dc" within a selftest,
+   restoring its value afterwards.  */
+
+class global_dc_test
+{
+ public:
+  /* Constructor.  Override "global_dc" with DC.  */
+  global_dc_test (diagnostic_context *dc);
+
+  /* Destructor.  Restore the saved global_dc.  */
+  ~global_dc_test ();
+
+ private:
+  diagnostic_context *m_saved_global_dc;
+};
+
 } // namespace selftest
 
 #endif /* #if CHECKING_P */
diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index df36c34..9e08545 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -22,6 +22,7 @@ if ![info exists TEST_ALWAYS_FLAGS] {
     set TEST_ALWAYS_FLAGS ""
 }
 set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never $TEST_ALWAYS_FLAGS"
+set TEST_ALWAYS_FLAGS "-fdiagnostics-show-location=once $TEST_ALWAYS_FLAGS"
 
 proc prune_gcc_output { text } {
     global srcdir
-- 
1.8.5.3

