From 2e1ed6ea7ea1e524f2f5b5035327ed280fff475d Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 19 Oct 2020 20:19:48 -0400 Subject: [PATCH] Add GCC_EXTRA_DIAGNOSTIC_OUTPUT environment variable for fix-it hints GCC has had the ability to emit fix-it hints in machine-readable form since GCC 7 via -fdiagnostics-parseable-fixits and -fdiagnostics-generate-patch. The former emits additional specially-formatted lines to stderr; the option and its format were directly taken from a pre-existing option in clang. Ideally this could be used by IDEs so that the user can select specific fix-it hints and have the IDE apply them to the user's source code (perhaps turning them into clickable elements, perhaps with an "Apply All" option, etc). Eclipse CDT has supported this option in this way for a few years: https://bugs.eclipse.org/bugs/show_bug.cgi?id=497670 As a user of Emacs I would like Emacs to support such a feature. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25987 tracks supporting GCC fix-it output in Emacs. The discussion there identifies two issues with the existing option: (a) columns in the output are specified as byte-offsets within the line (for exact compatibility with the option in clang), whereas emacs would prefer to consume them as what GCC 11 calls "display columns". https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-column-unit (b) injecting a command-line option into the build is a fiddly manual step, varying between build systems. It's far easier for the user if Emacs simply sets an environment variable when compiling, GCC uses this to enable the option if it recognizes the value, and the emacs compilation buffer decodes the additional lines of output and adds appropriate widgets. In some ways it is a workaround for not having a language server. Doing it this way means that for the various combinations of older and newer GCC and older and newer Emacs that a sufficiently modern combination of both can automatically support the rich fix-it UI, whereas other combinations will either not provide the envvar, or silently ignore it, gracefully doing nothing extra. Hence this patch adds a new GCC_EXTRA_DIAGNOSTIC_OUTPUT environment variable to GCC which enables output of machine-parseable fix-it hints. GCC_EXTRA_DIAGNOSTIC_OUTPUT=fixits-v1 is equivalent to the existing -fdiagnostics-parseable-fixits option. GCC_EXTRA_DIAGNOSTIC_OUTPUT=fixits-v2 is the same, but changes the column output mode to "display columns" rather than bytes, as required by Emacs. gcc/ChangeLog: * diagnostic.c (diagnostic_initialize): Eliminate parseable_fixits_p in favor of initializing extra_output_kind from GCC_EXTRA_DIAGNOSTIC_OUTPUT. (convert_column_unit): New function, split out from... (diagnostic_converted_column): ...this. (print_parseable_fixits): Add "column_unit" and "tabstop" params. Use them to call convert_column_unit on the column values. (diagnostic_report_diagnostic): Eliminate conditional on parseable_fixits_p in favor of a switch statement on extra_output_kind, passing the appropriate values to the new params of print_parseable_fixits. (selftest::test_print_parseable_fixits_none): Update for new params of print_parseable_fixits. (selftest::test_print_parseable_fixits_insert): Likewise. (selftest::test_print_parseable_fixits_remove): Likewise. (selftest::test_print_parseable_fixits_replace): Likewise. (selftest::test_print_parseable_fixits_bytes_vs_display_columns): New. (selftest::diagnostic_c_tests): Call it. * diagnostic.h (enum diagnostics_extra_output_kind): New. (diagnostic_context::parseable_fixits_p): Delete field in favor of... (diagnostic_context::extra_output_kind): ...this new field. * doc/invoke.texi (Environment Variables): Add GCC_EXTRA_DIAGNOSTIC_OUTPUT. * opts.c (common_handle_option): Update handling of OPT_fdiagnostics_parseable_fixits for change to diagnostic_context fields. --- gcc/diagnostic.c | 138 ++++++++++++++++++++++++++++++++++++-------- gcc/diagnostic.h | 23 +++++++- gcc/doc/invoke.texi | 19 ++++++ gcc/opts.c | 4 +- 4 files changed, 155 insertions(+), 29 deletions(-) diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 1b6c9845892..7724f0e050b 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -219,7 +219,14 @@ diagnostic_initialize (diagnostic_context *context, int n_opts) context->show_line_numbers_p = false; context->min_margin_width = 0; context->show_ruler_p = false; - context->parseable_fixits_p = false; + if (const char *var = getenv ("GCC_EXTRA_DIAGNOSTIC_OUTPUT")) + { + if (!strcmp (var, "fixits-v1")) + context->extra_output_kind = EXTRA_DIAGNOSTIC_OUTPUT_fixits_v1; + else if (!strcmp (var, "fixits-v2")) + context->extra_output_kind = EXTRA_DIAGNOSTIC_OUTPUT_fixits_v2; + /* Silently ignore unrecognized values. */ + } context->column_unit = DIAGNOSTICS_COLUMN_UNIT_DISPLAY; context->column_origin = 1; context->tabstop = 8; @@ -358,29 +365,40 @@ diagnostic_get_color_for_kind (diagnostic_t kind) } /* Given an expanded_location, convert the column (which is in 1-based bytes) - to the requested units and origin. Return -1 if the column is - invalid (<= 0). */ -int -diagnostic_converted_column (diagnostic_context *context, expanded_location s) + to the requested units, without converting the origin. + Return -1 if the column is invalid (<= 0). */ + +static int +convert_column_unit (enum diagnostics_column_unit column_unit, + int tabstop, + expanded_location s) { if (s.column <= 0) return -1; - int one_based_col; - switch (context->column_unit) + switch (column_unit) { + default: + gcc_unreachable (); + case DIAGNOSTICS_COLUMN_UNIT_DISPLAY: - one_based_col = location_compute_display_column (s, context->tabstop); - break; + return location_compute_display_column (s, tabstop); case DIAGNOSTICS_COLUMN_UNIT_BYTE: - one_based_col = s.column; - break; - - default: - gcc_unreachable (); + return s.column; } +} +/* Given an expanded_location, convert the column (which is in 1-based bytes) + to the requested units and origin. Return -1 if the column is + invalid (<= 0). */ +int +diagnostic_converted_column (diagnostic_context *context, expanded_location s) +{ + int one_based_col + = convert_column_unit (context->column_unit, context->tabstop, s); + if (one_based_col <= 0) + return -1; return one_based_col + (context->column_origin - 1); } @@ -920,11 +938,16 @@ print_escaped_string (pretty_printer *pp, const char *text) pp_character (pp, '"'); } -/* Implementation of -fdiagnostics-parseable-fixits. Print a - machine-parseable version of all fixits in RICHLOC to PP. */ +/* Implementation of -fdiagnostics-parseable-fixits and + GCC_EXTRA_DIAGNOSTIC_OUTPUT. + Print a machine-parseable version of all fixits in RICHLOC to PP, + using COLUMN_UNIT to express columns. + Use TABSTOP when handling DIAGNOSTICS_COLUMN_UNIT_DISPLAY. */ static void -print_parseable_fixits (pretty_printer *pp, rich_location *richloc) +print_parseable_fixits (pretty_printer *pp, rich_location *richloc, + enum diagnostics_column_unit column_unit, + int tabstop) { gcc_assert (pp); gcc_assert (richloc); @@ -942,9 +965,13 @@ print_parseable_fixits (pretty_printer *pp, rich_location *richloc) /* For compatibility with clang, print as a half-open range. */ location_t next_loc = hint->get_next_loc (); expanded_location next_exploc = expand_location (next_loc); + int start_col + = convert_column_unit (column_unit, tabstop, start_exploc); + int next_col + = convert_column_unit (column_unit, tabstop, next_exploc); pp_printf (pp, ":{%i:%i-%i:%i}:", - start_exploc.line, start_exploc.column, - next_exploc.line, next_exploc.column); + start_exploc.line, start_col, + next_exploc.line, next_col); print_escaped_string (pp, hint->get_string ()); pp_newline (pp); } @@ -1210,10 +1237,22 @@ diagnostic_report_diagnostic (diagnostic_context *context, if (context->show_option_requested) print_option_information (context, diagnostic, orig_diag_kind); (*diagnostic_finalizer (context)) (context, diagnostic, orig_diag_kind); - if (context->parseable_fixits_p) + switch (context->extra_output_kind) { - print_parseable_fixits (context->printer, diagnostic->richloc); + default: + break; + case EXTRA_DIAGNOSTIC_OUTPUT_fixits_v1: + print_parseable_fixits (context->printer, diagnostic->richloc, + DIAGNOSTICS_COLUMN_UNIT_BYTE, + context->tabstop); + pp_flush (context->printer); + break; + case EXTRA_DIAGNOSTIC_OUTPUT_fixits_v2: + print_parseable_fixits (context->printer, diagnostic->richloc, + DIAGNOSTICS_COLUMN_UNIT_DISPLAY, + context->tabstop); pp_flush (context->printer); + break; } diagnostic_action_after_output (context, diagnostic->kind); diagnostic->x_data = NULL; @@ -2012,7 +2051,7 @@ test_print_parseable_fixits_none () pretty_printer pp; rich_location richloc (line_table, UNKNOWN_LOCATION); - print_parseable_fixits (&pp, &richloc); + print_parseable_fixits (&pp, &richloc, DIAGNOSTICS_COLUMN_UNIT_BYTE, 8); ASSERT_STREQ ("", pp_formatted_text (&pp)); } @@ -2031,7 +2070,7 @@ test_print_parseable_fixits_insert () location_t where = linemap_position_for_column (line_table, 10); richloc.add_fixit_insert_before (where, "added content"); - print_parseable_fixits (&pp, &richloc); + print_parseable_fixits (&pp, &richloc, DIAGNOSTICS_COLUMN_UNIT_BYTE, 8); ASSERT_STREQ ("fix-it:\"test.c\":{5:10-5:10}:\"added content\"\n", pp_formatted_text (&pp)); } @@ -2053,7 +2092,7 @@ test_print_parseable_fixits_remove () where.m_finish = linemap_position_for_column (line_table, 20); richloc.add_fixit_remove (where); - print_parseable_fixits (&pp, &richloc); + print_parseable_fixits (&pp, &richloc, DIAGNOSTICS_COLUMN_UNIT_BYTE, 8); ASSERT_STREQ ("fix-it:\"test.c\":{5:10-5:21}:\"\"\n", pp_formatted_text (&pp)); } @@ -2075,11 +2114,59 @@ test_print_parseable_fixits_replace () where.m_finish = linemap_position_for_column (line_table, 20); richloc.add_fixit_replace (where, "replacement"); - print_parseable_fixits (&pp, &richloc); + print_parseable_fixits (&pp, &richloc, DIAGNOSTICS_COLUMN_UNIT_BYTE, 8); ASSERT_STREQ ("fix-it:\"test.c\":{5:10-5:21}:\"replacement\"\n", pp_formatted_text (&pp)); } +/* Verify that print_parseable_fixits correctly handles + DIAGNOSTICS_COLUMN_UNIT_BYTE vs DIAGNOSTICS_COLUMN_UNIT_COLUMN. */ + +static void +test_print_parseable_fixits_bytes_vs_display_columns () +{ + line_table_test ltt; + rich_location richloc (line_table, UNKNOWN_LOCATION); + + /* 1-based byte offsets: 12345677778888999900001234567. */ + const char *const content = "smile \xf0\x9f\x98\x82 colour\n"; + /* 1-based display cols: 123456[......7-8.....]9012345. */ + const int tabstop = 8; + + temp_source_file tmp (SELFTEST_LOCATION, ".c", content); + const char *const fname = tmp.get_filename (); + + linemap_add (line_table, LC_ENTER, false, fname, 0); + linemap_line_start (line_table, 1, 100); + linemap_add (line_table, LC_LEAVE, false, NULL, 0); + source_range where; + where.m_start = linemap_position_for_column (line_table, 12); + where.m_finish = linemap_position_for_column (line_table, 17); + richloc.add_fixit_replace (where, "color"); + + const int buf_len = strlen (fname) + 100; + char *const expected = XNEWVEC (char, buf_len); + + { + pretty_printer pp; + print_parseable_fixits (&pp, &richloc, DIAGNOSTICS_COLUMN_UNIT_BYTE, + tabstop); + snprintf (expected, buf_len, + "fix-it:\"%s\":{1:12-1:18}:\"color\"\n", fname); + ASSERT_STREQ (expected, pp_formatted_text (&pp)); + } + { + pretty_printer pp; + print_parseable_fixits (&pp, &richloc, DIAGNOSTICS_COLUMN_UNIT_DISPLAY, + tabstop); + snprintf (expected, buf_len, + "fix-it:\"%s\":{1:10-1:16}:\"color\"\n", fname); + ASSERT_STREQ (expected, pp_formatted_text (&pp)); + } + + XDELETEVEC (expected); +} + /* Verify that diagnostic_get_location_text (..., SHOW_COLUMN) generates EXPECTED_LOC_TEXT, given FILENAME, LINE, COLUMN, with @@ -2202,6 +2289,7 @@ diagnostic_c_tests () test_print_parseable_fixits_insert (); test_print_parseable_fixits_remove (); test_print_parseable_fixits_replace (); + test_print_parseable_fixits_bytes_vs_display_columns (); test_diagnostic_get_location_text (); test_num_digits (); diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h index 1d2fa119643..e88d63c2128 100644 --- a/gcc/diagnostic.h +++ b/gcc/diagnostic.h @@ -69,6 +69,22 @@ enum diagnostic_path_format DPF_HTML }; +/* An enum for capturing values of GCC_EXTRA_DIAGNOSTIC_OUTPUT, + and for -fdiagnostics-parseable-fixits. */ + +enum diagnostics_extra_output_kind +{ + /* No extra output, or an unrecognized value. */ + EXTRA_DIAGNOSTIC_OUTPUT_none, + + /* Emit fix-it hints using the "fixits-v1" format, equivalent to + -fdiagnostics-parseable-fixits. */ + EXTRA_DIAGNOSTIC_OUTPUT_fixits_v1, + + /* Emit fix-it hints using the "fixits-v2" format. */ + EXTRA_DIAGNOSTIC_OUTPUT_fixits_v2 +}; + /* A diagnostic is described by the MESSAGE to send, the FILE and LINE of its context and its KIND (ice, error, warning, note, ...) See complete list in diagnostic.def. */ @@ -296,9 +312,10 @@ struct diagnostic_context source output. */ bool show_ruler_p; - /* If true, print fixits in machine-parseable form after the - rest of the diagnostic. */ - bool parseable_fixits_p; + /* Used to specify additional diagnostic output to be emitted after the + rest of the diagnostic. This is for implementing + -fdiagnostics-parseable-fixits and GCC_EXTRA_DIAGNOSTIC_OUTPUT. */ + enum diagnostics_extra_output_kind extra_output_kind; /* What units to use when outputting the column number. */ enum diagnostics_column_unit column_unit; diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 55f7a9854aa..f83fe8f735c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -31998,6 +31998,25 @@ Recognize EUCJP characters. If @env{LANG} is not defined, or if it has some other value, then the compiler uses @code{mblen} and @code{mbtowc} as defined by the default locale to recognize and translate multibyte characters. + +@item GCC_EXTRA_DIAGNOSTIC_OUTPUT +If @env{GCC_EXTRA_DIAGNOSTIC_OUTPUT} is set to one of the following values, +then additional text will be emitted to stderr when fix-it hints are +emitted. @option{-fdiagnostics-parseable-fixits} and +@option{-fno-diagnostics-parseable-fixits} take precedence over this +environment variable. + +@table @samp +@item fixits-v1 +Emit parseable fix-it hints, equivalent to +@option{-fdiagnostics-parseable-fixits}. In particular, columns are +expressed as a count of bytes, starting at byte 1 for the initial column. + +@item fixits-v2 +As @code{fixits-v1}, but columns are expressed as display columns, +as per @option{-fdiagnostics-column-unit=display}. +@end table + @end table @noindent diff --git a/gcc/opts.c b/gcc/opts.c index da503c32dd0..b776f3b31e1 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -2422,7 +2422,9 @@ common_handle_option (struct gcc_options *opts, break; case OPT_fdiagnostics_parseable_fixits: - dc->parseable_fixits_p = value; + dc->extra_output_kind = (value + ? EXTRA_DIAGNOSTIC_OUTPUT_fixits_v1 + : EXTRA_DIAGNOSTIC_OUTPUT_none); break; case OPT_fdiagnostics_column_unit_: -- 2.26.2