| Message ID | m3396jwaqe.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi,
On 05/29/2012 12:19 PM, Dodji Seketeli wrote:
> The _Pragma is effectively ignored,
hi Dodji, and sorry for taking the occasion to mention other issues with
pragmas which definitely can be handled separately, but I can't resist
;) In fact we have a number of open PRs:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47347
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48914
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431
all having to do with "#pragma GCC diagnostic ignored" and since you
looked into this code I'm wondering if you can see a pattern, something
which may explain all of them at once? (too good to be true, eh?)
Thanks anyway,
Paolo.
Hello I am friendly pinging this patch that felt below my radar. Dodji Seketeli <dodji@redhat.com> a écrit: > Hello, > > Consider this short test snippet: > > -------------------------8------------------- > #define STRINGIFY(x) #x > #define TEST(x) \ > _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ > typedef int myint; > > void bar () > { > TEST(myint) > } > -------------------------8------------------- > > The _Pragma is effectively ignored, and compiling with > -Wunused-local-typedefs warns on the local typedef, even though the > pragma should have prevented the warning to be emitted. > > This is because when the preprocessor sees the _Pragma operator and > then goes to handle the first token ('GCC' here) that makes up its > operands, it retains the spelling location of that token, not its > virtual location. > > Later when diagnostic_report_diagnostic is called to emit the warning > (or ignore it because of the pragma), it compares the location of the > first operand of the pragma with the location of the unused location, > (by calling linemap_location_before_p) and that comparison fails > because in this case, both locations should be virtual. > > This patch fixes the issue by teaching the pragma handling to use > virtual locations. > > Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. > > libcpp/ > > PR preprocessor/53469 > * directives.c (do_pragma): Use the virtual location for the > pragma token, instead of its spelling location. > > gcc/testsuite/ > > PR preprocessor/53469 > * gcc.dg/cpp/_Pragma7.c: New test case. > --- > gcc/testsuite/ChangeLog | 5 +++++ > gcc/testsuite/gcc.dg/cpp/_Pragma7.c | 14 ++++++++++++++ > libcpp/ChangeLog | 6 ++++++ > libcpp/directives.c | 8 +++++--- > 4 files changed, 30 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/cpp/_Pragma7.c > > diff --git a/gcc/testsuite/gcc.dg/cpp/_Pragma7.c b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c > new file mode 100644 > index 0000000..a7a5b1b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c > @@ -0,0 +1,14 @@ > +/* > + Origin: PR preprocessor/53469 > + { dg-do compile } > + */ > + > +#define STRINGIFY(x) #x > +#define TEST(x) \ > + _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ > + typedef int myint; > + > +void bar () > +{ > + TEST(myint) > +} > diff --git a/libcpp/directives.c b/libcpp/directives.c > index e46280e..cd880f1 100644 > --- a/libcpp/directives.c > +++ b/libcpp/directives.c > @@ -1347,13 +1347,15 @@ static void > do_pragma (cpp_reader *pfile) > { > const struct pragma_entry *p = NULL; > - const cpp_token *token, *pragma_token = pfile->cur_token; > + const cpp_token *token, *pragma_token; > + source_location pragma_token_virt_loc = 0; > cpp_token ns_token; > unsigned int count = 1; > > pfile->state.prevent_expansion++; > > - token = cpp_get_token (pfile); > + pragma_token = token = cpp_get_token_with_location (pfile, > + &pragma_token_virt_loc); > ns_token = *token; > if (token->type == CPP_NAME) > { > @@ -1407,7 +1409,7 @@ do_pragma (cpp_reader *pfile) > { > if (p->is_deferred) > { > - pfile->directive_result.src_loc = pragma_token->src_loc; > + pfile->directive_result.src_loc = pragma_token_virt_loc; > pfile->directive_result.type = CPP_PRAGMA; > pfile->directive_result.flags = pragma_token->flags; > pfile->directive_result.val.pragma = p->u.ident;
PING^2. Dodji Seketeli <dodji@redhat.com> writes: > Hello, > > Consider this short test snippet: > > -------------------------8------------------- > #define STRINGIFY(x) #x > #define TEST(x) \ > _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ > typedef int myint; > > void bar () > { > TEST(myint) > } > -------------------------8------------------- > > The _Pragma is effectively ignored, and compiling with > -Wunused-local-typedefs warns on the local typedef, even though the > pragma should have prevented the warning to be emitted. > > This is because when the preprocessor sees the _Pragma operator and > then goes to handle the first token ('GCC' here) that makes up its > operands, it retains the spelling location of that token, not its > virtual location. > > Later when diagnostic_report_diagnostic is called to emit the warning > (or ignore it because of the pragma), it compares the location of the > first operand of the pragma with the location of the unused location, > (by calling linemap_location_before_p) and that comparison fails > because in this case, both locations should be virtual. > > This patch fixes the issue by teaching the pragma handling to use > virtual locations. > > Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. > > libcpp/ > > PR preprocessor/53469 > * directives.c (do_pragma): Use the virtual location for the > pragma token, instead of its spelling location. > > gcc/testsuite/ > > PR preprocessor/53469 > * gcc.dg/cpp/_Pragma7.c: New test case. > --- > gcc/testsuite/ChangeLog | 5 +++++ > gcc/testsuite/gcc.dg/cpp/_Pragma7.c | 14 ++++++++++++++ > libcpp/ChangeLog | 6 ++++++ > libcpp/directives.c | 8 +++++--- > 4 files changed, 30 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/cpp/_Pragma7.c > > diff --git a/gcc/testsuite/gcc.dg/cpp/_Pragma7.c b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c > new file mode 100644 > index 0000000..a7a5b1b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c > @@ -0,0 +1,14 @@ > +/* > + Origin: PR preprocessor/53469 > + { dg-do compile } > + */ > + > +#define STRINGIFY(x) #x > +#define TEST(x) \ > + _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ > + typedef int myint; > + > +void bar () > +{ > + TEST(myint) > +} > diff --git a/libcpp/directives.c b/libcpp/directives.c > index e46280e..cd880f1 100644 > --- a/libcpp/directives.c > +++ b/libcpp/directives.c > @@ -1347,13 +1347,15 @@ static void > do_pragma (cpp_reader *pfile) > { > const struct pragma_entry *p = NULL; > - const cpp_token *token, *pragma_token = pfile->cur_token; > + const cpp_token *token, *pragma_token; > + source_location pragma_token_virt_loc = 0; > cpp_token ns_token; > unsigned int count = 1; > > pfile->state.prevent_expansion++; > > - token = cpp_get_token (pfile); > + pragma_token = token = cpp_get_token_with_location (pfile, > + &pragma_token_virt_loc); > ns_token = *token; > if (token->type == CPP_NAME) > { > @@ -1407,7 +1409,7 @@ do_pragma (cpp_reader *pfile) > { > if (p->is_deferred) > { > - pfile->directive_result.src_loc = pragma_token->src_loc; > + pfile->directive_result.src_loc = pragma_token_virt_loc; > pfile->directive_result.type = CPP_PRAGMA; > pfile->directive_result.flags = pragma_token->flags; > pfile->directive_result.val.pragma = p->u.ident;
On Mon, Aug 27, 2012 at 09:52:04AM +0200, Dodji Seketeli wrote: > PING^2. This is ok for trunk. Thanks. > > Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. > > > > libcpp/ > > > > PR preprocessor/53469 > > * directives.c (do_pragma): Use the virtual location for the > > pragma token, instead of its spelling location. > > > > gcc/testsuite/ > > > > PR preprocessor/53469 > > * gcc.dg/cpp/_Pragma7.c: New test case. > > --- > > gcc/testsuite/ChangeLog | 5 +++++ > > gcc/testsuite/gcc.dg/cpp/_Pragma7.c | 14 ++++++++++++++ > > libcpp/ChangeLog | 6 ++++++ > > libcpp/directives.c | 8 +++++--- > > 4 files changed, 30 insertions(+), 3 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/cpp/_Pragma7.c > > > > diff --git a/gcc/testsuite/gcc.dg/cpp/_Pragma7.c b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c > > new file mode 100644 > > index 0000000..a7a5b1b > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c > > @@ -0,0 +1,14 @@ > > +/* > > + Origin: PR preprocessor/53469 > > + { dg-do compile } > > + */ > > + > > +#define STRINGIFY(x) #x > > +#define TEST(x) \ > > + _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ > > + typedef int myint; > > + > > +void bar () > > +{ > > + TEST(myint) > > +} > > diff --git a/libcpp/directives.c b/libcpp/directives.c > > index e46280e..cd880f1 100644 > > --- a/libcpp/directives.c > > +++ b/libcpp/directives.c > > @@ -1347,13 +1347,15 @@ static void > > do_pragma (cpp_reader *pfile) > > { > > const struct pragma_entry *p = NULL; > > - const cpp_token *token, *pragma_token = pfile->cur_token; > > + const cpp_token *token, *pragma_token; > > + source_location pragma_token_virt_loc = 0; > > cpp_token ns_token; > > unsigned int count = 1; > > > > pfile->state.prevent_expansion++; > > > > - token = cpp_get_token (pfile); > > + pragma_token = token = cpp_get_token_with_location (pfile, > > + &pragma_token_virt_loc); > > ns_token = *token; > > if (token->type == CPP_NAME) > > { > > @@ -1407,7 +1409,7 @@ do_pragma (cpp_reader *pfile) > > { > > if (p->is_deferred) > > { > > - pfile->directive_result.src_loc = pragma_token->src_loc; > > + pfile->directive_result.src_loc = pragma_token_virt_loc; > > pfile->directive_result.type = CPP_PRAGMA; > > pfile->directive_result.flags = pragma_token->flags; > > pfile->directive_result.val.pragma = p->u.ident; > > -- > Dodji Jakub
diff --git a/gcc/testsuite/gcc.dg/cpp/_Pragma7.c b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c new file mode 100644 index 0000000..a7a5b1b --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c @@ -0,0 +1,14 @@ +/* + Origin: PR preprocessor/53469 + { dg-do compile } + */ + +#define STRINGIFY(x) #x +#define TEST(x) \ + _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ + typedef int myint; + +void bar () +{ + TEST(myint) +} diff --git a/libcpp/directives.c b/libcpp/directives.c index e46280e..cd880f1 100644 --- a/libcpp/directives.c +++ b/libcpp/directives.c @@ -1347,13 +1347,15 @@ static void do_pragma (cpp_reader *pfile) { const struct pragma_entry *p = NULL; - const cpp_token *token, *pragma_token = pfile->cur_token; + const cpp_token *token, *pragma_token; + source_location pragma_token_virt_loc = 0; cpp_token ns_token; unsigned int count = 1; pfile->state.prevent_expansion++; - token = cpp_get_token (pfile); + pragma_token = token = cpp_get_token_with_location (pfile, + &pragma_token_virt_loc); ns_token = *token; if (token->type == CPP_NAME) { @@ -1407,7 +1409,7 @@ do_pragma (cpp_reader *pfile) { if (p->is_deferred) { - pfile->directive_result.src_loc = pragma_token->src_loc; + pfile->directive_result.src_loc = pragma_token_virt_loc; pfile->directive_result.type = CPP_PRAGMA; pfile->directive_result.flags = pragma_token->flags; pfile->directive_result.val.pragma = p->u.ident;
Hello, Consider this short test snippet: -------------------------8------------------- #define STRINGIFY(x) #x #define TEST(x) \ _Pragma(STRINGIFY(GCC diagnostic ignored "-Wunused-local-typedefs")) \ typedef int myint; void bar () { TEST(myint) } -------------------------8------------------- The _Pragma is effectively ignored, and compiling with -Wunused-local-typedefs warns on the local typedef, even though the pragma should have prevented the warning to be emitted. This is because when the preprocessor sees the _Pragma operator and then goes to handle the first token ('GCC' here) that makes up its operands, it retains the spelling location of that token, not its virtual location. Later when diagnostic_report_diagnostic is called to emit the warning (or ignore it because of the pragma), it compares the location of the first operand of the pragma with the location of the unused location, (by calling linemap_location_before_p) and that comparison fails because in this case, both locations should be virtual. This patch fixes the issue by teaching the pragma handling to use virtual locations. Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. libcpp/ PR preprocessor/53469 * directives.c (do_pragma): Use the virtual location for the pragma token, instead of its spelling location. gcc/testsuite/ PR preprocessor/53469 * gcc.dg/cpp/_Pragma7.c: New test case. --- gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/gcc.dg/cpp/_Pragma7.c | 14 ++++++++++++++ libcpp/ChangeLog | 6 ++++++ libcpp/directives.c | 8 +++++--- 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/_Pragma7.c