Re: ping: [PATCH] libcpp: Improve location for macro names [PR66290]
May I please ping this one? FWIW, it's 10 months old now without any feedback. https://gcc.gnu.org/pipermail/gcc-patches/2022-December/607647.html Most of the changes are just adapting the testsuite to look for the improved diagnostic location. Otherwise it's a handful of lines in libcpp and it just changes this: t.cpp:1: warning: macro "X" is not used [-Wunused-macros] 1 | #define X 1 | to this: t.cpp:1:9: warning: macro "X" is not used [-Wunused-macros] 1 | #define X 1 | ^ which closes out PR66290. Thank you! -Lewis On Thu, Jan 12, 2023 at 6:31 PM Lewis Hyatt wrote: > > https://gcc.gnu.org/pipermail/gcc-patches/2022-December/607647.html > May I please ping this one again? It will enable closing out the PR. Thanks! > > -Lewis > > On Thu, Dec 1, 2022 at 9:22 AM Lewis Hyatt wrote: > > > > Hello- > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html > > > > May I please ping this one? Thanks! > > I have also re-attached the rebased patch here. > > > > -Lewis > > > > On Wed, Oct 12, 2022 at 06:37:50PM -0400, Lewis Hyatt wrote: > > > Hello- > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html > > > > > > Since Jeff was kind enough to ack one of my other preprocessor patches > > > today, I have become emboldened to ping this one again too :). Would > > > anyone have some time to take a look at it please? Thanks! > > > > > > -Lewis > > > > > > On Thu, Sep 15, 2022 at 6:31 PM Lewis Hyatt wrote: > > > > > > > > Hello- > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html > > > > May I please ping this patch? Thank you. > > > > > > > > -Lewis > > > > > > > > On Fri, Aug 5, 2022 at 12:14 PM Lewis Hyatt wrote: > > > > > > > > > > > > > > > When libcpp reports diagnostics whose locus is a macro name (such as > > > > > for > > > > > -Wunused-macros), it uses the location in the cpp_macro object that > > > > > was > > > > > stored by _cpp_new_macro. This is currently set to > > > > > pfile->directive_line, > > > > > which contains the line number only and no column information. This > > > > > patch > > > > > changes the stored location to the src_loc for the token defining the > > > > > macro > > > > > name, which includes the location and range information. > > > > > > > > > > libcpp/ChangeLog: > > > > > > > > > > PR c++/66290 > > > > > * macro.cc (_cpp_create_definition): Add location argument. > > > > > * internal.h (_cpp_create_definition): Adjust prototype. > > > > > * directives.cc (do_define): Pass new location argument to > > > > > _cpp_create_definition. > > > > > (do_undef): Stop passing inferior location to > > > > > cpp_warning_with_line; > > > > > the default from cpp_warning is better. > > > > > (cpp_pop_definition): Pass new location argument to > > > > > _cpp_create_definition. > > > > > * pch.cc (cpp_read_state): Likewise. > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > PR c++/66290 > > > > > * c-c++-common/cpp/macro-ranges.c: New test. > > > > > * c-c++-common/cpp/line-2.c: Adapt to check for column > > > > > information > > > > > on macro-related libcpp warnings. > > > > > * c-c++-common/cpp/line-3.c: Likewise. > > > > > * c-c++-common/cpp/macro-arg-count-1.c: Likewise. > > > > > * c-c++-common/cpp/pr58844-1.c: Likewise. > > > > > * c-c++-common/cpp/pr58844-2.c: Likewise. > > > > > * c-c++-common/cpp/warning-zero-location.c: Likewise. > > > > > * c-c++-common/pragma-diag-14.c: Likewise. > > > > > * c-c++-common/pragma-diag-15.c: Likewise. > > > > > * g++.dg/modules/macro-2_d.C: Likewise. > > > > > * g++.dg/modules/macro-4_d.C: Likewise. > > > > > * g++.dg/modules/macro-4_e.C: Likewise. > > > > > * g++.dg/spellcheck-macro-ordering.C: Likewise. > > > > > * gcc.dg/builtin-redefine.c: Likewise. > > > > > * gcc.dg/cpp/Wunused.c: Likewise. > > > > > * gcc.dg/cpp/redef2.c: Likewise. > > > > > * gcc.dg/cpp/redef3.c: Likewise. > > > > > * gcc.dg/cpp/redef4.c: Likewise. > > > > > * gcc.dg/cpp/ucnid-11-utf8.c: Likewise. > > > > > * gcc.dg/cpp/ucnid-11.c: Likewise. > > > > > * gcc.dg/cpp/undef2.c: Likewise. > > > > > * gcc.dg/cpp/warn-redefined-2.c: Likewise. > > > > > * gcc.dg/cpp/warn-redefined.c: Likewise. > > > > > * gcc.dg/cpp/warn-unused-macros-2.c: Likewise. > > > > > * gcc.dg/cpp/warn-unused-macros.c: Likewise. > > > > > --- > > > > > > > > > > Notes: > > > > > Hello- > > > > > > > > > > The PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66290) was > > > > > originally > > > > > about the entirely wrong location for -Wunused-macros in C++ > > > > > mode, which > > > > > behavior was fixed by r13-1903, but before closing it out I > > > > > wanted to also > > > >
ping: [PATCH] libcpp: Improve location for macro names [PR66290]
https://gcc.gnu.org/pipermail/gcc-patches/2022-December/607647.html May I please ping this one again? It will enable closing out the PR. Thanks! -Lewis On Thu, Dec 1, 2022 at 9:22 AM Lewis Hyatt wrote: > > Hello- > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html > > May I please ping this one? Thanks! > I have also re-attached the rebased patch here. > > -Lewis > > On Wed, Oct 12, 2022 at 06:37:50PM -0400, Lewis Hyatt wrote: > > Hello- > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html > > > > Since Jeff was kind enough to ack one of my other preprocessor patches > > today, I have become emboldened to ping this one again too :). Would > > anyone have some time to take a look at it please? Thanks! > > > > -Lewis > > > > On Thu, Sep 15, 2022 at 6:31 PM Lewis Hyatt wrote: > > > > > > Hello- > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html > > > May I please ping this patch? Thank you. > > > > > > -Lewis > > > > > > On Fri, Aug 5, 2022 at 12:14 PM Lewis Hyatt wrote: > > > > > > > > > > > > When libcpp reports diagnostics whose locus is a macro name (such as for > > > > -Wunused-macros), it uses the location in the cpp_macro object that was > > > > stored by _cpp_new_macro. This is currently set to > > > > pfile->directive_line, > > > > which contains the line number only and no column information. This > > > > patch > > > > changes the stored location to the src_loc for the token defining the > > > > macro > > > > name, which includes the location and range information. > > > > > > > > libcpp/ChangeLog: > > > > > > > > PR c++/66290 > > > > * macro.cc (_cpp_create_definition): Add location argument. > > > > * internal.h (_cpp_create_definition): Adjust prototype. > > > > * directives.cc (do_define): Pass new location argument to > > > > _cpp_create_definition. > > > > (do_undef): Stop passing inferior location to > > > > cpp_warning_with_line; > > > > the default from cpp_warning is better. > > > > (cpp_pop_definition): Pass new location argument to > > > > _cpp_create_definition. > > > > * pch.cc (cpp_read_state): Likewise. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR c++/66290 > > > > * c-c++-common/cpp/macro-ranges.c: New test. > > > > * c-c++-common/cpp/line-2.c: Adapt to check for column > > > > information > > > > on macro-related libcpp warnings. > > > > * c-c++-common/cpp/line-3.c: Likewise. > > > > * c-c++-common/cpp/macro-arg-count-1.c: Likewise. > > > > * c-c++-common/cpp/pr58844-1.c: Likewise. > > > > * c-c++-common/cpp/pr58844-2.c: Likewise. > > > > * c-c++-common/cpp/warning-zero-location.c: Likewise. > > > > * c-c++-common/pragma-diag-14.c: Likewise. > > > > * c-c++-common/pragma-diag-15.c: Likewise. > > > > * g++.dg/modules/macro-2_d.C: Likewise. > > > > * g++.dg/modules/macro-4_d.C: Likewise. > > > > * g++.dg/modules/macro-4_e.C: Likewise. > > > > * g++.dg/spellcheck-macro-ordering.C: Likewise. > > > > * gcc.dg/builtin-redefine.c: Likewise. > > > > * gcc.dg/cpp/Wunused.c: Likewise. > > > > * gcc.dg/cpp/redef2.c: Likewise. > > > > * gcc.dg/cpp/redef3.c: Likewise. > > > > * gcc.dg/cpp/redef4.c: Likewise. > > > > * gcc.dg/cpp/ucnid-11-utf8.c: Likewise. > > > > * gcc.dg/cpp/ucnid-11.c: Likewise. > > > > * gcc.dg/cpp/undef2.c: Likewise. > > > > * gcc.dg/cpp/warn-redefined-2.c: Likewise. > > > > * gcc.dg/cpp/warn-redefined.c: Likewise. > > > > * gcc.dg/cpp/warn-unused-macros-2.c: Likewise. > > > > * gcc.dg/cpp/warn-unused-macros.c: Likewise. > > > > --- > > > > > > > > Notes: > > > > Hello- > > > > > > > > The PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66290) was > > > > originally > > > > about the entirely wrong location for -Wunused-macros in C++ mode, > > > > which > > > > behavior was fixed by r13-1903, but before closing it out I wanted > > > > to also > > > > address a second point brought up in the PR comments, namely that > > > > we do not > > > > include column information when emitting diagnostics for macro > > > > names, such as > > > > is done for -Wunused-macros. The attached patch updates the > > > > location stored in > > > > the cpp_macro object so that it includes the column and range > > > > information for > > > > the token comprising the macro name; previously, the location was > > > > just the > > > > generic one pointing to the whole line. > > > > > > > > The change to libcpp is very small, the reason for all the > > > > testsuite changes is > > > > that I have updated all tests explicitly looking for the columnless > > > > diagnostics > > > > (with the "-:" syntax to dg-warning et al) so that they expect a > > > > column >
Ping: [PATCH] libcpp: Improve location for macro names [PR66290]
Hello- https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599397.html May I please ping this patch? Thank you. -Lewis On Fri, Aug 5, 2022 at 12:14 PM Lewis Hyatt wrote: > > > When libcpp reports diagnostics whose locus is a macro name (such as for > -Wunused-macros), it uses the location in the cpp_macro object that was > stored by _cpp_new_macro. This is currently set to pfile->directive_line, > which contains the line number only and no column information. This patch > changes the stored location to the src_loc for the token defining the macro > name, which includes the location and range information. > > libcpp/ChangeLog: > > PR c++/66290 > * macro.cc (_cpp_create_definition): Add location argument. > * internal.h (_cpp_create_definition): Adjust prototype. > * directives.cc (do_define): Pass new location argument to > _cpp_create_definition. > (do_undef): Stop passing inferior location to cpp_warning_with_line; > the default from cpp_warning is better. > (cpp_pop_definition): Pass new location argument to > _cpp_create_definition. > * pch.cc (cpp_read_state): Likewise. > > gcc/testsuite/ChangeLog: > > PR c++/66290 > * c-c++-common/cpp/macro-ranges.c: New test. > * c-c++-common/cpp/line-2.c: Adapt to check for column information > on macro-related libcpp warnings. > * c-c++-common/cpp/line-3.c: Likewise. > * c-c++-common/cpp/macro-arg-count-1.c: Likewise. > * c-c++-common/cpp/pr58844-1.c: Likewise. > * c-c++-common/cpp/pr58844-2.c: Likewise. > * c-c++-common/cpp/warning-zero-location.c: Likewise. > * c-c++-common/pragma-diag-14.c: Likewise. > * c-c++-common/pragma-diag-15.c: Likewise. > * g++.dg/modules/macro-2_d.C: Likewise. > * g++.dg/modules/macro-4_d.C: Likewise. > * g++.dg/modules/macro-4_e.C: Likewise. > * g++.dg/spellcheck-macro-ordering.C: Likewise. > * gcc.dg/builtin-redefine.c: Likewise. > * gcc.dg/cpp/Wunused.c: Likewise. > * gcc.dg/cpp/redef2.c: Likewise. > * gcc.dg/cpp/redef3.c: Likewise. > * gcc.dg/cpp/redef4.c: Likewise. > * gcc.dg/cpp/ucnid-11-utf8.c: Likewise. > * gcc.dg/cpp/ucnid-11.c: Likewise. > * gcc.dg/cpp/undef2.c: Likewise. > * gcc.dg/cpp/warn-redefined-2.c: Likewise. > * gcc.dg/cpp/warn-redefined.c: Likewise. > * gcc.dg/cpp/warn-unused-macros-2.c: Likewise. > * gcc.dg/cpp/warn-unused-macros.c: Likewise. > --- > > Notes: > Hello- > > The PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66290) was originally > about the entirely wrong location for -Wunused-macros in C++ mode, which > behavior was fixed by r13-1903, but before closing it out I wanted to also > address a second point brought up in the PR comments, namely that we do > not > include column information when emitting diagnostics for macro names, > such as > is done for -Wunused-macros. The attached patch updates the location > stored in > the cpp_macro object so that it includes the column and range information > for > the token comprising the macro name; previously, the location was just the > generic one pointing to the whole line. > > The change to libcpp is very small, the reason for all the testsuite > changes is > that I have updated all tests explicitly looking for the columnless > diagnostics > (with the "-:" syntax to dg-warning et al) so that they expect a column > instead. I also added a new test which verifies the expected range > information > in diagnostics with carets. > > Bootstrap + regtest on x86-64 Linux looks good. Please let me know if it > looks > OK? Thanks! > > -Lewis > > libcpp/directives.cc | 13 +- > libcpp/internal.h | 2 +- > libcpp/macro.cc | 12 +- > libcpp/pch.cc | 2 +- > gcc/testsuite/c-c++-common/cpp/line-2.c | 2 +- > gcc/testsuite/c-c++-common/cpp/line-3.c | 2 +- > .../c-c++-common/cpp/macro-arg-count-1.c | 4 +- > gcc/testsuite/c-c++-common/cpp/macro-ranges.c | 52 ++ > gcc/testsuite/c-c++-common/cpp/pr58844-1.c| 4 +- > gcc/testsuite/c-c++-common/cpp/pr58844-2.c| 4 +- > .../c-c++-common/cpp/warning-zero-location.c | 2 +- > gcc/testsuite/c-c++-common/pragma-diag-14.c | 2 +- > gcc/testsuite/c-c++-common/pragma-diag-15.c | 2 +- > gcc/testsuite/g++.dg/modules/macro-2_d.C | 4 +- > gcc/testsuite/g++.dg/modules/macro-4_d.C | 4 +- > gcc/testsuite/g++.dg/modules/macro-4_e.C | 2 +- > .../g++.dg/spellcheck-macro-ordering.C| 2 +- > gcc/testsuite/gcc.dg/builtin-redefine.c | 18 +- > gcc/testsuite/gcc.dg/cpp/Wunused.c| 6 +- > gcc/testsuite/gcc.dg/cpp/redef2.c | 20 +- > gcc/testsu