Starting with 228049 #define A \ _Pragma ("GCC diagnostic push") \ _Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"") #define B \ _Pragma ("GCC diagnostic pop") #define C(x) \ A \ static inline void bar (void) { x (); } \ B __attribute__((deprecated)) void foo (void); C (foo) warns about deprecated use, while before that we didn't warn.
r228049 , so that we get a link.
I think this is the same as bug 69543.
Well, that one has been introduced in r232893, while this one far earlier.
Reverting one minor part of those changes fixes both of the PRs though: --- gcc/c-family/c-pragma.c.jj 2016-01-15 21:57:00.000000000 +0100 +++ gcc/c-family/c-pragma.c 2016-01-29 18:34:51.743943283 +0100 @@ -819,7 +819,7 @@ handle_pragma_diagnostic(cpp_reader *ARG arg = option_string + 1 + cl_options[option_index].opt_len; control_warning_option (option_index, (int) kind, arg, kind != DK_IGNORED, - loc, lang_mask, &handlers, + input_location, lang_mask, &handlers, &global_options, &global_options_set, global_dc); } I have no idea why, but if it doesn't break anything else, I think it would be better to revert it. The ChangeLog nor gcc-patches post doesn't mention that change anywhere.
(In reply to Jakub Jelinek from comment #4) > Reverting one minor part of those changes fixes both of the PRs though: > --- gcc/c-family/c-pragma.c.jj 2016-01-15 21:57:00.000000000 +0100 > +++ gcc/c-family/c-pragma.c 2016-01-29 18:34:51.743943283 +0100 > @@ -819,7 +819,7 @@ handle_pragma_diagnostic(cpp_reader *ARG > arg = option_string + 1 + cl_options[option_index].opt_len; > control_warning_option (option_index, (int) kind, > arg, kind != DK_IGNORED, > - loc, lang_mask, &handlers, > + input_location, lang_mask, &handlers, > &global_options, &global_options_set, > global_dc); > } > I have no idea why, but if it doesn't break anything else, I think it would > be better to revert it. The ChangeLog nor gcc-patches post doesn't mention > that change anywhere. Perhaps input_location points to the expansion point and loc to the spelling point. The latter is correct when one wants to point to the error within the pragma, however, the former should be used if one wants a pragma within a macro to apply to the location where the macro is expanded. What if you resolve the loc to the expansion point above? input_location does not always point to the correct thing.
(In reply to Jakub Jelinek from comment #0) > warns about deprecated use, while before that we didn't warn. It would be useful to see the locations in the output (and whether the warning uses %q+), since this is what matters when deciding if the pragmas apply or not.
In handle_pragma_diagnostic (gdb) p input_location $1 = 324032 (gdb) p expand_location (input_location) $2 = {file = 0x7fffffffe472 "pr69558.c", line = 17, column = 1, data = 0x0, sysp = false} (gdb) p loc $3 = 324224 (gdb) p expand_location (loc) $4 = {file = 0x7fffffffe472 "pr69558.c", line = 17, column = 7, data = 0x0, sysp = false} and the warning: Breakpoint 6, warning (opt=213, gmsgid=0x1951058 "%qD is deprecated") at ../../gcc/diagnostic.c:972 972 rich_location richloc (line_table, input_location); (gdb) p input_location $5 = 324032 (gdb) p expand_location (input_location) $6 = {file = 0x7fffffffe472 "pr69558.c", line = 17, column = 1, data = 0x0, sysp = false}
So input_location does not point to foo but to column 1, then the expansion location of the pragmas is the closing paren because of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61817#c3 In terms of expanded locations, the code looks like this: foo _Pragma ("GCC diagnostic push") _Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"") static inline void bar (void) { (); } _Pragma ("GCC diagnostic pop") But if we use input_location, it looks like everything, all pragmas and foo, are at exactly column 1. This works as long as we do a linear search in the history. If we ever try to do binary search, we would need to be careful with duplicates. Also, I guess if we do: _Pragma ("GCC diagnostic ignored \"-Wdeprecated-declarations\"") \ _Pragma ("GCC diagnostic error \"-Wdeprecated-declarations\"") then the second one doesn't work. I cannot think a way to make the above work properly without breaking something else. To keep the GCC 5 behaviour without using input_location, we would need to have the location of foo at the time of handling the pragmas, which I don't see how it is possible. This seems too much at stage 3. I think your patch with some FIXME note pointing here is fine for GCC 6. At least, we will get the same behavior as in GCC 5 and this won't be a regression.
(In reply to Manuel López-Ibáñez from comment #8) > then the second one doesn't work. I cannot think a way to make the above > work properly without breaking something else. Actually, the history check uses: if (linemap_location_before_p (line_table, context->classification_history[i].location, location)) and that seems quite smart about how to compare things. Perhaps if we force input_location to point to foo, things will work as expected. In any case, no for GCC 6 and there are probably many cases where this won't work because we don't have a location for 'foo' ( e.g., C(1) ).
(In reply to Jakub Jelinek from comment #3) > Well, that one has been introduced in r232893, while this one far earlier. They could be symptoms of the same problem though. Quoting myself from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69543#c2 > linemap_compare_locations does the right thing if passed a pair of ordinary > locations, or if both locations are within the same macro expansion. > > It generates nonsensical results if one of the locations is in a macro map, > which is what's happening [in PR 69543] (the location of the _Pragma, > after r232893) ...and r228049 may have exposed the same issue for this PR. The ultimate right fix (for both) may be to fix linemap_compare_locations to cope with macro locations.
(In reply to David Malcolm from comment #10) > The ultimate right fix (for both) may be to fix linemap_compare_locations to > cope with macro locations. ...then again, Jakub's patch from comment #4 may be OK as a stopgap given we're in stage 4.
Author: jakub Date: Mon Feb 1 22:36:07 2016 New Revision: 233058 URL: https://gcc.gnu.org/viewcvs?rev=233058&root=gcc&view=rev Log: PR preprocessor/69543 PR c/69558 * c-pragma.c (handle_pragma_diagnostic): Pass input_location instead of loc to control_warning_option. * gcc.dg/pr69543.c: New test. * gcc.dg/pr69558.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr69543.c trunk/gcc/testsuite/gcc.dg/pr69558.c Modified: trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-pragma.c trunk/gcc/testsuite/ChangeLog
Workaround applied, keeping this open to fix it for real for GCC 7.
Author: dmalcolm Date: Tue Feb 23 17:39:16 2016 New Revision: 233637 URL: https://gcc.gnu.org/viewcvs?rev=233637&root=gcc&view=rev Log: Add test coverage for _Pragma (PR preprocessor 69126, 69543, 69558) We had some regressions in the ability for _Pragma to disable a warning (PR preprocessor/69126, PR preprocessor/69543, PR preprocessor/69558). This patch attempts to add more test coverage for this, for the various combinations of: - various warnings: -Wunused-variable -Wuninitialized -Wdeprecated-declarations - various combinations of location of _Pragma relative to location of the warning: - _Pragma is in a macro, warning isn't a macro - neither is in a macro - _Pragma isnt't in a macro, warning is in a macro - in different macros - both in the same macro - C vs C++ frontend. It adds some XFAILs: - pr69543-1.c for C++ (fixed in the followup patch) - pr69543-3.c for both C and C++ - pr69543-4.c for both C and C++ - pr69558.c for C++ (moving it from gcc.dg to c-c++-common, marking it as xfail for C++ for now) gcc/testsuite/ChangeLog: PR preprocessor/69126 PR preprocessor/69543 PR preprocessor/69558 * c-c++-common/pr69126.c (MACRO_1, test_1): New. (f): Rename to... (test_2): ...this, and add leading comment. (MACRO_3, test_3): New. (MACRO_4A, MACRO_4B, test_4): New. (MACRO): Rename to... (MACRO_5): ...this. (g): Rename to... (test_5): ...this, updating for renaming of MACRO, and add leading comment. * c-c++-common/pr69543-1.c: New. * c-c++-common/pr69543-2.c: New. * c-c++-common/pr69543-3.c: New. * c-c++-common/pr69543-4.c: New. * c-c++-common/pr69558-1.c: New. * c-c++-common/pr69558-2.c: New. * c-c++-common/pr69558-3.c: New. * c-c++-common/pr69558-4.c: New. * gcc.dg/pr69558.c: Move to... * c-c++-common/pr69558.c: ...here. Add dg-bogus directives, with xfail for c++. Added: trunk/gcc/testsuite/c-c++-common/pr69543-1.c trunk/gcc/testsuite/c-c++-common/pr69543-2.c trunk/gcc/testsuite/c-c++-common/pr69543-3.c trunk/gcc/testsuite/c-c++-common/pr69543-4.c trunk/gcc/testsuite/c-c++-common/pr69558-1.c trunk/gcc/testsuite/c-c++-common/pr69558-2.c trunk/gcc/testsuite/c-c++-common/pr69558-3.c trunk/gcc/testsuite/c-c++-common/pr69558-4.c trunk/gcc/testsuite/c-c++-common/pr69558.c - copied, changed from r233636, trunk/gcc/testsuite/gcc.dg/pr69558.c Removed: trunk/gcc/testsuite/gcc.dg/pr69558.c Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/pr69126.c
David, do we consider this fixed for real now with r233637, or is some further work planed?
Can we close this now?
Remaining XFAILs for this bug: c-c++-common/pr69558.c (C++ only)
We can't close it at this point -- we haven't really fixed the core issue. David's patch was primarily to give more testing coverage for future work in this space. I believe we're currently relying on Jakub's hack patch from c#12 to keep glib2 happy. If we're not going to address for gcc-7 we should probably change the target milestone.
Same arguments that deferred this to gcc-7 apply now to defer it to gcc-8.
*** Bug 79476 has been marked as a duplicate of this bug. ***
Same arguments that deferred this to gcc-8 apply now to defer it to gcc-9. David, do you think you could handle this for GCC9?
Assigning this to me in the hope that I'll get to it in gcc 9 stage 1.
(In reply to David Malcolm from comment #22) > Assigning this to me in the hope that I'll get to it in gcc 9 stage 1. It's gcc 9 stage 1 now.
(In reply to Jakub Jelinek from comment #4) > Reverting one minor part of those changes fixes both of the PRs though: > --- gcc/c-family/c-pragma.c.jj 2016-01-15 21:57:00.000000000 +0100 > +++ gcc/c-family/c-pragma.c 2016-01-29 18:34:51.743943283 +0100 > @@ -819,7 +819,7 @@ handle_pragma_diagnostic(cpp_reader *ARG > arg = option_string + 1 + cl_options[option_index].opt_len; > control_warning_option (option_index, (int) kind, > arg, kind != DK_IGNORED, > - loc, lang_mask, &handlers, > + input_location, lang_mask, &handlers, > &global_options, &global_options_set, > global_dc); > } > I have no idea why, but if it doesn't break anything else, I think it would > be better to revert it. The ChangeLog nor gcc-patches post doesn't mention > that change anywhere. FYI, I was there as well, and found the following. loc points to the location of ")" in "C (foo)", while input_location points to "C" in the C FE and to ")" in the C++ FE :-) The reason why this does still not work in C++ is, that the source code is sorted by location in this way: "void foo (void) _Pragma ("GCC diagnostic push") Pragma ("GCC diagnostic ignored")" at the time when the warning is issued. while your patch fixed the C FE, by using the location of the macro expansion pint for the _Pragma, and therefore the stack looked again way: "_Pragma ("push") _Pragma ("ignored") void foo (void)" This works only because the warning is printed immediately. For late warnings the situation would look like "_Pragma ("push") _Pragma ("ignored") _Pragma ("pop") warning_location"
Author: edlinger Date: Wed Jul 18 19:36:01 2018 New Revision: 262861 URL: https://gcc.gnu.org/viewcvs?rev=262861&root=gcc&view=rev Log: libcpp: 2018-07-18 Bernd Edlinger <bernd.edlinger@hotmail.de> PR 69558 * macro.c (enter_macro_context): Change the location info for builtin macros and _Pragma from location of the closing parenthesis to location of the macro expansion point. testsuite: 2018-07-18 Bernd Edlinger <bernd.edlinger@hotmail.de> PR 69558 * 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. Added: trunk/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-2.c Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/pr69558.c trunk/gcc/testsuite/g++.dg/plugin/pragma_plugin.c trunk/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c trunk/gcc/testsuite/gcc.dg/pr61817-1.c trunk/gcc/testsuite/gcc.dg/pr61817-2.c trunk/libcpp/ChangeLog trunk/libcpp/macro.c
(In reply to Bernd Edlinger from comment #25) > Author: edlinger > Date: Wed Jul 18 19:36:01 2018 > New Revision: 262861 > > URL: https://gcc.gnu.org/viewcvs?rev=262861&root=gcc&view=rev > Log: > libcpp: > 2018-07-18 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR 69558 > * macro.c (enter_macro_context): Change the location info for builtin > macros and _Pragma from location of the closing parenthesis to > location > of the macro expansion point. > > testsuite: > 2018-07-18 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR 69558 > * 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. > > Added: > trunk/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-2.c > Modified: > trunk/gcc/testsuite/ChangeLog > trunk/gcc/testsuite/c-c++-common/pr69558.c > trunk/gcc/testsuite/g++.dg/plugin/pragma_plugin.c > trunk/gcc/testsuite/gcc.dg/cpp/builtin-macro-1.c > trunk/gcc/testsuite/gcc.dg/pr61817-1.c > trunk/gcc/testsuite/gcc.dg/pr61817-2.c > trunk/libcpp/ChangeLog > trunk/libcpp/macro.c So this still needs to be backported to the 7 and 8 branches before this bug can be closed, correct?
GCC 9.1 has been released.
GCC 9.2 has been released.
Hm, the target milestone is wrong. I believe this was fixed in gcc-9
(In reply to Bernd Edlinger from comment #29) > Hm, the target milestone is wrong. I believe this was fixed in gcc-9 changing it to something starting in 8 then
5 years old bug can't be P1.
Marking as fixed in 9.1 then.