[PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106
Hello,
within a compile cluster, only the preprocessed output of GCC is transferred to
remote nodes for compilation.
When GCC produces advanced diagnostics (with -fdiagnostics-show-caret), e.g.
prints out the affected source
line and fixit hints, it attempts to read the source file again, even when
compiling a preprocessed file (-fpreprocessed).
This leads to wrong diagnostics when building with a compile cluster, or, more
generally, when changing or deleting the original source file.
This patch attempts to alter the behavior by implementing a
location_get_source_line_preprocessed
function that can be used in diagnostic-show-locus.c in case a preprocessed
file is compiled.
There was some previous discussion on this behavior on PR preprocessor/79106.
This is my first patch to GCC, so in case something is wrong with the format,
please let me know.
Best regards
Lucas
2019-12-16 Lucas Bader
PR preprocessor/79106
* c-opts.c (c_common_handle_option): pass -fpreprocessed
option value to global diagnostic configuration
* diagnostic-show-locus.c (layout::layout): read line from source or
preprocessed
file based on -fpreprocessed value
(source_line::source_line): read line from source or preprocessed
file based on -fpreprocessed value
(layout::print_line): read line from source or preprocessed
file based on -fpreprocessed value
* diagnostic.h (diagnostic_context): new members for reading
source lines from preprocessed files
* diagnostic.c (diagnostic_initialize): initialize new members
* input.c (location_get_source_line_preprocessed): new function
to read source lines from preprocessed files
(test_reading_source_line_preprocessed): new test case
(input_c_tests): execute new test case
* opts-global.c (read_cmdline_options): pass input filename to global
diagnostic context
---
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index c913291c07c..8634de6eb3f 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -485,6 +485,7 @@ c_common_handle_option (size_t scode, const char *arg,
HOST_WIDE_INT value,
case OPT_fpreprocessed:
cpp_opts->preprocessed = value;
+ global_dc->is_preprocessed = value;
break;
case OPT_fdebug_cpp:
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index cb920f6b9d0..3a4838605de 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -896,7 +896,12 @@ layout::layout (diagnostic_context * context,
Center the primary caret to fit in max_width; all columns
will be adjusted accordingly. */
size_t max_width = m_context->caret_max_width;
- char_span line = location_get_source_line (m_exploc.file, m_exploc.line);
+ char_span line (NULL, 0);
+ if (global_dc->main_input_file_path != NULL && global_dc->is_preprocessed)
+line = location_get_source_line_preprocessed (
+ m_exploc.file, global_dc->main_input_file_path, m_exploc.line);
+ else
+line = location_get_source_line (m_exploc.file, m_exploc.line);
if (line && (size_t)m_exploc.column <= line.length ())
{
size_t right_margin = CARET_LINE_MARGIN;
@@ -1913,7 +1918,12 @@ public:
source_line::source_line (const char *filename, int line)
{
- char_span span = location_get_source_line (filename, line);
+ char_span span (NULL, 0);
+ if (global_dc->main_input_file_path != NULL && global_dc->is_preprocessed)
+span = location_get_source_line_preprocessed (
+ filename, global_dc->main_input_file_path, line);
+ else
+span = location_get_source_line (filename, line);
chars = span.get_buffer ();
width = span.length ();
}
@@ -2237,7 +2247,13 @@ layout::show_ruler (int max_column) const
void
layout::print_line (linenum_type row)
{
- char_span line = location_get_source_line (m_exploc.file, row);
+ char_span line (NULL, 0);
+ if (global_dc->main_input_file_path != NULL && global_dc->is_preprocessed)
+line = location_get_source_line_preprocessed (
+ m_exploc.file, global_dc->main_input_file_path, row);
+ else
+line = location_get_source_line (m_exploc.file, row);
+
if (!line)
return;
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index a29bcf155e2..6ef99d0e8f1 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -218,6 +218,8 @@ diagnostic_initialize (diagnostic_context *context, int
n_opts)
context->begin_group_cb = NULL;
context->end_group_cb = NULL;
context->final_cb = default_diagnostic_final_cb;
+ context->main_input_file_path = NULL;
+ context->is_preprocessed = false;
}
/* Maybe initialize the color support. We require clients to do this
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 91e4c509605..f233057fbbe 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -158,6 +158,12 @@ struct diagnostic_context
/* Maximum number of error
[PING] [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106
Happy New Year and a ping for https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01113.html -Original Message- From: Bader, Lucas Sent: Montag, 16. Dezember 2019 12:19 To: gcc-patches@gcc.gnu.org Subject: [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106 Hello, within a compile cluster, only the preprocessed output of GCC is transferred to remote nodes for compilation. When GCC produces advanced diagnostics (with -fdiagnostics-show-caret), e.g. prints out the affected source line and fixit hints, it attempts to read the source file again, even when compiling a preprocessed file (-fpreprocessed). This leads to wrong diagnostics when building with a compile cluster, or, more generally, when changing or deleting the original source file. This patch attempts to alter the behavior by implementing a location_get_source_line_preprocessed function that can be used in diagnostic-show-locus.c in case a preprocessed file is compiled. There was some previous discussion on this behavior on PR preprocessor/79106. This is my first patch to GCC, so in case something is wrong with the format, please let me know. Best regards Lucas
[PING] [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106
Hello, as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79106 is still open from what I can tell, I wanted to gently bump this patch I provided a while back. It was earmarked for gcc-11 in https://gcc.gnu.org/pipermail/gcc-patches/2020-January/539201.html but did not make it into the release. Original submission: https://gcc.gnu.org/legacy-ml/gcc-patches/2019-12/msg01113.html Please let me know if something is missing. Best Lucas -Original Message- From: Bader, Lucas Sent: Montag, 16. Dezember 2019 12:19 To: gcc-patches@gcc.gnu.org Subject: [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106 Hello, within a compile cluster, only the preprocessed output of GCC is transferred to remote nodes for compilation. When GCC produces advanced diagnostics (with -fdiagnostics-show-caret), e.g. prints out the affected source line and fixit hints, it attempts to read the source file again, even when compiling a preprocessed file (-fpreprocessed). This leads to wrong diagnostics when building with a compile cluster, or, more generally, when changing or deleting the original source file. This patch attempts to alter the behavior by implementing a location_get_source_line_preprocessed function that can be used in diagnostic-show-locus.c in case a preprocessed file is compiled. There was some previous discussion on this behavior on PR preprocessor/79106. This is my first patch to GCC, so in case something is wrong with the format, please let me know. Best regards Lucas
Ping^3 [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106
Hello and Happy New Year, as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79106 is still open, I wanted to again bump this patch I provided a while back. It was earmarked for gcc-11 in https://gcc.gnu.org/pipermail/gcc-patches/2020-January/539201.html but did not make it into the release. Original submission: https://gcc.gnu.org/legacy-ml/gcc-patches/2019-12/msg01113.html Please let me know if something is missing. Best Lucas -Original Message- From: Bader, Lucas Sent: Montag, 16. Dezember 2019 12:19 To: gcc-patches@gcc.gnu.org Subject: [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106 Hello, within a compile cluster, only the preprocessed output of GCC is transferred to remote nodes for compilation. When GCC produces advanced diagnostics (with -fdiagnostics-show-caret), e.g. prints out the affected source line and fixit hints, it attempts to read the source file again, even when compiling a preprocessed file (-fpreprocessed). This leads to wrong diagnostics when building with a compile cluster, or, more generally, when changing or deleting the original source file. This patch attempts to alter the behavior by implementing a location_get_source_line_preprocessed function that can be used in diagnostic-show-locus.c in case a preprocessed file is compiled. There was some previous discussion on this behavior on PR preprocessor/79106. This is my first patch to GCC, so in case something is wrong with the format, please let me know. Best regards Lucas
[PATCH v3] get source line for diagnostic from preprocessed file [PR preprocessor/79106]
Hi, I reworked my patch based on your feedback and added data structures for caching linemarker lookups. It implements a two-stage approach consisting of plain memoization of calls to get_source_line_preprocessed and a hash table for storing linemarkers per source file. The details should hopefully be clear enough from the code. In an internal benchmark (~250 .ii files with ~10k linemarkers each, producing many warnings), the caching version achieves a median speedup of 60% over the initial version of the patch. Additionally, I have looked into the failing test cases (g++.dg/lookup/missing-std-include-11.C) and adapted the test to the new behavior. In my opinion, as “#include “ lines and comments are lost during preprocessing, it is correct to not emit diagnostics for those lines when operating on preprocessed output. I’m not sure if the behavior should be different for “-save-temps” however. Possibly, we could check for the existence of the original file before deciding on the path to take for reading the line. I have additionally tested the patch under valgrind and found no issues. Looking forward to your feedback. Best Lucas -- Within a compile cluster, only the preprocessed output of GCC is transferred to remote nodes for compilation. When GCC produces advanced diagnostics (with -fdiagnostics-show-caret), e.g. prints out the affected source line and fixit hints, it attempts to read the source file again, even when compiling a preprocessed file (-fpreprocessed). This leads to wrong diagnostics when building with a compile cluster, or, more generally, when changing or deleting the original source file. This change alters GCC to read from the preprocessed file instead by calculating the corresponding source line. This behavior is consistent with clang. The patch implements this efficiently by using a cache for linemarkers that are already seen and a memoization of lines that have already been requested. gcc/c-family/ChangeLog: PR preprocessor/79106 * c-opts.cc (c_common_handle_option): Pass -fpreprocessed option value to global diagnostic configuration. gcc/ChangeLog: PR preprocessor/79106 * diagnostic-show-locus.cc (get_source_line_maybe_preprocessed): Read line from source or preprocessed file based on -fpreprocessed value. (layout::calculate_x_offset_display): Use new function. (source_line::source_line): Use new function. (layout_printer::print_line): Use new function. * diagnostic.cc (diagnostic_context::initialize): Initialize new members. * diagnostic.h: Add new members for reading source lines from preprocessed files. * input.cc (file_cache_slot::evict): Also empty linemarker cache. (file_cache_slot::create): Initialize new linemarker cache. (file_cache_slot::file_cache_slot): Initialize new linemarker cache. (file_cache_slot::~file_cache_slot): Delete linemarker cache. (file_cache_slot::linemarker_cache::add_linemarker_unique): New function for adding linemarkers to the linemarker cache. (file_cache_slot::linemarker_cache::add_result_line): Memoize single result of get_source_line_preprocessed. (file_cache_slot::linemarker_cache::get_result_line): Retrieve memoized result of get_source_line_preprocessed. (is_linemarker_for_file): New function for testing a line for a linemarker. (file_cache::get_source_line_preprocessed): New function for reading lines from preprocessed sources. (test_parsing_linemarker): New test case. (test_linemarker_cache): New test case. (test_reading_source_line_preprocessed): New test case. (input_cc_tests): Add new test cases. * input.h (class file_cache): Add new member function. * opts-global.cc (read_cmdline_options): Pass input filename to global diagnostic context. gcc/testsuite/ChangeLog: PR preprocessor/79106 * g++.dg/lookup/missing-std-include-11.C: Adapt to new behavior. Signed-off-by: Lucas Bader --- gcc/c-family/c-opts.cc| 1 + gcc/diagnostic-show-locus.cc | 25 +- gcc/diagnostic.cc | 2 + gcc/diagnostic.h | 6 + gcc/input.cc | 681 ++ gcc/input.h | 2 + gcc/opts-global.cc| 4 + .../g++.dg/lookup/missing-std-include-11.C| 2 +- 8 files changed, 718 insertions(+), 5 deletions(-) diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc index 87b231861a6..105d8fa6eff 100644 --- a/gcc/c-family/c-opts.cc +++ b/gcc/c-family/c-opts.cc @@ -534,6 +534,7 @@ c_common_handle_option (size_t scode, const char *arg, HOST
[PATCH v2] get source line for diagnostic from preprocessed file [PR preprocessor/79106]
Hi,
as discussed, I rebased and tested my patch against current master.
Additionally, it now has the Signed-off-by tag.
Looking forward to your comments.
Best
Lucas
Within a compile cluster, only the preprocessed output of GCC is transferred
to remote nodes for compilation. When GCC produces advanced diagnostics
(with -fdiagnostics-show-caret), e.g. prints out the affected source
line and fixit hints, it attempts to read the source file again, even when
compiling a preprocessed file (-fpreprocessed). This leads to wrong
diagnostics when building with a compile cluster, or, more generally,
when changing or deleting the original source file.
This patch alters GCC to read from the preprocessed file instead by
calculating the corresponding source line. This behavior is consistent
with clang.
gcc/c-family/ChangeLog:
* c-opts.cc (c_common_handle_option): pass -fpreprocessed
option value to global diagnostic configuration
gcc/ChangeLog:
* diagnostic-show-locus.cc (layout::calculate_x_offset_display): read
line from source or preprocessed
file based on -fpreprocessed value
(source_line::source_line): read line from source or preprocessed
file based on -fpreprocessed value
(layout_printer::print_line): read line from source or preprocessed
file based on -fpreprocessed value
* diagnostic.cc (diagnostic_context::initialize): initialize new members
* diagnostic.h: new members for reading
source lines from preprocessed files
* input.cc (file_cache::get_source_line_preprocessed): new function
to read source lines from preprocessed files
(test_reading_source_line_preprocessed): new test case
(input_cc_tests): execute new test case
* input.h (class file_cache): add new member function
* opts-global.cc (read_cmdline_options): pass input filename to global
diagnostic context
Signed-off-by: Lucas Bader
---
gcc/c-family/c-opts.cc | 1 +
gcc/diagnostic-show-locus.cc | 25 --
gcc/diagnostic.cc| 2 +
gcc/diagnostic.h | 6 ++
gcc/input.cc | 169 +++
gcc/input.h | 2 +
gcc/opts-global.cc | 4 +
7 files changed, 204 insertions(+), 5 deletions(-)
diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index 87b231861a6..105d8fa6eff 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -534,6 +534,7 @@ c_common_handle_option (size_t scode, const char *arg,
HOST_WIDE_INT value,
case OPT_fpreprocessed:
cpp_opts->preprocessed = value;
+ global_dc->m_is_preprocessed = value;
break;
case OPT_fdebug_cpp:
diff --git a/gcc/diagnostic-show-locus.cc b/gcc/diagnostic-show-locus.cc
index 898efe74acf..7ac183a126f 100644
--- a/gcc/diagnostic-show-locus.cc
+++ b/gcc/diagnostic-show-locus.cc
@@ -1786,8 +1786,13 @@ layout::calculate_x_offset_display ()
return;
}
- const char_span line = m_file_cache.get_source_line (m_exploc.file,
- m_exploc.line);
+ char_span line (NULL, 0);
+ if (global_dc->m_main_input_file_path != NULL &&
global_dc->m_is_preprocessed)
+line = m_file_cache.get_source_line_preprocessed (
+ m_exploc.file, global_dc->m_main_input_file_path, m_exploc.line);
+ else
+line = m_file_cache.get_source_line (m_exploc.file, m_exploc.line);
+
if (!line)
{
/* Nothing to do, we couldn't find the source line. */
@@ -2780,7 +2785,12 @@ public:
source_line::source_line (file_cache &fc, const char *filename, int line)
{
- char_span span = fc.get_source_line (filename, line);
+ char_span span (NULL, 0);
+ if (global_dc->m_main_input_file_path != NULL &&
global_dc->m_is_preprocessed)
+span = fc.get_source_line_preprocessed (
+ filename, global_dc->m_main_input_file_path, line);
+ else
+span = fc.get_source_line (filename, line);
chars = span.get_buffer ();
width = span.length ();
}
@@ -3132,8 +3142,13 @@ layout_printer::show_ruler (int max_column)
void
layout_printer::print_line (linenum_type row)
{
- char_span line
-= m_layout.m_file_cache.get_source_line (m_layout.m_exploc.file, row);
+ char_span line (NULL, 0);
+ if (global_dc->m_main_input_file_path != NULL &&
global_dc->m_is_preprocessed)
+line = m_layout.m_file_cache.get_source_line_preprocessed (
+ m_layout.m_exploc.file, global_dc->m_main_input_file_path, row);
+ else
+line = m_layout.m_file_cache.get_source_line (m_layout.m_exploc.file, row);
+
if (!line)
return;
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index c2f6714c24a..c8a93d407ad 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -290,6 +290,8 @@ diagnostic_context::initialize (int n_opts)
m_diagrams.m_theme = nullptr;
m_original_argv = nullptr;
m_diagnostic_buffer = nullptr;
+ m_main_input_file_path = nullptr;
RE: [PATCH v2] get source line for diagnostic from preprocessed file [PR preprocessor/79106]
Thanks a lot for your detailed feedback! I will rework my patch, especially to make the get_source_line_preprocessed function more readable and more efficient. Some comments in the mean time: > This may sound silly, but please use "num" rather than "no" as an > abbreviation for "number": to my under-caffeinated brain "no" reads > as > the opposite of "yes". I fully agree, seems like 5 years ago my naming taste was a bit off. > What about buffer[1]? Does this have to be a ' '? ... > Don't we also need a trailing '"' and a newline? Ack, doesn't hurt to check buffer[1]. The newline might not come directly after the trailing '"' however, as line markers can contain additional flags after the source file name. If we have: # (\d+) "(.+)" it should be safe to assume it's a line marker. > Has this code been tested on large examples? It looks to me like this function does a linear scan starting at line 1 every time in the .i/.ii file upwards looking for markers of interest; Good points, I am looking into tracking line markers in the file_cache_slot. I am currently testing a version with a simple vector for memoization. FWIW we are using this patch in our internal gcc fork for building a pretty substantial C++ codebase and have not seen build time regressions directly linked to it. However, testing it against a fairly large .ii file that produces many warnings from that codebase (~400k LoC) shows significant speedup with the tracking version: ~/gcc-dev/bin/gcc TEST.cpp.ii 62.58s user 1.01s system 99% cpu 1:03.66 total # path as is ~/gcc-dev/bin/gcc TEST.cpp.ii 32.80s user 0.95s system 99% cpu 33.749 total # with line marker memoization > This function is rather complicated I agree. To be honest, looking at it after a couple of years was also not very pleasant. > Something else that occurred to me: assuming the file in question has gone through libcpp, we've already parsed the line markers, and line_table will have a series of line map instances representing the ranges in question. According to some comments I found, it seems that for e.g the C front-end, it can happen that we start emitting diagnostics before the line map has seen the end of the file, so it could probably only act as a hint? Best Lucas -Original Message- From: David Malcolm Sent: Wednesday, 29 January 2025 16:17 To: Bader, Lucas ; gcc-patches@gcc.gnu.org Subject: Re: [PATCH v2] get source line for diagnostic from preprocessed file [PR preprocessor/79106] On Wed, 2025-01-29 at 09:16 -0500, David Malcolm wrote: > On Wed, 2025-01-29 at 13:05 +, Bader, Lucas wrote: > > Hi, > > > > as discussed, I rebased and tested my patch against current master. > > Additionally, it now has the Signed-off-by tag. > > Looking forward to your comments. > > > > Best > > Lucas > > Thanks for the updated patch. > > Various notes inline throughout below... > > > > > > > > > Within a compile cluster, only the preprocessed output of GCC is > > transferred > > to remote nodes for compilation. When GCC produces advanced > > diagnostics > > (with -fdiagnostics-show-caret), e.g. prints out the affected > > source > > line and fixit hints, it attempts to read the source file again, > > even > > when > > compiling a preprocessed file (-fpreprocessed). This leads to wrong > > diagnostics when building with a compile cluster, or, more > > generally, > > when changing or deleting the original source file. > > > > This patch alters GCC to read from the preprocessed file instead by > > calculating the corresponding source line. This behavior is > > consistent > > with clang. > > > > gcc/c-family/ChangeLog: > > > > * c-opts.cc (c_common_handle_option): pass -fpreprocessed > > option value to global diagnostic configuration > > > > gcc/ChangeLog: > > > > * diagnostic-show-locus.cc > > (layout::calculate_x_offset_display): read line from source or > > preprocessed > > file based on -fpreprocessed value > > (source_line::source_line): read line from source or > > preprocessed > > file based on -fpreprocessed value > > (layout_printer::print_line): read line from source or > > preprocessed > > file based on -fpreprocessed value > > > > * diagnostic.cc (diagnostic_context::initialize): > > initialize > > new members > > * diagnostic.h: new members for reading > > source lines from preprocessed files > > > > * input.cc (file_cache::get_source_line_preprocessed): new > > function > >
Re: Ping^3 [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106
Thanks for having a look Jeff. Would be great to hear your thoughts on this patch David. Best Lucas > On 18. Jan 2025, at 17:41, Jeff Law wrote: > > > >> On 1/3/25 7:27 AM, Bader, Lucas wrote: >> Hello and Happy New Year, >> as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79106 is still open, I >> wanted to again bump this patch I provided a while back. >> It was earmarked for gcc-11 in >> https://gcc.gnu.org/pipermail/gcc-patches/2020-January/539201.html but did >> not make it into the release. >> Original submission: >> https://gcc.gnu.org/legacy-ml/gcc-patches/2019-12/msg01113.html >> Please let me know if something is missing. > Martin S. is no longer active in GCC development and stuff like input > locations tends to fall more towards David M's space. > > David M probably needs to chime in here (on cc). > > Jeff
RE: [PATCH] get source line for diagnostic from preprocessed file / PR preprocessor/79106
Hi Dave,
thanks for having a look. I'll rebase the change and resend it with the DCO
sign-off.
Best
Lucas
-Original Message-
From: David Malcolm
Sent: Tuesday, 28 January 2025 23:36
To: Bader, Lucas ; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] get source line for diagnostic from preprocessed file / PR
preprocessor/79106
On Mon, 2019-12-16 at 11:18 +0000, Bader, Lucas wrote:
> Hello,
Hi Lucas.
Thanks for the patch, and sorry for not seeing this patch before and
thus the long delay in reviewing it.
I started reviewing this and have various comments (mostly about
introducing subroutines to simplify the logic), but I notice the patch
doesn't have a Signed-off-by tag, which is the easy way to deal with
copyright/licensing issues on contributions.
See https://gcc.gnu.org/contribute.html#legal
Are you able to resend the patch with that tag? (or simply state that
you can certify the Developer Certificate of Origin for it; see the
above link).
The patch will have slightly bitrotted since 2019 since input.c is now
input.cc, and has had a few other patches (and FWIW Andi Kleen also has
a bunch of pending patches for it). I can have a go at rebasing it if
that would help (and perhaps incorporate my own feedback directly).
Sorry again for the delay.
Dave
>
> within a compile cluster, only the preprocessed output of GCC is
> transferred to remote nodes for compilation.
> When GCC produces advanced diagnostics (with -fdiagnostics-show-
> caret), e.g. prints out the affected source
> line and fixit hints, it attempts to read the source file again, even
> when compiling a preprocessed file (-fpreprocessed).
> This leads to wrong diagnostics when building with a compile cluster,
> or, more generally, when changing or deleting the original source
> file.
>
> This patch attempts to alter the behavior by implementing a
> location_get_source_line_preprocessed
> function that can be used in diagnostic-show-locus.c in case a
> preprocessed file is compiled.
> There was some previous discussion on this behavior on PR
> preprocessor/79106.
>
> This is my first patch to GCC, so in case something is wrong with the
> format, please let me know.
>
> Best regards
> Lucas
>
> 2019-12-16 Lucas Bader
>
> PR preprocessor/79106
> * c-opts.c (c_common_handle_option): pass -fpreprocessed
> option value to global diagnostic configuration
>
> * diagnostic-show-locus.c (layout::layout): read line from
> source or preprocessed
> file based on -fpreprocessed value
> (source_line::source_line): read line from source or
> preprocessed
> file based on -fpreprocessed value
> (layout::print_line): read line from source or preprocessed
> file based on -fpreprocessed value
>
> * diagnostic.h (diagnostic_context): new members for reading
> source lines from preprocessed files
> * diagnostic.c (diagnostic_initialize): initialize new
> members
>
> * input.c (location_get_source_line_preprocessed): new
> function
> to read source lines from preprocessed files
> (test_reading_source_line_preprocessed): new test case
> (input_c_tests): execute new test case
>
> * opts-global.c (read_cmdline_options): pass input filename
> to global
> diagnostic context
>
> ---
>
> diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
> index c913291c07c..8634de6eb3f 100644
> --- a/gcc/c-family/c-opts.c
> +++ b/gcc/c-family/c-opts.c
> @@ -485,6 +485,7 @@ c_common_handle_option (size_t scode, const char
> *arg, HOST_WIDE_INT value,
>
> case OPT_fpreprocessed:
>cpp_opts->preprocessed = value;
> + global_dc->is_preprocessed = value;
>break;
>
> case OPT_fdebug_cpp:
> diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-
> locus.c
> index cb920f6b9d0..3a4838605de 100644
> --- a/gcc/diagnostic-show-locus.c
> +++ b/gcc/diagnostic-show-locus.c
> @@ -896,7 +896,12 @@ layout::layout (diagnostic_context * context,
> Center the primary caret to fit in max_width; all columns
> will be adjusted accordingly. */
>size_t max_width = m_context->caret_max_width;
> - char_span line = location_get_source_line (m_exploc.file,
> m_exploc.line);
> + char_span line (NULL, 0);
> + if (global_dc->main_input_file_path != NULL && global_dc-
> >is_preprocessed)
> +line = location_get_source_line_preprocessed (
> + m_exploc.file, global_dc->main_input_file_path,
> m_exploc.line);
> + else
> +line = location_get_source_line (m_exploc.file, m_exploc.line);
>if (line && (size_t)m_exploc.column <= line.length ())
> {
>size_t ri
[PING] [PATCH v3] get source line for diagnostic from preprocessed file [PR preprocessor/79106]
Gentle ping for https://gcc.gnu.org/pipermail/gcc-patches/2025-February/675450.html
[PATCH v4] get source line for diagnostic from preprocessed file [PR preprocessor/79106]
We found one more issue with the patch in our internal compile cluster. When
using "-fdirectives-only", linemarkers do not always start at the beginning of
a line.
This version of the patch (last submission
https://gcc.gnu.org/pipermail/gcc-patches/2025-February/675450.html) fixes this
by skipping
blanks before parsing the linemarker. Also rebased to latest trunk.
Best
Lucas
--
Within a compile cluster, only the preprocessed output of GCC is transferred
to remote nodes for compilation. When GCC produces advanced diagnostics
(with -fdiagnostics-show-caret), e.g. prints out the affected source
line and fixit hints, it attempts to read the source file again, even when
compiling a preprocessed file (-fpreprocessed). This leads to wrong
diagnostics when building with a compile cluster, or, more generally,
when changing or deleting the original source file.
This change alters GCC to read from the preprocessed file instead by
calculating the corresponding source line. This behavior is consistent
with clang.
The patch implements this efficiently by using a cache for
linemarkers that are already seen and a memoization of lines
that have already been requested.
gcc/c-family/ChangeLog:
PR preprocessor/79106
* c-opts.cc (c_common_handle_option): Pass -fpreprocessed
option value to global diagnostic configuration.
gcc/ChangeLog:
PR preprocessor/79106
* diagnostic-show-locus.cc (get_source_line_maybe_preprocessed): Read
line from source or preprocessed file based on -fpreprocessed value.
(layout::calculate_x_offset_display): Use new function.
(source_line::source_line): Use new function.
(layout_printer::print_line): Use new function.
* diagnostic.cc (diagnostic_context::initialize): Initialize new
members.
* diagnostic.h: Add new members for reading source lines from
preprocessed files.
* input.cc (file_cache_slot::evict): Also empty linemarker
cache.
(file_cache_slot::create): Initialize new linemarker cache.
(file_cache_slot::file_cache_slot): Initialize new linemarker
cache.
(file_cache_slot::~file_cache_slot): Delete linemarker cache.
(file_cache_slot::linemarker_cache::add_linemarker_unique): New
function for adding linemarkers to the linemarker cache.
(file_cache_slot::linemarker_cache::add_result_line): Memoize
single result of get_source_line_preprocessed.
(file_cache_slot::linemarker_cache::get_result_line): Retrieve
memoized result of get_source_line_preprocessed.
(is_linemarker_for_file): New function for testing a line for
a linemarker.
(file_cache::get_source_line_preprocessed): New function for
reading lines from preprocessed sources.
(test_parsing_linemarker): New test case.
(test_linemarker_cache): New test case.
(test_reading_source_line_preprocessed): New test case.
(input_cc_tests): Add new test cases.
* input.h (class file_cache): Add new member function.
* opts-global.cc (read_cmdline_options): Pass input filename to global
diagnostic context.
gcc/testsuite/ChangeLog:
PR preprocessor/79106
* g++.dg/lookup/missing-std-include-11.C: Adapt to new behavior.
Signed-off-by: Lucas Bader
---
gcc/c-family/c-opts.cc| 1 +
gcc/diagnostic-show-locus.cc | 25 +-
gcc/diagnostic.cc | 2 +
gcc/diagnostic.h | 6 +
gcc/input.cc | 696 ++
gcc/input.h | 2 +
gcc/opts-global.cc| 4 +
.../g++.dg/lookup/missing-std-include-11.C| 2 +-
8 files changed, 733 insertions(+), 5 deletions(-)
diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index d43b3aef102..40c38ae9318 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -534,6 +534,7 @@ c_common_handle_option (size_t scode, const char *arg,
HOST_WIDE_INT value,
case OPT_fpreprocessed:
cpp_opts->preprocessed = value;
+ global_dc->m_is_preprocessed = value;
break;
case OPT_fdebug_cpp:
diff --git a/gcc/diagnostic-show-locus.cc b/gcc/diagnostic-show-locus.cc
index 898efe74acf..f08d325bfcf 100644
--- a/gcc/diagnostic-show-locus.cc
+++ b/gcc/diagnostic-show-locus.cc
@@ -1768,6 +1768,20 @@ layout::calculate_linenum_width ()
m_linenum_width = MAX (m_linenum_width, m_options.min_margin_width - 1);
}
+/* Get the source line depending on the global context, i.e. whether we are
+ compiling a preprocessed file or not. */
+static char_span
+get_source_line_maybe_preprocessed (file_cache &fc, const char *filename,
+ int line_num)
+{
+ if (global_d
