diff mbox series

[committed] diagnostics: add line numbers to source (PR other/84889)

Message ID 1533831633-37107-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series [committed] diagnostics: add line numbers to source (PR other/84889) | expand

Commit Message

David Malcolm Aug. 9, 2018, 4:20 p.m. UTC 
This patch adds a left margin to the lines of source (and annotations)
printed by diagnostic_show_locus, so that e.g. rather than:

test.c: In function 'test':
test.c:12:15: error: 'struct foo' has no member named 'm_bar'; did you mean 'bar'?
   return ptr->m_bar;
               ^~~~~
               bar

we print:

test.c: In function 'test':
test.c:12:15: error: 'struct foo' has no member named 'm_bar'; did you mean 'bar'?
12 |   return ptr->m_bar;
   |               ^~~~~
   |               bar

Similarly, for a multiline case (in C++ this time), this:

bad-binary-ops.C: In function 'int test_2()':
bad-binary-ops.C:26:4: error: no match for 'operator+' (operand types are 's' and 't')
   return (some_function ()
           ~~~~~~~~~~~~~~~~
    + some_other_function ());
    ^~~~~~~~~~~~~~~~~~~~~~~~

becomes:

bad-binary-ops.C: In function 'int test_2()':
bad-binary-ops.C:26:4: error: no match for 'operator+' (operand types are 's' and 't')
25 |   return (some_function ()
   |           ~~~~~~~~~~~~~~~~
26 |    + some_other_function ());
   |    ^~~~~~~~~~~~~~~~~~~~~~~~

I believe this slightly improves the readability of the output, in that it:
- distinguishes between the user's source code vs the annotation lines
  that we're adding (the underlinings and fix-it hints here)
- shows the line numbers in another place (potentially helpful for
  multiline diagnostics, where the user can see the line numbers directly,
  rather than have to figure them out relative to the caret: in the 2nd
  example, note how the diagnostic is reported at line 26, but the first
  line printed is actually line 25)

I'm not sure that this is the precise format we want to go with [1], but
I think it's an improvement over the status quo, and we're in stage 1
of gcc 9, so there's plenty of time to shake out issues.

I've turned it on by default; it can be disabled via
-fno-diagnostics-show-line-numbers (it's also turned off in the testsuite, to
avoid breaking numerous existing test cases).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; adds
18 PASS results to gcc.sum.

Committed to trunk as r263450.

Dave

[1] Some possible variants:
  - maybe just "LL|" rather than "LL | "
  - maybe ':' rather than '|'
  - maybe we should have some leading indentation, to better split up
    the diagnostics visually via the left-hand column
  - etc

gcc/ChangeLog:
	PR other/84889
	* common.opt (fdiagnostics-show-line-numbers): New option.
	* diagnostic-show-locus.c (class layout): Add fields
	"m_show_line_numbers_p" and "m_linenum_width";
	(num_digits): New function.
	(test_num_digits): New function.
	(layout::layout): Initialize new fields.  Update m_x_offset
	logic to handle any left margin.
	(layout::print_source_line): Print line number when requested.
	(layout::start_annotation_line): New member function.
	(layout::print_annotation_line): Call it.
	(layout::print_leading_fixits): Likewise.
	(layout::print_trailing_fixits): Likewise.  Update calls to
	move_to_column for new parameter.
	(layout::get_x_bound_for_row): Add "add_left_margin" param and use
	it to potentially call start_annotation_line.
	(layout::show_ruler): Call start_annotation_line.
	(selftest::test_line_numbers_multiline_range): New selftest.
	(selftest::diagnostic_show_locus_c_tests): Call test_num_digits
	and selftest::test_line_numbers_multiline_range.
	* diagnostic.c (diagnostic_initialize): Initialize
	show_line_numbers_p.
	* diagnostic.h (struct diagnostic_context): Add field
	"show_line_numbers_p".
	* doc/invoke.texi (Diagnostic Message Formatting Options): Add
	-fno-diagnostics-show-line-numbers.
	* dwarf2out.c (gen_producer_string): Add
	OPT_fdiagnostics_show_line_numbers to the ignored options.
	* lto-wrapper.c (merge_and_complain): Likewise to the "pick
	one setting" options.
	(append_compiler_options): Likewise to the dropped options.
	(append_diag_options): Likewise to the passed-on options.
	* opts.c (common_handle_option): Handle the new option.
	* toplev.c (general_init): Set up global_dc->show_line_numbers_p.

gcc/testsuite/ChangeLog:
	PR other/84889
	* gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c: New
	test.
	* gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c:
	New test.
	* gcc.dg/plugin/plugin.exp (plugin_test_list): Add the new tests.
	* lib/prune.exp: Add -fno-diagnostics-show-line-numbers to
	TEST_ALWAYS_FLAGS.
---
 gcc/common.opt                                     |   4 +
 gcc/diagnostic-show-locus.c                        | 163 ++++++++++++++++++++-
 gcc/diagnostic.c                                   |   1 +
 gcc/diagnostic.h                                   |   4 +
 gcc/doc/invoke.texi                                |   8 +
 gcc/dwarf2out.c                                    |   1 +
 gcc/lto-wrapper.c                                  |   3 +
 gcc/opts.c                                         |   4 +
 .../diagnostic-test-show-locus-bw-line-numbers.c   | 115 +++++++++++++++
 ...diagnostic-test-show-locus-color-line-numbers.c |  24 +++
 gcc/testsuite/gcc.dg/plugin/plugin.exp             |   4 +-
 gcc/testsuite/lib/prune.exp                        |   2 +-
 gcc/toplev.c                                       |   2 +
 13 files changed, 325 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
 create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c

Comments

Joseph Myers Aug. 9, 2018, 3:40 p.m. UTC | #1 
On Thu, 9 Aug 2018, David Malcolm wrote:

> This patch adds a left margin to the lines of source (and annotations)
> printed by diagnostic_show_locus, so that e.g. rather than:

To confirm: if the lines contain tabs anywhere, will the code replace them 
by spaces when a margin is added to ensure that all lines really do get 
consistently indented by the same amount as part of adding the left 
margin?
David Malcolm Aug. 9, 2018, 3:45 p.m. UTC | #2 
On Thu, 2018-08-09 at 15:40 +0000, Joseph Myers wrote:
> On Thu, 9 Aug 2018, David Malcolm wrote:
> 
> > This patch adds a left margin to the lines of source (and
> > annotations)
> > printed by diagnostic_show_locus, so that e.g. rather than:
> 
> To confirm: if the lines contain tabs anywhere, will the code replace
> them 
> by spaces when a margin is added to ensure that all lines really do
> get 
> consistently indented by the same amount as part of adding the left 
> margin?

I hadn't thought of that; bother.  (that's why we make these kinds of
changes in stage 1).

I'm not sure that we already handled that case properly; the previous
behavior was to print a leading space before the source line.

I'll investigate.

Thanks
Dave
David Malcolm Aug. 9, 2018, 8:09 p.m. UTC | #3 
On Thu, 2018-08-09 at 11:45 -0400, David Malcolm wrote:
> On Thu, 2018-08-09 at 15:40 +0000, Joseph Myers wrote:
> > On Thu, 9 Aug 2018, David Malcolm wrote:
> > 
> > > This patch adds a left margin to the lines of source (and
> > > annotations)
> > > printed by diagnostic_show_locus, so that e.g. rather than:
> > 
> > To confirm: if the lines contain tabs anywhere, will the code
> > replace
> > them 
> > by spaces when a margin is added to ensure that all lines really do
> > get 
> > consistently indented by the same amount as part of adding the
> > left 
> > margin?
> 
> I hadn't thought of that; bother.  (that's why we make these kinds of
> changes in stage 1).
> 
> I'm not sure that we already handled that case properly; the previous
> behavior was to print a leading space before the source line.
> 
> I'll investigate.

It turns out that we convert tab characters to *single* space
characters when printing source code.

This behavior has been present since Manu first implemented
-fdiagnostics-show-caret in r186305 (aka
5a9830842f69ebb059061e26f8b0699cbd85121e, PR 24985), where it was this
logic (there in diagnostic.c's diagnostic_show_locus):
      char c = *line == '\t' ? ' ' : *line;
      pp_character (context->printer, c);

(that logic is now in diagnostic-show-locus.c in
layout::print_source_line)

Arguably this is a bug, but it's intimately linked to the way in which
we track "column numbers".  Our "column numbers" are currently simply a
1-based byte-count, I believe, so a tab character is treated by us as
simply an increment of 1 right now.  There are similar issues with
multibyte characters, which are being tracked in PR 49973.

Adding the left margin with line numbers doesn't change this bug, but
makes fixing it slightly more complicated.

I've opened PR other/86904 ("Column numbers ignore tab characters") to
track it.

Dave
Manuel López-Ibáñez Aug. 17, 2018, 4:29 p.m. UTC | #4 
On 09/08/18 21:09, David Malcolm wrote:
> 
> It turns out that we convert tab characters to *single* space
> characters when printing source code.
> 
> This behavior has been present since Manu first implemented
> -fdiagnostics-show-caret in r186305 (aka
> 5a9830842f69ebb059061e26f8b0699cbd85121e, PR 24985), where it was this
> logic (there in diagnostic.c's diagnostic_show_locus):
>        char c = *line == '\t' ? ' ' : *line;
>        pp_character (context->printer, c);
> 
> (that logic is now in diagnostic-show-locus.c in
> layout::print_source_line)
> 
> Arguably this is a bug, but it's intimately linked to the way in which
> we track "column numbers".  Our "column numbers" are currently simply a
> 1-based byte-count, I believe, so a tab character is treated by us as
> simply an increment of 1 right now.  There are similar issues with
> multibyte characters, which are being tracked in PR 49973.


Hi David,

At the time, this was done on purpose for two reasons:

1) The way we counted column numbers already counted tabs as 1-space and ...
2) It leads to wasting less horizontal space and more consistent output.

I believe that (1) was due to this bug: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52899 which got fixed.

The GCS says that column numbers should count tab stops as 8 spaces 
[https://www.gnu.org/prep/standards/html_node/Errors.html].
I believe that -ftabstop=8 is the default after PR52899 was fixed.

However, I see that GCC trunk still counts tabs as 1-column, probably because 
emacs counts tabs as one column when interpreting column numbers in the output 
of GCC. As long as both of these things are true, I believe it doesn't make 
much sense to print 8 spaces (or a tab) instead of a 1-column space. It will 
make interpreting the column numbers much harder and break the parsing of GCC 
diagnostics done by emacs.

Note that if we print the tab directly, the width of the tab in the terminal 
may not be the same as in the editor the user is using. Moreover, if the user 
is using tabs consistently (instead of using tabs on some lines and spaces in 
others), replacing tabs with 1 space will only reduce the visual space per 
indentation level, but the indentation structure will remain consistent.

I wish I had added a summary of the above to the code as a comment.

Finally, PR49973 is about GCC counting multiple columns for characters that 
should be counted as one column. This should be fixed in our line-map 
implementation using wcwidth() when lexing. It is not the same issue at all. 
Once column numbers are correctly counted, the output should be fine as well 
(the caret line does not change multi-byte characters).

I hope the above helps,

	Manuel.
Andreas Schwab Aug. 17, 2018, 4:50 p.m. UTC | #5 
On Aug 17 2018, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:

> However, I see that GCC trunk still counts tabs as 1-column, probably
> because emacs counts tabs as one column when interpreting column numbers
> in the output of GCC.

That is not true.  Emacs is using screen columns by default for almost
20 years now (see compilation-error-screen-columns).

Andreas.
Manuel López-Ibáñez Aug. 17, 2018, 5:49 p.m. UTC | #6 
On 17/08/18 17:50, Andreas Schwab wrote:
> On Aug 17 2018, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> 
>> However, I see that GCC trunk still counts tabs as 1-column, probably
>> because emacs counts tabs as one column when interpreting column numbers
>> in the output of GCC.
> 
> That is not true.  Emacs is using screen columns by default for almost
> 20 years now (see compilation-error-screen-columns).

Maybe I didn't properly explain what I mean. I mean that Emacs counts a tab as 
a "GCC" column, whatever number of visual columns the tab is configured to be 
in Emacs. The following code (with a tab before 'a'):

/* ñ    /* */
/* a    /* */
	a

With gcc -c test.c -Wcomment, it gives:

test.c:1:10: warning: "/*" within comment [-Wcomment]
  /* ñ    /* */
           ^
test.c:2:9: warning: "/*" within comment [-Wcomment]
  /* a    /* */
          ^
test.c:3:2: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ at end of input
   a
   ^

When the above appears in a compilation-mode buffer, I can click (or press 
enter) on each diagnostic and Emacs will jump exactly to what is pointed by the 
"^". That is, for Emacs, 3:2 does not mean line 3, *visual* column 2. In my 
Emacs, it jumps to column 8 (where the "a" is).

Changing the output to be 3:9 (if GCC starts interpreting tabs as 8 spaces as 
recommended by the GCS or to the value given by -ftabstop) will make Emacs jump 
to the wrong place.

This is independent of multi-byte characters. GCC is pointing to the wrong 
place when the line contains "ñ".

I hope the above is clearer,

Manuel.
Andreas Schwab Aug. 17, 2018, 6:59 p.m. UTC | #7 
On Aug 17 2018, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:

> In my Emacs, it jumps to column 8 (where the "a" is).

That's because there is no way to place it on column 1.

Andreas.
diff mbox series

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 5bb6452..b35a7e9 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1233,6 +1233,10 @@  fdiagnostics-show-caret
 Common Var(flag_diagnostics_show_caret) Init(1)
 Show the source line with a caret indicating the column.
 
+fdiagnostics-show-line-numbers
+Common Var(flag_diagnostics_show_line_numbers) Init(1)
+Show line numbers in the left margin when showing source
+
 fdiagnostics-color
 Common Alias(fdiagnostics-color=,always,never)
 ;
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index cf9e5c2..238c689 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -251,6 +251,7 @@  class layout
   void print_source_line (linenum_type row, const char *line, int line_width,
 			  line_bounds *lbounds_out);
   bool should_print_annotation_line_p (linenum_type row) const;
+  void start_annotation_line () const;
   void print_annotation_line (linenum_type row, const line_bounds lbounds);
   void print_trailing_fixits (linenum_type row);
 
@@ -276,7 +277,7 @@  class layout
 		       int last_non_ws);
 
   void
-  move_to_column (int *column, int dest_column);
+  move_to_column (int *column, int dest_column, bool add_left_margin);
 
  private:
   diagnostic_context *m_context;
@@ -286,9 +287,11 @@  class layout
   expanded_location m_exploc;
   colorizer m_colorizer;
   bool m_colorize_source_p;
+  bool m_show_line_numbers_p;
   auto_vec <layout_range> m_layout_ranges;
   auto_vec <const fixit_hint *> m_fixit_hints;
   auto_vec <line_span> m_line_spans;
+  int m_linenum_width;
   int m_x_offset;
 };
 
@@ -805,6 +808,56 @@  fixit_cmp (const void *p_a, const void *p_b)
   return hint_a->get_start_loc () - hint_b->get_start_loc ();
 }
 
+/* Get the number of digits in the decimal representation
+   of VALUE.  */
+
+static int
+num_digits (int value)
+{
+  /* Perhaps simpler to use log10 for this, but doing it this way avoids
+     using floating point.  */
+  gcc_assert (value >= 0);
+
+  if (value == 0)
+    return 1;
+
+  int digits = 0;
+  while (value > 0)
+    {
+      digits++;
+      value /= 10;
+    }
+  return digits;
+}
+
+
+#if CHECKING_P
+
+/* Selftest for num_digits.  */
+
+static void
+test_num_digits ()
+{
+  ASSERT_EQ (1, num_digits (0));
+  ASSERT_EQ (1, num_digits (9));
+  ASSERT_EQ (2, num_digits (10));
+  ASSERT_EQ (2, num_digits (99));
+  ASSERT_EQ (3, num_digits (100));
+  ASSERT_EQ (3, num_digits (999));
+  ASSERT_EQ (4, num_digits (1000));
+  ASSERT_EQ (4, num_digits (9999));
+  ASSERT_EQ (5, num_digits (10000));
+  ASSERT_EQ (5, num_digits (99999));
+  ASSERT_EQ (6, num_digits (100000));
+  ASSERT_EQ (6, num_digits (999999));
+  ASSERT_EQ (7, num_digits (1000000));
+  ASSERT_EQ (7, num_digits (9999999));
+  ASSERT_EQ (8, num_digits (10000000));
+  ASSERT_EQ (8, num_digits (99999999));
+}
+
+#endif /* #if CHECKING_P */
+
 /* Implementation of class layout.  */
 
 /* Constructor for class layout.
@@ -826,9 +879,11 @@  layout::layout (diagnostic_context * context,
   m_exploc (richloc->get_expanded_location (0)),
   m_colorizer (context, diagnostic_kind),
   m_colorize_source_p (context->colorize_source_p),
+  m_show_line_numbers_p (context->show_line_numbers_p),
   m_layout_ranges (richloc->get_num_locations ()),
   m_fixit_hints (richloc->get_num_fixit_hints ()),
   m_line_spans (1 + richloc->get_num_locations ()),
+  m_linenum_width (0),
   m_x_offset (0)
 {
   for (unsigned int idx = 0; idx < richloc->get_num_locations (); idx++)
@@ -854,6 +909,14 @@  layout::layout (diagnostic_context * context,
   /* Populate m_line_spans.  */
   calculate_line_spans ();
 
+  /* Determine m_linenum_width.  */
+  gcc_assert (m_line_spans.length () > 0);
+  const line_span *last_span = &m_line_spans[m_line_spans.length () - 1];
+  int highest_line = last_span->m_last_line;
+  if (highest_line < 0)
+    highest_line = 0;
+  m_linenum_width = num_digits (highest_line);
+
   /* Adjust m_x_offset.
      Center the primary caret to fit in max_width; all columns
      will be adjusted accordingly.  */
@@ -863,6 +926,8 @@  layout::layout (diagnostic_context * context,
     {
       size_t right_margin = CARET_LINE_MARGIN;
       size_t column = m_exploc.column;
+      if (m_show_line_numbers_p)
+	column += m_linenum_width + 2;
       right_margin = MIN (line.length () - column, right_margin);
       right_margin = max_width - right_margin;
       if (line.length () >= max_width && column > right_margin)
@@ -1183,7 +1248,15 @@  layout::print_source_line (linenum_type row, const char *line, int line_width,
 							   line_width);
   line += m_x_offset;
 
-  pp_space (m_pp);
+  if (m_show_line_numbers_p)
+    {
+      int width = num_digits (row);
+      for (int i = 0; i < m_linenum_width - width; i++)
+	pp_space (m_pp);
+      pp_printf (m_pp, "%i | ", row);
+    }
+  else
+    pp_space (m_pp);
   int first_non_ws = INT_MAX;
   int last_non_ws = 0;
   int column;
@@ -1245,6 +1318,20 @@  layout::should_print_annotation_line_p (linenum_type row) const
   return false;
 }
 
+/* Begin an annotation line.  If m_show_line_numbers_p, print the left
+   margin, which is empty for annotation lines.  Otherwise, do nothing.  */
+
+void
+layout::start_annotation_line () const
+{
+  if (m_show_line_numbers_p)
+    {
+      for (int i = 0; i < m_linenum_width; i++)
+	pp_space (m_pp);
+      pp_string (m_pp, " |");
+    }
+}
+
 /* Print a line consisting of the caret/underlines for the given
    source line.  */
 
@@ -1254,7 +1341,9 @@  layout::print_annotation_line (linenum_type row, const line_bounds lbounds)
   int x_bound = get_x_bound_for_row (row, m_exploc.column,
 				     lbounds.m_last_non_ws);
 
+  start_annotation_line ();
   pp_space (m_pp);
+
   for (int column = 1 + m_x_offset; column < x_bound; column++)
     {
       bool in_range_p;
@@ -1316,6 +1405,7 @@  layout::print_leading_fixits (linenum_type row)
 	     helps them stand out from each other, and from
 	     the surrounding text.  */
 	  m_colorizer.set_normal_text ();
+	  start_annotation_line ();
 	  pp_character (m_pp, '+');
 	  m_colorizer.set_fixit_insert ();
 	  /* Print all but the trailing newline of the fix-it hint.
@@ -1712,6 +1802,9 @@  layout::print_trailing_fixits (linenum_type row)
   correction *c;
   int column = m_x_offset;
 
+  if (!corrections.m_corrections.is_empty ())
+    start_annotation_line ();
+
   FOR_EACH_VEC_ELT (corrections.m_corrections, i, c)
     {
       /* For now we assume each fixit hint can only touch one line.  */
@@ -1719,7 +1812,7 @@  layout::print_trailing_fixits (linenum_type row)
 	{
 	  /* This assumes the insertion just affects one line.  */
 	  int start_column = c->m_printed_columns.start;
-	  move_to_column (&column, start_column);
+	  move_to_column (&column, start_column, true);
 	  m_colorizer.set_fixit_insert ();
 	  pp_string (m_pp, c->m_text);
 	  m_colorizer.set_normal_text ();
@@ -1737,7 +1830,7 @@  layout::print_trailing_fixits (linenum_type row)
 					       finish_column)
 	      || c->m_len == 0)
 	    {
-	      move_to_column (&column, start_column);
+	      move_to_column (&column, start_column, true);
 	      m_colorizer.set_fixit_delete ();
 	      for (; column <= finish_column; column++)
 		pp_character (m_pp, '-');
@@ -1748,7 +1841,7 @@  layout::print_trailing_fixits (linenum_type row)
 	     a new line) if we have actual replacement text.  */
 	  if (c->m_len > 0)
 	    {
-	      move_to_column (&column, start_column);
+	      move_to_column (&column, start_column, true);
 	      m_colorizer.set_fixit_insert ();
 	      pp_string (m_pp, c->m_text);
 	      m_colorizer.set_normal_text ();
@@ -1758,7 +1851,7 @@  layout::print_trailing_fixits (linenum_type row)
     }
 
   /* Add a trailing newline, if necessary.  */
-  move_to_column (&column, 0);
+  move_to_column (&column, 0, false);
 }
 
 /* Disable any colorization and emit a newline.  */
@@ -1855,15 +1948,18 @@  layout::get_x_bound_for_row (linenum_type row, int caret_column,
 
 /* Given *COLUMN as an x-coordinate, print spaces to position
    successive output at DEST_COLUMN, printing a newline if necessary,
-   and updating *COLUMN.  */
+   and updating *COLUMN.  If ADD_LEFT_MARGIN, then print the (empty)
+   left margin after any newline.  */
 
 void
-layout::move_to_column (int *column, int dest_column)
+layout::move_to_column (int *column, int dest_column, bool add_left_margin)
 {
   /* Start a new line if we need to.  */
   if (*column > dest_column)
     {
       print_newline ();
+      if (add_left_margin)
+	start_annotation_line ();
       *column = m_x_offset;
     }
 
@@ -1883,6 +1979,7 @@  layout::show_ruler (int max_column) const
   /* Hundreds.  */
   if (max_column > 99)
     {
+      start_annotation_line ();
       pp_space (m_pp);
       for (int column = 1 + m_x_offset; column <= max_column; column++)
 	if (column % 10 == 0)
@@ -1893,6 +1990,7 @@  layout::show_ruler (int max_column) const
     }
 
   /* Tens.  */
+  start_annotation_line ();
   pp_space (m_pp);
   for (int column = 1 + m_x_offset; column <= max_column; column++)
     if (column % 10 == 0)
@@ -1902,6 +2000,7 @@  layout::show_ruler (int max_column) const
   pp_newline (m_pp);
 
   /* Units.  */
+  start_annotation_line ();
   pp_space (m_pp);
   for (int column = 1 + m_x_offset; column <= max_column; column++)
     pp_character (m_pp, '0' + (column % 10));
@@ -3139,12 +3238,58 @@  test_fixit_deletion_affecting_newline (const line_table_case &case_)
 		pp_formatted_text (dc.printer));
 }
 
+/* Verify that line numbers are correctly printed for the case of
+   a multiline range in which the width of the line numbers changes
+   (e.g. from "9" to "10").  */
+
+static void
+test_line_numbers_multiline_range ()
+{
+  /* Create a tempfile and write some text to it.  */
+  pretty_printer pp;
+  for (int i = 0; i < 20; i++)
+    /* .........0000000001111111.
+   .............1234567890123456.  */
+    pp_printf (&pp, "this is line %i\n", i + 1);
+  temp_source_file tmp (SELFTEST_LOCATION, ".txt", pp_formatted_text (&pp));
+  line_table_test ltt;
+
+  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);
+
+  /* Create a multi-line location, starting at the "line" of line 9, with
+     a caret on the "is" of line 10, finishing on the "this" line 11.  */
+
+  location_t start
+    = linemap_position_for_line_and_column (line_table, ord_map, 9, 9);
+  location_t caret
+    = linemap_position_for_line_and_column (line_table, ord_map, 10, 6);
+  location_t finish
+    = linemap_position_for_line_and_column (line_table, ord_map, 11, 4);
+  location_t loc = make_location (caret, start, finish);
+
+  test_diagnostic_context dc;
+  dc.show_line_numbers_p = true;
+  gcc_rich_location richloc (loc);
+  diagnostic_show_locus (&dc, &richloc, DK_ERROR);
+  ASSERT_STREQ ("\n"
+		" 9 | this is line 9\n"
+		"   |         ~~~~~~\n"
+		"10 | this is line 10\n"
+		"   | ~~~~~^~~~~~~~~~\n"
+		"11 | this is line 11\n"
+		"   | ~~~~  \n",
+		pp_formatted_text (dc.printer));
+}
+
 /* Run all of the selftests within this file.  */
 
 void
 diagnostic_show_locus_c_tests ()
 {
   test_line_span ();
+  test_num_digits ();
 
   test_layout_range_for_single_point ();
   test_layout_range_for_single_line ();
@@ -3164,6 +3309,8 @@  diagnostic_show_locus_c_tests ()
   for_each_line_table_case (test_fixit_insert_containing_newline_2);
   for_each_line_table_case (test_fixit_replace_containing_newline);
   for_each_line_table_case (test_fixit_deletion_affecting_newline);
+
+  test_line_numbers_multiline_range ();
 }
 
 } // namespace selftest
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 5205944..f43d3d9 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -175,6 +175,7 @@  diagnostic_initialize (diagnostic_context *context, int n_opts)
   context->lock = 0;
   context->inhibit_notes_p = false;
   context->colorize_source_p = false;
+  context->show_line_numbers_p = false;
   context->show_ruler_p = false;
   context->parseable_fixits_p = false;
   context->edit_context_ptr = NULL;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index cf3a610..744aec1 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -204,6 +204,10 @@  struct diagnostic_context
      a token, which would look strange).  */
   bool colorize_source_p;
 
+  /* When printing source code, should there be a left-hand margin
+     showing line numbers?  */
+  bool show_line_numbers_p;
+
   /* Usable by plugins; if true, print a debugging ruler above the
      source output.  */
   bool show_ruler_p;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 438274e..0e9a9c3 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -267,6 +267,7 @@  Objective-C and Objective-C++ Dialects}.
 -fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]}  @gol
 -fdiagnostics-color=@r{[}auto@r{|}never@r{|}always@r{]}  @gol
 -fno-diagnostics-show-option  -fno-diagnostics-show-caret @gol
+-fno-diagnostics-show-line-numbers @gol
 -fdiagnostics-parseable-fixits  -fdiagnostics-generate-patch @gol
 -fdiagnostics-show-template-tree -fno-elide-type @gol
 -fno-show-column}
@@ -3710,6 +3711,13 @@  the @option{-fmessage-length=n} option is given.  When the output is done
 to the terminal, the width is limited to the width given by the
 @env{COLUMNS} environment variable or, if not set, to the terminal width.
 
+@item -fno-diagnostics-show-line-numbers
+@opindex fno-diagnostics-show-line-numbers
+@opindex fdiagnostics-show-line-numbers
+By default, when printing source code (via @option{-fdiagnostics-show-caret}),
+a left margin is printed, showing line numbers.  This option suppresses this
+left margin.
+
 @item -fdiagnostics-parseable-fixits
 @opindex fdiagnostics-parseable-fixits
 Emit fix-it hints in a machine-parseable format, suitable for consumption
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b67481d..9ed4730 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -24247,6 +24247,7 @@  gen_producer_string (void)
       case OPT_fdiagnostics_show_location_:
       case OPT_fdiagnostics_show_option:
       case OPT_fdiagnostics_show_caret:
+      case OPT_fdiagnostics_show_line_numbers:
       case OPT_fdiagnostics_color_:
       case OPT_fverbose_asm:
       case OPT____:
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index cf4a8c6..39d9f08 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -255,6 +255,7 @@  merge_and_complain (struct cl_decoded_option **decoded_options,
 
 	  /* Fallthru.  */
 	case OPT_fdiagnostics_show_caret:
+	case OPT_fdiagnostics_show_line_numbers:
 	case OPT_fdiagnostics_show_option:
 	case OPT_fdiagnostics_show_location_:
 	case OPT_fshow_column:
@@ -536,6 +537,7 @@  append_compiler_options (obstack *argv_obstack, struct cl_decoded_option *opts,
       switch (option->opt_index)
 	{
 	case OPT_fdiagnostics_show_caret:
+	case OPT_fdiagnostics_show_line_numbers:
 	case OPT_fdiagnostics_show_option:
 	case OPT_fdiagnostics_show_location_:
 	case OPT_fshow_column:
@@ -582,6 +584,7 @@  append_diag_options (obstack *argv_obstack, struct cl_decoded_option *opts,
 	{
 	case OPT_fdiagnostics_color_:
 	case OPT_fdiagnostics_show_caret:
+	case OPT_fdiagnostics_show_line_numbers:
 	case OPT_fdiagnostics_show_option:
 	case OPT_fdiagnostics_show_location_:
 	case OPT_fshow_column:
diff --git a/gcc/opts.c b/gcc/opts.c
index 17d9198..4153263 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2175,6 +2175,10 @@  common_handle_option (struct gcc_options *opts,
       dc->show_caret = value;
       break;
 
+    case OPT_fdiagnostics_show_line_numbers:
+      dc->show_line_numbers_p = value;
+      break;
+
     case OPT_fdiagnostics_color_:
       diagnostic_color_init (dc, value);
       break;
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
new file mode 100644
index 0000000..66a2faa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
@@ -0,0 +1,115 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdiagnostics-show-caret -fdiagnostics-show-line-numbers" } */
+
+/* This is a collection of unittests for diagnostic_show_locus;
+   see the overview in diagnostic_plugin_test_show_locus.c.
+
+   In particular, note the discussion of why we need a very long line here:
+01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
+   and that we can't use macros in this file.  */
+
+void test_simple (void)
+{
+#if 0
+  myvar = myvar.x; /* { dg-warning "test" } */
+
+/* { dg-begin-multiline-output "" }
+14 |   myvar = myvar.x;
+   |           ~~~~~^~
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+void test_multiline (void)
+{
+#if 0
+  x = (first_function ()
+       + second_function ()); /* { dg-warning "test" } */
+
+/* { dg-begin-multiline-output "" }
+26 |   x = (first_function ()
+   |        ~~~~~~~~~~~~~~~~~
+27 |        + second_function ());
+   |        ^ ~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+void test_very_wide_line (void)
+{
+#if 0
+                                                                                float f = foo * bar; /* { dg-warning "95: test" } */
+/* { dg-begin-multiline-output "" }
+   | 0         0         0         0         0         0         1         
+   | 4         5         6         7         8         9         0         
+   | 0123456789012345678901234567890123456789012345678901234567890123456789
+41 |                                          float f = foo * bar;
+   |                                                    ~~~~^~~~~
+   |                                                    bar * foo
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of insertion fixit hints
+   (example taken from PR 62316).  */
+
+void test_fixit_insert (void)
+{
+#if 0
+   int a[2][2] = { 0, 1 , 2, 3 }; /* { dg-warning "insertion hints" } */
+/* { dg-begin-multiline-output "" }
+59 |    int a[2][2] = { 0, 1 , 2, 3 };
+   |                    ^~~~
+   |                    {   }
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of "remove" fixit hints.  */
+
+void test_fixit_remove (void)
+{
+#if 0
+  int a;; /* { dg-warning "example of a removal hint" } */
+/* { dg-begin-multiline-output "" }
+73 |   int a;;
+   |         ^
+   |         -
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of "replace" fixit hints.  */
+
+void test_fixit_replace (void)
+{
+#if 0
+  gtk_widget_showall (dlg); /* { dg-warning "example of a replacement hint" } */
+/* { dg-begin-multiline-output "" }
+87 |   gtk_widget_showall (dlg);
+   |   ^~~~~~~~~~~~~~~~~~
+   |   gtk_widget_show_all
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+
+/* Unit test for rendering of fix-it hints that add new lines.  */
+
+void test_fixit_insert_newline (void)
+{
+#if 0
+  switch (op)
+    {
+    case 'a':
+      x = a;
+    case 'b':  /* { dg-warning "newline insertion" } */
+      x = b;
+    }
+/* { dg-begin-multiline-output "" }
+    |+      break;
+106 |     case 'b':
+    |     ^~~~~~~~
+   { dg-end-multiline-output "" } */
+#endif
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c
new file mode 100644
index 0000000..a80b6de
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdiagnostics-show-caret -fplugin-arg-diagnostic_plugin_test_show_locus-color -fdiagnostics-show-line-numbers" } */
+
+/* This is a collection of unittests for diagnostic_show_locus;
+   see the overview in diagnostic_plugin_test_show_locus.c.
+
+   In particular, note the discussion of why we need a very long line here:
+01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
+   and that we can't use macros in this file.  */
+
+void test_multiline (void)
+{
+#if 0
+  x = (first_function ()
+       + second_function ()); /* { dg-warning "test" } */
+
+/* { dg-begin-multiline-output "" }
+14 |   x = (first_function ()
+   |        ~~~~~~~~~~~~~~~~~
+15 |        + second_function ());
+   |        ^ ~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+#endif
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 5a19fc9..b2f8507 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -72,8 +72,10 @@  set plugin_test_list [list \
     { diagnostic_plugin_test_show_locus.c \
 	  diagnostic-test-show-locus-bw.c \
 	  diagnostic-test-show-locus-color.c \
+	  diagnostic-test-show-locus-bw-line-numbers.c \
+	  diagnostic-test-show-locus-color-line-numbers.c \
 	  diagnostic-test-show-locus-parseable-fixits.c \
-	  diagnostic-test-show-locus-generate-patch.c } \
+	  diagnostic-test-show-locus-generate-patch.c }\
     { diagnostic_plugin_test_tree_expression_range.c \
 	  diagnostic-test-expressions-1.c } \
     { diagnostic_plugin_show_trees.c \
diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index 1e11dc9..df36c34 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -21,7 +21,7 @@  load_lib multiline.exp
 if ![info exists TEST_ALWAYS_FLAGS] {
     set TEST_ALWAYS_FLAGS ""
 }
-set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fdiagnostics-color=never $TEST_ALWAYS_FLAGS"
+set TEST_ALWAYS_FLAGS "-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never $TEST_ALWAYS_FLAGS"
 
 proc prune_gcc_output { text } {
     global srcdir
diff --git a/gcc/toplev.c b/gcc/toplev.c
index f23e8ae..aa943a8 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1112,6 +1112,8 @@  general_init (const char *argv0, bool init_signals)
 
   global_dc->show_caret
     = global_options_init.x_flag_diagnostics_show_caret;
+  global_dc->show_line_numbers_p
+    = global_options_init.x_flag_diagnostics_show_line_numbers;
   global_dc->show_option_requested
     = global_options_init.x_flag_diagnostics_show_option;
   global_dc->show_column