diagnostics: add minimum width to left margin for line numbers
authorDavid Malcolm <dmalcolm@redhat.com>
Mon, 15 Oct 2018 22:16:59 +0000 (22:16 +0000)
committerDavid Malcolm <dmalcolm@gcc.gnu.org>
Mon, 15 Oct 2018 22:16:59 +0000 (22:16 +0000)
This patch adds a minimum width to the left margin used for printing
line numbers.   I set the default to 6.  Hence rather than:

some-filename:9:1: some message
9 | some source text
  | ^~~~~~~~~~~~~~~~
some-filename:10:1: another message
10 | more source text
   | ^~~~~~~~~~~~~~~~

we now print:

some-filename:9:42: some message
    9 | some source text
      | ^~~~~~~~~~~~~~~~
some-filename:10:42: another message
   10 | more source text
      | ^~~~~~~~~~~~~~~~

This implicitly fixes issues with margins failing to line up due
to different lengths of the number when we haven't read the full
file yet and so don't know the highest possible line number, for
line numbers up to 99999.

Doing so adds some whitespace on the left-hand side, for non-huge
files, at least.  I believe that this makes it easier to see where each
diagnostic starts, by visually breaking things up at the leftmost
column; my hope is to make it easier for the eye to see the different
diagnostics as if they were different "paragraphs".

gcc/ChangeLog:
* common.opt (fdiagnostics-minimum-margin-width=): New option.
* diagnostic-show-locus.c (layout::layout): Apply the minimum
margin width.
(layout::start_annotation_line): Only print up to 3 of the
margin character, to avoid touching the left-hand side.
(selftest::test_diagnostic_show_locus_fixit_lines): Update for
minimum margin width, as set by test_diagnostic_context's ctor.
(selftest::test_fixit_insert_containing_newline): Likewise.
(selftest::test_fixit_insert_containing_newline_2): Likewise.
(selftest::test_line_numbers_multiline_range): Clear
dc.min_margin_width.
* diagnostic.c (diagnostic_initialize): Initialize
min_margin_width.
* diagnostic.h (struct diagnostic_context): Add field
"min_margin_width".
* doc/invoke.texi: Add -fdiagnostics-minimum-margin-width=.
* opts.c (common_handle_option): Handle
OPT_fdiagnostics_minimum_margin_width_.
* selftest-diagnostic.c
(selftest::test_diagnostic_context::test_diagnostic_context):
Initialize min_margin_width to 6.
* toplev.c (general_init): Initialize global_dc->min_margin_width.

gcc/testsuite/ChangeLog:
* gcc.dg/missing-header-fixit-3.c: Update expected indentation
to reflect minimum margin width.
* gcc.dg/missing-header-fixit-4.c: Likewise.
* gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c:
Likewise.
* gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c:
Likewise.
* gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers-2.c:
New test.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add it.

From-SVN: r265178

16 files changed:
gcc/ChangeLog
gcc/common.opt
gcc/diagnostic-show-locus.c
gcc/diagnostic.c
gcc/diagnostic.h
gcc/doc/invoke.texi
gcc/opts.c
gcc/selftest-diagnostic.c
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/missing-header-fixit-3.c
gcc/testsuite/gcc.dg/missing-header-fixit-4.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers-2.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c
gcc/testsuite/gcc.dg/plugin/plugin.exp
gcc/toplev.c

index 82f3f02dabc398ca8a26ba85478f586fea2d1a6f..789e43b238848bafc65c297f45d253ee4ed1d24d 100644 (file)
@@ -1,3 +1,28 @@
+2018-10-15  David Malcolm  <dmalcolm@redhat.com>
+
+       * common.opt (fdiagnostics-minimum-margin-width=): New option.
+       * diagnostic-show-locus.c (layout::layout): Apply the minimum
+       margin width.
+       (layout::start_annotation_line): Only print up to 3 of the
+       margin character, to avoid touching the left-hand side.
+       (selftest::test_diagnostic_show_locus_fixit_lines): Update for
+       minimum margin width, as set by test_diagnostic_context's ctor.
+       (selftest::test_fixit_insert_containing_newline): Likewise.
+       (selftest::test_fixit_insert_containing_newline_2): Likewise.
+       (selftest::test_line_numbers_multiline_range): Clear
+       dc.min_margin_width.
+       * diagnostic.c (diagnostic_initialize): Initialize
+       min_margin_width.
+       * diagnostic.h (struct diagnostic_context): Add field
+       "min_margin_width".
+       * doc/invoke.texi: Add -fdiagnostics-minimum-margin-width=.
+       * opts.c (common_handle_option): Handle
+       OPT_fdiagnostics_minimum_margin_width_.
+       * selftest-diagnostic.c
+       (selftest::test_diagnostic_context::test_diagnostic_context):
+       Initialize min_margin_width to 6.
+       * toplev.c (general_init): Initialize global_dc->min_margin_width.
+
 2018-10-15  David Malcolm  <dmalcolm@redhat.com>
 
        * gcc-rich-location.h (gcc_rich_location::add_location_if_nearby):
index 53aac194899bcaf856e915e67e8af48089c42d02..2971dc21b1fee55e8aee9a313023635c093b23a4 100644 (file)
@@ -1281,6 +1281,10 @@ fdiagnostics-show-option
 Common Var(flag_diagnostics_show_option) Init(1)
 Amend appropriate diagnostic messages with the command line option that controls them.
 
+fdiagnostics-minimum-margin-width=
+Common Joined UInteger Var(diagnostics_minimum_margin_width) Init(6)
+Set minimum width of left margin of source code when showing source
+
 fdisable-
 Common Joined RejectNegative Var(common_deferred_options) Defer
 -fdisable-[tree|rtl|ipa]-<pass>=range1+range2 disables an optimization pass.
index 43a49ea89137504412443f8d38c547db0867d350..a42ff819512ff1adbc170fbef5b91f47de2a4e93 100644 (file)
@@ -930,6 +930,9 @@ layout::layout (diagnostic_context * context,
   /* If we're showing jumps in the line-numbering, allow at least 3 chars.  */
   if (m_line_spans.length () > 1)
     m_linenum_width = MAX (m_linenum_width, 3);
+  /* If there's a minimum margin width, apply it (subtracting 1 for the space
+     after the line number.  */
+  m_linenum_width = MAX (m_linenum_width, context->min_margin_width - 1);
 
   /* Adjust m_x_offset.
      Center the primary caret to fit in max_width; all columns
@@ -1386,7 +1389,12 @@ layout::start_annotation_line (char margin_char) const
 {
   if (m_show_line_numbers_p)
     {
-      for (int i = 0; i < m_linenum_width; i++)
+      /* Print the margin.  If MARGIN_CHAR != ' ', then print up to 3
+        of it, right-aligned, padded with spaces.  */
+      int i;
+      for (i = 0; i < m_linenum_width - 3; i++)
+       pp_space (m_pp);
+      for (; i < m_linenum_width; i++)
        pp_character (m_pp, margin_char);
       pp_string (m_pp, " |");
     }
@@ -3027,12 +3035,12 @@ test_diagnostic_show_locus_fixit_lines (const line_table_case &case_)
     dc.show_line_numbers_p = true;
     diagnostic_show_locus (&dc, &richloc, DK_ERROR);
     ASSERT_STREQ ("\n"
-                 "  3 |                        y\n"
-                 "    |                        .\n"
-                 "....\n"
-                 "  6 |                         : 0.0};\n"
-                 "    |                         ^\n"
-                 "    |                         =\n",
+                 "    3 |                        y\n"
+                 "      |                        .\n"
+                 "......\n"
+                 "    6 |                         : 0.0};\n"
+                 "      |                         ^\n"
+                 "      |                         =\n",
                  pp_formatted_text (dc.printer));
   }
 }
@@ -3523,10 +3531,10 @@ test_fixit_insert_containing_newline (const line_table_case &case_)
       dc.show_line_numbers_p = true;
       diagnostic_show_locus (&dc, &richloc, DK_ERROR);
       ASSERT_STREQ ("\n"
-                   "2 |       x = a;\n"
-                   "+ |+      break;\n"
-                   "3 |     case 'b':\n"
-                   "  |     ^~~~~~~~~\n",
+                   "    2 |       x = a;\n"
+                   "  +++ |+      break;\n"
+                   "    3 |     case 'b':\n"
+                   "      |     ^~~~~~~~~\n",
                    pp_formatted_text (dc.printer));
     }
   }
@@ -3605,11 +3613,11 @@ test_fixit_insert_containing_newline_2 (const line_table_case &case_)
     dc.show_line_numbers_p = true;
     diagnostic_show_locus (&dc, &richloc, DK_ERROR);
     ASSERT_STREQ ("\n"
-                 "+ |+#include <stdio.h>\n"
-                 "1 | test (int ch)\n"
-                 "2 | {\n"
-                 "3 |  putchar (ch);\n"
-                 "  |  ^~~~~~~\n",
+                 "  +++ |+#include <stdio.h>\n"
+                 "    1 | test (int ch)\n"
+                 "    2 | {\n"
+                 "    3 |  putchar (ch);\n"
+                 "      |  ^~~~~~~\n",
                  pp_formatted_text (dc.printer));
   }
 }
@@ -3734,6 +3742,7 @@ test_line_numbers_multiline_range ()
 
   test_diagnostic_context dc;
   dc.show_line_numbers_p = true;
+  dc.min_margin_width = 0;
   gcc_rich_location richloc (loc);
   diagnostic_show_locus (&dc, &richloc, DK_ERROR);
   ASSERT_STREQ ("\n"
index 8575065069b5aa05285b6070c9e8328cdc211154..a572c084aac0fb8b2b4ab77388a14ea0bc0b7fde 100644 (file)
@@ -177,6 +177,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts)
   context->colorize_source_p = false;
   context->show_labels_p = false;
   context->show_line_numbers_p = false;
+  context->min_margin_width = 0;
   context->show_ruler_p = false;
   context->parseable_fixits_p = false;
   context->edit_context_ptr = NULL;
index d4c605ea8f356724a70a8aedd94d25964a939424..3498a9ba7bb7a9aa9235ca822700700a8f9412f9 100644 (file)
@@ -211,6 +211,10 @@ struct diagnostic_context
      showing line numbers?  */
   bool show_line_numbers_p;
 
+  /* If printing source code, what should the minimum width of the margin
+     be?  Line numbers will be right-aligned, and padded to this width.  */
+  int min_margin_width;
+
   /* Usable by plugins; if true, print a debugging ruler above the
      source output.  */
   bool show_ruler_p;
index 802cc642453aef2d2c516bcbda22246252ec87c1..06a00a29de73aa509b6a15ebb34dfc182cf94cd2 100644 (file)
@@ -270,6 +270,7 @@ Objective-C and Objective-C++ Dialects}.
 -fdiagnostics-color=@r{[}auto@r{|}never@r{|}always@r{]}  @gol
 -fno-diagnostics-show-option  -fno-diagnostics-show-caret @gol
 -fno-diagnostics-show-labels -fno-diagnostics-show-line-numbers @gol
+-fdiagnostics-minimum-margin-width=@var{width} @gol
 -fdiagnostics-parseable-fixits  -fdiagnostics-generate-patch @gol
 -fdiagnostics-show-template-tree -fno-elide-type @gol
 -fno-show-column}
@@ -3819,6 +3820,11 @@ 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-minimum-margin-width=@var{width}
+@opindex -fdiagnostics-minimum-margin-width
+This option controls the minimum width of the left margin printed by
+@option{-fdiagnostics-show-line-numbers}.  It defaults to 6.
+
 @item -fdiagnostics-parseable-fixits
 @opindex fdiagnostics-parseable-fixits
 Emit fix-it hints in a machine-parseable format, suitable for consumption
index dc12c2ecefd074db13fd830f255344493bdae9d9..3b61e17a7ab07b0bff93f6d8f0bdafaf80d7a8e5 100644 (file)
@@ -2228,6 +2228,10 @@ common_handle_option (struct gcc_options *opts,
       dc->show_option_requested = value;
       break;
 
+    case OPT_fdiagnostics_minimum_margin_width_:
+      dc->min_margin_width = value;
+      break;
+
     case OPT_fdump_:
       /* Deferred.  */
       break;
index f3c255e3346409c2fbf4a256d752389f9875dcde..4a7f0dec4c92a0e3c1135cb4a8a712a780c2a15b 100644 (file)
@@ -40,6 +40,7 @@ test_diagnostic_context::test_diagnostic_context ()
   show_labels_p = true;
   show_column = true;
   start_span = start_span_cb;
+  min_margin_width = 6;
 }
 
 test_diagnostic_context::~test_diagnostic_context ()
index 46085c0cecfe36a59b9f2723af9a0ccb1bd92ccf..37a9ed3f5b7b7c26e0a3333783823d3eaa11e843 100644 (file)
@@ -1,3 +1,16 @@
+2018-10-15  David Malcolm  <dmalcolm@redhat.com>
+
+       * gcc.dg/missing-header-fixit-3.c: Update expected indentation
+       to reflect minimum margin width.
+       * gcc.dg/missing-header-fixit-4.c: Likewise.
+       * gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c:
+       Likewise.
+       * gcc.dg/plugin/diagnostic-test-show-locus-color-line-numbers.c:
+       Likewise.
+       * gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers-2.c:
+       New test.
+       * gcc.dg/plugin/plugin.exp (plugin_test_list): Add it.
+
 2018-10-15  Tobias Burnus  <burnus@net-b.de>
 
        PR fortran/87597
index a692b4d21b392c323dc4c939257d4487434ef821..dd53bf65d3c86dc9531e8a55e6148ec04f5016d4 100644 (file)
@@ -10,12 +10,12 @@ void test (int i, int j)
   /* { dg-message "include '<stdio.h>' or provide a declaration of 'printf'" "" { target *-*-* } 1 } */
 #if 0
 /* { dg-begin-multiline-output "" }
-9 |   printf ("%i of %i\n", i, j);
-  |   ^~~~~~
+    9 |   printf ("%i of %i\n", i, j);
+      |   ^~~~~~
    { dg-end-multiline-output "" } */
 /* { dg-begin-multiline-output "" }
-+ |+#include <stdio.h>
-1 | /* Example of a fix-it hint that adds a #include directive,
+  +++ |+#include <stdio.h>
+    1 | /* Example of a fix-it hint that adds a #include directive,
    { dg-end-multiline-output "" } */
 #endif
 }
index 0ed3e2c2922295da0cfe1801534e1cc7bad845d8..942897d8c79fd6a615b1ab367cec10f54e55db32 100644 (file)
@@ -11,13 +11,13 @@ void test (int i, int j)
   /* { dg-warning "implicit declaration of function" "" { target *-*-* } printf } */
   /* { dg-warning "incompatible implicit declaration" "" { target *-*-* } printf } */
   /* { dg-begin-multiline-output "" }
-10 |   printf ("%i of %i\n", i, j);
-   |   ^~~~~~
+   10 |   printf ("%i of %i\n", i, j);
+      |   ^~~~~~
    { dg-end-multiline-output "" } */
   /* { dg-message "include '<stdio.h>' or provide a declaration of 'printf'" "" { target *-*-* } 4 } */
   /* { dg-begin-multiline-output "" }
-3 | #include "empty.h"
-+ |+#include <stdio.h>
-4 | int the_next_line;
+    3 | #include "empty.h"
+  +++ |+#include <stdio.h>
+    4 | int the_next_line;
    { dg-end-multiline-output "" } */
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers-2.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers-2.c
new file mode 100644 (file)
index 0000000..a9d022a
--- /dev/null
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* Verify "-fdiagnostics-minimum-margin-width=0".  */
+/* { dg-options "-O -fdiagnostics-show-caret -fdiagnostics-show-line-numbers -fdiagnostics-minimum-margin-width=0" } */
+
+/* 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 "" }
+15 |   myvar = myvar.x;
+   |           ~~~~~^~
+   { dg-end-multiline-output "" } */
+#endif
+}
index 63e585528faae686918a765be2a87567388027c8..1e8f73bdb5d92763b7c74864a6696767291b02b0 100644 (file)
@@ -14,8 +14,8 @@ void test_simple (void)
   myvar = myvar.x; /* { dg-warning "test" } */
 
 /* { dg-begin-multiline-output "" }
-14 |   myvar = myvar.x;
-   |           ~~~~~^~
+   14 |   myvar = myvar.x;
+      |           ~~~~~^~
    { dg-end-multiline-output "" } */
 #endif
 }
@@ -27,12 +27,12 @@ void test_multiline (void)
        + second_function ()); /* { dg-warning "test" } */
 
 /* { dg-begin-multiline-output "" }
-26 |   x = (first_function ()
-   |        ~~~~~~~~~~~~~~~~~
-27 |        + second_function ());
-   |        ^ ~~~~~~~~~~~~~~~~~~
-   |        |
-   |        label
+   26 |   x = (first_function ()
+      |        ~~~~~~~~~~~~~~~~~
+   27 |        + second_function ());
+      |        ^ ~~~~~~~~~~~~~~~~~~
+      |        |
+      |        label
    { dg-end-multiline-output "" } */
 #endif
 }
@@ -42,14 +42,14 @@ 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
-43 |                                          float f = foo * bar;
-   |                                                    ~~~~^~~~~
-   |                                                        |
-   |                                                        label
-   |                                                    bar * foo
+      |        0         0         0         0         0         1         1  
+      |        5         6         7         8         9         0         1  
+      | 3456789012345678901234567890123456789012345678901234567890123456789012
+   43 |                                       float f = foo * bar;
+      |                                                 ~~~~^~~~~
+      |                                                     |
+      |                                                     label 0
+      |                                                 bar * foo
    { dg-end-multiline-output "" } */
 #endif
 }
@@ -62,9 +62,9 @@ void test_fixit_insert (void)
 #if 0
    int a[2][2] = { 0, 1 , 2, 3 }; /* { dg-warning "insertion hints" } */
 /* { dg-begin-multiline-output "" }
-63 |    int a[2][2] = { 0, 1 , 2, 3 };
-   |                    ^~~~
-   |                    {   }
+   63 |    int a[2][2] = { 0, 1 , 2, 3 };
+      |                    ^~~~
+      |                    {   }
    { dg-end-multiline-output "" } */
 #endif
 }
@@ -76,9 +76,9 @@ void test_fixit_remove (void)
 #if 0
   int a;; /* { dg-warning "example of a removal hint" } */
 /* { dg-begin-multiline-output "" }
-77 |   int a;;
-   |         ^
-   |         -
+   77 |   int a;;
+      |         ^
+      |         -
    { dg-end-multiline-output "" } */
 #endif
 }
@@ -90,9 +90,9 @@ void test_fixit_replace (void)
 #if 0
   gtk_widget_showall (dlg); /* { dg-warning "example of a replacement hint" } */
 /* { dg-begin-multiline-output "" }
-91 |   gtk_widget_showall (dlg);
-   |   ^~~~~~~~~~~~~~~~~~
-   |   gtk_widget_show_all
+   91 |   gtk_widget_showall (dlg);
+      |   ^~~~~~~~~~~~~~~~~~
+      |   gtk_widget_show_all
    { dg-end-multiline-output "" } */
 #endif
 }
@@ -111,10 +111,10 @@ void test_fixit_insert_newline (void)
       x = b;
     }
 /* { dg-begin-multiline-output "" }
-109 |       x = a;
-+++ |+      break;
-110 |     case 'b':
-    |     ^~~~~~~~
+  109 |       x = a;
+  +++ |+      break;
+  110 |     case 'b':
+      |     ^~~~~~~~
    { dg-end-multiline-output "" } */
 #endif
 }
index 0453c52b2ccfa2328f6a30e9acbf5cd4ea141c2b..bb83862d550d96a910555661c4dac0c2eff96ce6 100644 (file)
@@ -15,12 +15,12 @@ void test_multiline (void)
        + second_function ()); /* { dg-warning "test" } */
 
 /* { dg-begin-multiline-output "" }
-14 |   x = (\e[32m\e[Kfirst_function ()\e[m\e[K
-   |        \e[32m\e[K~~~~~~~~~~~~~~~~~\e[m\e[K
-15 |        \e[01;35m\e[K+\e[m\e[K \e[34m\e[Ksecond_function ()\e[m\e[K);
-   |        \e[01;35m\e[K^\e[m\e[K \e[34m\e[K~~~~~~~~~~~~~~~~~~\e[m\e[K
-   |        \e[01;35m\e[K|\e[m\e[K
-   |        \e[01;35m\e[Klabel\e[m\e[K
+   14 |   x = (\e[32m\e[Kfirst_function ()\e[m\e[K
+      |        \e[32m\e[K~~~~~~~~~~~~~~~~~\e[m\e[K
+   15 |        \e[01;35m\e[K+\e[m\e[K \e[34m\e[Ksecond_function ()\e[m\e[K);
+      |        \e[01;35m\e[K^\e[m\e[K \e[34m\e[K~~~~~~~~~~~~~~~~~~\e[m\e[K
+      |        \e[01;35m\e[K|\e[m\e[K
+      |        \e[01;35m\e[Klabel\e[m\e[K
    { dg-end-multiline-output "" } */
 #endif
 }
index 1d06c04562e304ff7a985f228684965a30beef96..d92ede79c0d5b0529981826340964bec3b3dd580 100644 (file)
@@ -76,6 +76,7 @@ set plugin_test_list [list \
          diagnostic-test-show-locus-color.c \
          diagnostic-test-show-locus-no-labels.c \
          diagnostic-test-show-locus-bw-line-numbers.c \
+         diagnostic-test-show-locus-bw-line-numbers-2.c \
          diagnostic-test-show-locus-color-line-numbers.c \
          diagnostic-test-show-locus-parseable-fixits.c \
          diagnostic-test-show-locus-generate-patch.c }\
index 9fb83d4e43f2ce19ad511a7d54e37c7ecbf00b2b..d7ea11abf5349460d2ad2f53946714a68960cc7d 100644 (file)
@@ -1120,6 +1120,8 @@ general_init (const char *argv0, bool init_signals)
     = global_options_init.x_flag_diagnostics_show_line_numbers;
   global_dc->show_option_requested
     = global_options_init.x_flag_diagnostics_show_option;
+  global_dc->min_margin_width
+    = global_options_init.x_diagnostics_minimum_margin_width;
   global_dc->show_column
     = global_options_init.x_flag_show_column;
   global_dc->internal_error = internal_error_function;