On Mon, Apr 07, 2025 at 01:58:08PM -0400, Marek Polacek wrote:
> On Wed, Feb 12, 2025 at 08:27:37PM -0500, Lewis Hyatt 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. */
>
> I don't love duplicating the comment like this, it's going to get out of sync.
>
> > +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)
>
> I think checking == UNKNOWN_LOCATION is clearer.
>
> > + 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 */
>
> Usually this would be the first line and also mention "c/".
>
> But those comments are details. I think the patch is OK for GCC 16.
>
> Thanks,
> Marek
>
Thanks very much for the review. I have pushed the below version with your
suggested changes as r16-179.
-Lewis
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.
diff --git a/gcc/c-family/c-lex.cc b/gcc/c-family/c-lex.cc
index e450c9a57f0..fef6ae6f457 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 == UNKNOWN_LOCATION)
+ 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..734da215470
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/pragma-diagnostic-loc-2.c
@@ -0,0 +1,15 @@
+/* PR c/118838 */
+/* { dg-do compile } */
+/* { dg-additional-options "-Wunknown-pragmas" } */
+
+/* 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 ();
}
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 3bf33ab4884..7c147ae5847 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -1169,6 +1169,8 @@ 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;
+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
