| Message ID | AM5PR0701MB2657D3E486A5F6CD3F5AA0EEE4410@AM5PR0701MB2657.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix _Pragma GCC diagnostic in macro expansions | expand |
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00197.html On 07/04/18 12:53, Bernd Edlinger wrote: > Hi, > > currently _Pragma("GCC diagnostic ...") does not properly > work in macro expansions. > > Consider the following code: > > #define B _Pragma("GCC diagnostic push") \ > _Pragma("GCC diagnostic ignored \"-Wattributes\"") > #define E _Pragma("GCC diagnostic pop") > > #define X() B int __attribute((unknown_attr)) x; E /* { dg-bogus "attribute directive ignored" } */ > > void test1(void) > { > X() /* { dg-bogus "in expansion of macro" } */ > } > > > Warnings happen in C++ despite the _Pragma, while C happens to suppress the warnings > more or less by accident. > > This is connected to the fact that GCC uses the location of the closing parenthesis of the > function-like macro expansion in the _Pragma, while the rest of the locations are relative > to the macro expansion point, which is the letter X in this case. > > This patch changes the location of builtin macros and _Pragma to use the macro expansion > point instead of the closing parenthesis. > > A few test cases had to be adjusted, most changes were necessary because the __LINE__ > location moved to the macro expansion point, which looks like a straight improvement. > > In pr61817-2.c the location of __LINE__ depends on -ftrack-macro-expansion, > when enabled the location of the macro argument is the spelling location, while all other > locations change to the macro expansion point. > > The C++ pagma plugin.c is also affected by the change, because the input_location is now > the spelling location of _Pragma in DO_PRAGMA and has to be converted to the expansion > point of the macro to get the expected result. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. >
On Wed, 2018-07-04 at 10:53 +0000, Bernd Edlinger wrote: Sorry for the delay in reviewing this. > Hi, > > currently _Pragma("GCC diagnostic ...") does not properly > work in macro expansions. > > Consider the following code: > > #define B _Pragma("GCC diagnostic push") \ > _Pragma("GCC diagnostic ignored \"-Wattributes\"") > #define E _Pragma("GCC diagnostic pop") > > #define X() B int __attribute((unknown_attr)) x; E /* { dg-bogus > "attribute directive ignored" } */ > > void test1(void) > { > X() /* { dg-bogus "in expansion of macro" } */ > } > > > Warnings happen in C++ despite the _Pragma, while C happens to > suppress the warnings > more or less by accident. > > This is connected to the fact that GCC uses the location of the > closing parenthesis of the > function-like macro expansion in the _Pragma, while the rest of the > locations are relative > to the macro expansion point, which is the letter X in this case. > > This patch changes the location of builtin macros and _Pragma to use > the macro expansion > point instead of the closing parenthesis. While reviewing the various PRs for the testcases this touches, I noticed: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61817#c3 which has a link to: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1911.htm which has: Suggested Technical Corrigendum Add to 6.10.8.1, paragraph 1, item __LINE__: The line number of a pp token is implementation defined to be the (physical) line number of either the first character or the last character of the pp token. The line number of a __LINE__ that spans multiple physical lines is implementation defined to be either the first line or the last line of that __LINE__. The line number of a __LINE__ in a macro body is the line number of the macro invocation. The line number of a macro invocation that spans multiple (physical or logical) lines is implementation defined to be either the line number of the first character of the macro name, the last character of the macro name or the closing ')' (if there is one). I don't know the status of that suggested corrigendum, but if I'm reading it right, changing the location from that of the closing ')' to that of the macro name would keep us in compliance with that final sentence (after extracting the location's line number). > A few test cases had to be adjusted, most changes were necessary > because the __LINE__ > location moved to the macro expansion point, which looks like a > straight improvement. > > In pr61817-2.c the location of __LINE__ depends on -ftrack-macro- > expansion, > when enabled the location of the macro argument is the spelling > location, while all other > locations change to the macro expansion point. > > The C++ pagma plugin.c is also affected by the change, because the > input_location is now > the spelling location of _Pragma in DO_PRAGMA and has to be converted > to the expansion > point of the macro to get the expected result. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? Some nits: > libcpp: > 2018-07-04 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * macro.c (enter_macro_context): Change the location info for builtin > macros from location of the closing parenthesis to location of the macro > expansion point. Please update to say "builtin macros and _Pragma" here, rather than just "builtin macros" here. > testsuite: > 2018-07-04 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * c-c++-common/cpp/diagnostic-pragma-2.c: New test. > * c-c++-common/pr69558.c: Remove xfail. > * gcc.dg/cpp/builtin-macro-1.c: Adjust test expectations. > * gcc.dg/pr61817-1.c: Likewise. > * gcc.dg/pr61817-2.c: Likewise. > * g++.dg/plugin/pragma_plugin.c: Warn at expansion_point_location. Please reference PR 69558 in both ChangeLog entries (given that this fixes an XFAIL). OK for trunk with the above changes; thanks. It looks like with this change we can remove Jakub's r233058 hack. I briefly tested with it, and it seems to work. But that can wait for a followup. Dave
Index: gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-2.c =================================================================== --- gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-2.c (revision 0) +++ gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-2.c (working copy) @@ -0,0 +1,14 @@ +/* { dg-do compile } */ + +#define B _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wattributes\"") +#define E _Pragma("GCC diagnostic pop") + +#define X() B int __attribute((unknown_attr)) x; E /* { dg-bogus "attribute directive ignored" } */ +#define Y B int __attribute((unknown_attr)) y; E /* { dg-bogus "attribute directive ignored" } */ + +void test1(void) +{ + X() /* { dg-bogus "in expansion of macro" } */ + Y /* { dg-bogus "in expansion of macro" } */ +} Index: gcc/testsuite/c-c++-common/pr69558.c =================================================================== --- gcc/testsuite/c-c++-common/pr69558.c (revision 262287) +++ gcc/testsuite/c-c++-common/pr69558.c (working copy) @@ -11,9 +11,9 @@ _Pragma ("GCC diagnostic pop") #define C(x) \ A \ - static inline void bar (void) { x (); } /* { dg-bogus "in definition of|deprecated" "" { xfail { c++ } } } */ \ + static inline void bar (void) { x (); } /* { dg-bogus "in definition of|deprecated" "" } */ \ B -__attribute__((deprecated)) void foo (void); /* { dg-bogus "declared here" "" { xfail { c++ } } } */ +__attribute__((deprecated)) void foo (void); /* { dg-bogus "declared here" "" } */ C (foo) /* { dg-bogus "is deprecated" } */ Index: gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c =================================================================== --- gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c (revision 262287) +++ gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c (working copy) @@ -1,8 +1,8 @@ /* Origin PR preprocessor/64803 This test ensures that the value the __LINE__ macro expands to is - constant and corresponds to the line of the closing parenthesis of - the top-most function-like macro expansion it's part of. + constant and corresponds to the line of the macro expansion point + the function-like macro expansion it's part of. { dg-do run } { do-options -no-integrated-cpp } */ @@ -19,8 +19,8 @@ M(a ); - assert(L20 == 20); /* 20 is the line number of the - closing parenthesis of the + assert(L19 == 19); /* 19 is the line number of the + macro expansion point of the invocation of the M macro. Please adjust in case the layout of this file changes. */ Index: gcc/testsuite/gcc.dg/pr61817-1.c =================================================================== --- gcc/testsuite/gcc.dg/pr61817-1.c (revision 262287) +++ gcc/testsuite/gcc.dg/pr61817-1.c (working copy) @@ -14,6 +14,6 @@ enum { ) }; -A(a == 15); -A(b == 15); -A(c == 15); +A(a == 10); +A(b == 10); +A(c == 10); Index: gcc/testsuite/gcc.dg/pr61817-2.c =================================================================== --- gcc/testsuite/gcc.dg/pr61817-2.c (revision 262287) +++ gcc/testsuite/gcc.dg/pr61817-2.c (working copy) @@ -14,6 +14,6 @@ enum { ) }; -A(a == 15); -A(b == 15); -A(c == 15); +A(a == 10); +A(b == 10); +A(c == 14); Index: gcc/testsuite/g++.dg/plugin/pragma_plugin.c =================================================================== --- gcc/testsuite/g++.dg/plugin/pragma_plugin.c (revision 262287) +++ gcc/testsuite/g++.dg/plugin/pragma_plugin.c (working copy) @@ -33,14 +33,15 @@ handle_pragma_sayhello (cpp_reader *dumm } if (TREE_STRING_LENGTH (message) > 1) { + location_t loc = expansion_point_location (input_location); if (cfun) - warning (OPT_Wpragmas, - "%<pragma GCCPLUGIN sayhello%> from function %qE: %s", - cfun->decl, TREE_STRING_POINTER (message)); + warning_at (loc, OPT_Wpragmas, + "%<pragma GCCPLUGIN sayhello%> from function %qE: %s", + cfun->decl, TREE_STRING_POINTER (message)); else - warning (OPT_Wpragmas, - "%<pragma GCCPLUGIN sayhello%> outside of function: %s", - TREE_STRING_POINTER (message)); + warning_at (loc, OPT_Wpragmas, + "%<pragma GCCPLUGIN sayhello%> outside of function: %s", + TREE_STRING_POINTER (message)); } } Index: libcpp/macro.c =================================================================== --- libcpp/macro.c (revision 262287) +++ libcpp/macro.c (working copy) @@ -1410,29 +1410,25 @@ enter_macro_context (cpp_reader *pfile, pfile->about_to_expand_macro_p = false; /* Handle built-in macros and the _Pragma operator. */ { - source_location loc, expand_loc; + source_location expand_loc; if (/* The top-level macro invocation that triggered the expansion - we are looking at is with a standard macro ...*/ + we are looking at is with a standard macro ... */ !(pfile->top_most_macro_node->flags & NODE_BUILTIN) - /* ... and it's a function-like macro invocation. */ - && pfile->top_most_macro_node->value.macro->fun_like) - { - /* Then the location of the end of the macro invocation is the - location of the closing parenthesis. */ - loc = pfile->cur_token[-1].src_loc; - expand_loc = loc; - } + /* ... and it's a function-like macro invocation, */ + && pfile->top_most_macro_node->value.macro->fun_like + /* ... and we are tracking the macro expansion. */ + && CPP_OPTION (pfile, track_macro_expansion)) + /* Then the location of the end of the macro invocation is the + location of the expansion point of this macro. */ + expand_loc = location; else - { - /* Otherwise, the location of the end of the macro invocation is - the location of the expansion point of that top-level macro - invocation. */ - loc = location; - expand_loc = pfile->invocation_location; - } + /* Otherwise, the location of the end of the macro invocation is + the location of the expansion point of that top-level macro + invocation. */ + expand_loc = pfile->invocation_location; - return builtin_macro (pfile, node, loc, expand_loc); + return builtin_macro (pfile, node, location, expand_loc); } }
Hi, currently _Pragma("GCC diagnostic ...") does not properly work in macro expansions. Consider the following code: #define B _Pragma("GCC diagnostic push") \ _Pragma("GCC diagnostic ignored \"-Wattributes\"") #define E _Pragma("GCC diagnostic pop") #define X() B int __attribute((unknown_attr)) x; E /* { dg-bogus "attribute directive ignored" } */ void test1(void) { X() /* { dg-bogus "in expansion of macro" } */ } Warnings happen in C++ despite the _Pragma, while C happens to suppress the warnings more or less by accident. This is connected to the fact that GCC uses the location of the closing parenthesis of the function-like macro expansion in the _Pragma, while the rest of the locations are relative to the macro expansion point, which is the letter X in this case. This patch changes the location of builtin macros and _Pragma to use the macro expansion point instead of the closing parenthesis. A few test cases had to be adjusted, most changes were necessary because the __LINE__ location moved to the macro expansion point, which looks like a straight improvement. In pr61817-2.c the location of __LINE__ depends on -ftrack-macro-expansion, when enabled the location of the macro argument is the spelling location, while all other locations change to the macro expansion point. The C++ pagma plugin.c is also affected by the change, because the input_location is now the spelling location of _Pragma in DO_PRAGMA and has to be converted to the expansion point of the macro to get the expected result. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. libcpp: 2018-07-04 Bernd Edlinger <bernd.edlinger@hotmail.de> * macro.c (enter_macro_context): Change the location info for builtin macros from location of the closing parenthesis to location of the macro expansion point. testsuite: 2018-07-04 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-c++-common/cpp/diagnostic-pragma-2.c: New test. * c-c++-common/pr69558.c: Remove xfail. * gcc.dg/cpp/builtin-macro-1.c: Adjust test expectations. * gcc.dg/pr61817-1.c: Likewise. * gcc.dg/pr61817-2.c: Likewise. * g++.dg/plugin/pragma_plugin.c: Warn at expansion_point_location.