Hello- May I please ping this patch? Thanks! https://gcc.gnu.org/pipermail/gcc-patches/2025-February/675645.html
-Lewis On Wed, Feb 12, 2025 at 8:27 PM Lewis Hyatt <lhy...@gmail.com> wrote: > > Hello- > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118838 > > This patch addresses the issue mentioned in the PR (another instance of > _Pragma string location issues). bootstrap + regtest all languages on > aarch64 looks good. Is it OK please for now or for stage 1? Note, it is not > a regression, since this never worked in C or C++ frontends; but on the > other hand, r15-4505 for GCC 15 fixed some related issues, so it could be > nice if this one gets in along with it. Thanks! > > -Lewis > > -- >8 -- > > The warning for -Wunknown-pragmas is issued at the location provided by > libcpp to the def_pragma() callback. This location is > cpp_reader::directive_line, which is a location for the start of the line > only; it is also not a valid location in case the unknown pragma was lexed > from a _Pragma string. These factors make it impossible to suppress > -Wunknown-pragmas via _Pragma("GCC diagnostic...") directives on the same > source line, as in the PR and the test case. Address that by issuing the > warning at a better location returned by cpp_get_diagnostic_override_loc(). > libcpp already maintains this location to handle _Pragma-related diagnostics > internally; it was needed also to make a publicly accessible version of it. > > gcc/c-family/ChangeLog: > > PR c/118838 > * c-lex.cc (cb_def_pragma): Call cpp_get_diagnostic_override_loc() > to get a valid location at which to issue -Wunknown-pragmas, in case > it was triggered from a _Pragma. > > libcpp/ChangeLog: > > PR c/118838 > * errors.cc (cpp_get_diagnostic_override_loc): New function. > * include/cpplib.h (cpp_get_diagnostic_override_loc): Declare. > > gcc/testsuite/ChangeLog: > > PR c/118838 > * c-c++-common/cpp/pragma-diagnostic-loc-2.c: New test. > * g++.dg/gomp/macro-4.C: Adjust expected output. > * gcc.dg/gomp/macro-4.c: Likewise. > * gcc.dg/cpp/Wunknown-pragmas-1.c: Likewise. > --- > libcpp/errors.cc | 10 +++++++++ > libcpp/include/cpplib.h | 5 +++++ > gcc/c-family/c-lex.cc | 7 +++++- > .../cpp/pragma-diagnostic-loc-2.c | 15 +++++++++++++ > gcc/testsuite/g++.dg/gomp/macro-4.C | 8 +++---- > gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c | 22 +++++++++++-------- > gcc/testsuite/gcc.dg/gomp/macro-4.c | 8 +++---- > 7 files changed, 57 insertions(+), 18 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc-2.c > > diff --git a/libcpp/errors.cc b/libcpp/errors.cc > index 9621c4b66ea..d9efb6acd30 100644 > --- a/libcpp/errors.cc > +++ b/libcpp/errors.cc > @@ -52,6 +52,16 @@ cpp_diagnostic_get_current_location (cpp_reader *pfile) > } > } > > +/* Sometimes a diagnostic needs to be generated before libcpp has been able > + to generate a valid location for the current token; in that case, the > + non-zero location returned by this function is the preferred one to use. > */ > + > +location_t > +cpp_get_diagnostic_override_loc (const cpp_reader *pfile) > +{ > + return pfile->diagnostic_override_loc; > +} > + > /* Print a diagnostic at the given location. */ > > ATTRIBUTE_CPP_PPDIAG (5, 0) > diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h > index 90aa3160ebf..04d4621da3c 100644 > --- a/libcpp/include/cpplib.h > +++ b/libcpp/include/cpplib.h > @@ -1168,6 +1168,11 @@ extern const char *cpp_probe_header_unit (cpp_reader > *, const char *file, > extern const char *cpp_get_narrow_charset_name (cpp_reader *) ATTRIBUTE_PURE; > extern const char *cpp_get_wide_charset_name (cpp_reader *) ATTRIBUTE_PURE; > > +/* Sometimes a diagnostic needs to be generated before libcpp has been able > + to generate a valid location for the current token; in that case, the > + non-zero location returned by this function is the preferred one to use. > */ > +extern location_t cpp_get_diagnostic_override_loc (const cpp_reader *); > + > /* This function reads the file, but does not start preprocessing. It > returns the name of the original file; this is the same as the > input file, except for preprocessed input. This will generate at > diff --git a/gcc/c-family/c-lex.cc b/gcc/c-family/c-lex.cc > index e450c9a57f0..df84020de62 100644 > --- a/gcc/c-family/c-lex.cc > +++ b/gcc/c-family/c-lex.cc > @@ -248,7 +248,12 @@ cb_def_pragma (cpp_reader *pfile, location_t loc) > { > const unsigned char *space, *name; > const cpp_token *s; > - location_t fe_loc = loc; > + > + /* If we are processing a _Pragma, LOC is not a valid location, but > libcpp > + will provide a good location via this function instead. */ > + location_t fe_loc = cpp_get_diagnostic_override_loc (pfile); > + if (!fe_loc) > + fe_loc = loc; > > space = name = (const unsigned char *) ""; > > diff --git a/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc-2.c > b/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc-2.c > new file mode 100644 > index 00000000000..e7e8cf23759 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc-2.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-Wunknown-pragmas" } */ > + > +/* PR 118838 */ > +/* Make sure the warnings are suppressed as expected. */ > + > +/* The tokens need to be all on the same line here. */ > +_Pragma ("GCC diagnostic push") _Pragma ("GCC diagnostic ignored > \"-Wunknown-pragmas\"") _Pragma ("__unknown__") _Pragma ("GCC diagnostic pop") > + > +#define MACRO \ > + _Pragma ("GCC diagnostic push") \ > + _Pragma ("GCC diagnostic ignored \"-Wunknown-pragmas\"") \ > + _Pragma ("__unknown__") \ > + _Pragma ("GCC diagnostic pop") > +MACRO > diff --git a/gcc/testsuite/g++.dg/gomp/macro-4.C > b/gcc/testsuite/g++.dg/gomp/macro-4.C > index dcc8bcbc8e5..5aa6d97c660 100644 > --- a/gcc/testsuite/g++.dg/gomp/macro-4.C > +++ b/gcc/testsuite/g++.dg/gomp/macro-4.C > @@ -3,7 +3,7 @@ > // { dg-options "-fopenmp -Wunknown-pragmas" } > > #define p _Pragma ("omp parallel") > -#define omp_p _Pragma ("omp p") > +#define omp_p _Pragma ("omp p") // { dg-warning "ignoring '#pragma > omp _Pragma'" } > > void bar (void); > > @@ -12,18 +12,18 @@ foo (void) > { > #pragma omp p // { dg-warning "-:ignoring '#pragma omp _Pragma'" } > bar (); > - omp_p // { dg-warning "-:ignoring '#pragma omp > _Pragma'" } > + omp_p // { dg-note "in expansion of macro 'omp_p'" } > bar (); > } > > #define parallel serial > -#define omp_parallel _Pragma ("omp parallel") > +#define omp_parallel _Pragma ("omp parallel") // { dg-warning "ignoring > '#pragma omp serial'" } > > void > baz (void) > { > #pragma omp parallel // { dg-warning "-:ignoring '#pragma omp serial'" } > bar (); > - omp_parallel // { dg-warning "-:ignoring '#pragma omp serial'" } > + omp_parallel // { dg-note "in expansion of macro 'omp_parallel'" } > bar (); > } > diff --git a/gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c > b/gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c > index 06a244e097d..f3c790195e1 100644 > --- a/gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c > +++ b/gcc/testsuite/gcc.dg/cpp/Wunknown-pragmas-1.c > @@ -8,21 +8,25 @@ > #pragma unknown1 /* { dg-warning "-:unknown1" "unknown1" } */ > > #define COMMA , > -#define FOO(x) x > -#define BAR(x) _Pragma("unknown_before") x > -#define BAZ(x) x _Pragma("unknown_after") > +#define FOO(x) x /* { dg-note "in definition of macro 'FOO'" } */ > > -int _Pragma("unknown2") bar1; /* { dg-warning "-:unknown2" "unknown2" } */ > +#define BAR1(x) _Pragma("unknown_before1") x /* { dg-warning > "unknown_before1" } */ > +#define BAR2(x) _Pragma("unknown_before2") x /* { dg-warning > "unknown_before2" } */ > > -FOO(int _Pragma("unknown3") bar2); /* { dg-warning "-:unknown3" "unknown3" } > */ > +#define BAZ1(x) x _Pragma("unknown_after1") /* { dg-warning > "unknown_after1" } */ > +#define BAZ2(x) x _Pragma("unknown_after2") /* { dg-warning > "unknown_after2" } */ > > -int BAR(bar3); /* { dg-warning "-:unknown_before" "unknown_before 1" } */ > +int _Pragma("unknown2") bar1; /* { dg-warning "unknown2" "unknown2" } */ > > -BAR(int bar4); /* { dg-warning "-:unknown_before" "unknown_before 2" } */ > +FOO(int _Pragma("unknown3") bar2); /* { dg-warning "unknown3" "unknown3" } */ > > -int BAZ(bar5); /* { dg-warning "-:unknown_after" "unknown_after 1" } */ > +int BAR1(bar3); /* { dg-note "in expansion of macro 'BAR1'" } */ > > -int BAZ(bar6;) /* { dg-warning "-:unknown_after" "unknown_after 2" } */ > +BAR2(int bar4); /* { dg-note "in expansion of macro 'BAR2'" } */ > + > +int BAZ1(bar5); /* { dg-note "in expansion of macro 'BAZ1'" } */ > + > +int BAZ2(bar6;) /* { dg-note "in expansion of macro 'BAZ2'" } */ > > FOO(int bar7; _Pragma("unknown4")) /* { dg-warning "-:unknown4" "unknown4" } > */ > > diff --git a/gcc/testsuite/gcc.dg/gomp/macro-4.c > b/gcc/testsuite/gcc.dg/gomp/macro-4.c > index a4ed9a3980a..961c599f2fe 100644 > --- a/gcc/testsuite/gcc.dg/gomp/macro-4.c > +++ b/gcc/testsuite/gcc.dg/gomp/macro-4.c > @@ -3,7 +3,7 @@ > /* { dg-options "-fopenmp -Wunknown-pragmas" } */ > > #define p _Pragma ("omp parallel") > -#define omp_p _Pragma ("omp p") > +#define omp_p _Pragma ("omp p") /* { dg-warning "ignoring '#pragma > omp _Pragma'" } */ > > void bar (void); > > @@ -12,18 +12,18 @@ foo (void) > { > #pragma omp p /* { dg-warning "-:ignoring '#pragma omp _Pragma'" } > */ > bar (); > - omp_p /* { dg-warning "-:ignoring '#pragma omp > _Pragma'" } */ > + omp_p /* { dg-note "in expansion of macro 'omp_p'" > } */ > bar (); > } > > #define parallel serial > -#define omp_parallel _Pragma ("omp parallel") > +#define omp_parallel _Pragma ("omp parallel") /* { dg-warning "ignoring > '#pragma omp serial'" } */ > > void > baz (void) > { > #pragma omp parallel /* { dg-warning "-:ignoring '#pragma omp serial'" } */ > bar (); > - omp_parallel /* { dg-warning "-:ignoring '#pragma omp serial'" } */ > + omp_parallel /* { dg-note "in expansion of macro 'omp_parallel'" > } */ > bar (); > }