| Message ID | 20200430130227.GV2424@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | diagnostics: get_option_html_page fixes | expand |
On Thu, 2020-04-30 at 15:02 +0200, Jakub Jelinek wrote: > Hi! > > (Sorry to David, apparently I've posted it only privately, not to > gcc-patches, so reposting). > > While testing the --with-documentation-root-url= changes, I run into > [Wreturn-type] URL pointing to gfortran documentation where it > obviously > isn't documented. The following patch updates the list of options to > match > reality (on the other side -Wconversion-extra is gfortran only option > documented in gfortran.texi). > > Or, perhaps better use the attached patch instead, which doesn't have > a > hardcoded list and instead uses the flags? I went through options.c > and the updated list of options matches exactly the cases where > CL_Fortran > is set for "-W*" options together with CL_C and/or CL_CXX (ok, there > is also > -Wall and -Wextra, but hopefully we don't emit [Wall] or [Wextra] for > anything). > > I have successfully bootstrapped/regtested the second patch (CL_C & > CL_CXX) > on x86_64-linux and i686-linux, ok for trunk, or do you prefer the > first > one? Thanks for working on this; sorry for getting these wrong. Is is possible to build gfortran without C and C++? If so, then if I'm reading things right the second patch (CL_C & CL_CXX) would get it wrong for various Fortran warnings for such a build, as the CL_C/CL_CXX wouldn't be defined and it will erroneously use the Fortran docs page, rather than the common one for them. The second patch seems so much cleaner that having a hardcoded list, but getting the correct result is more important. Dave
On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote: > Thanks for working on this; sorry for getting these wrong. > > Is is possible to build gfortran without C and C++? If so, then if I'm It is possible without C++, but not without C. E.g. --enable-languages=fortran,go will actually enable c,fortran,go,lto. *,c,*) ;; *) enable_languages=c,${enable_languages} ;; So, strictly speaking the #ifdef CL_C isn't needed there, so if you want, I can drop that #ifdef and only use CL_CXX ifdef. Jakub
On Thu, 2020-04-30 at 23:26 +0200, Jakub Jelinek wrote: > On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote: > > Thanks for working on this; sorry for getting these wrong. > > > > Is is possible to build gfortran without C and C++? If so, then if > > I'm > > It is possible without C++, but not without C. > > E.g. --enable-languages=fortran,go will actually enable > c,fortran,go,lto. > *,c,*) > ;; > *) > enable_languages=c,${enable_languages} > ;; > So, strictly speaking the #ifdef CL_C isn't needed there, so if you > want, > I can drop that #ifdef and only use CL_CXX ifdef. Thanks. Perhaps a silly question, but does the patch give the right answers if someone does that? (which I think is equivalent to "Are there any options labeled with both Fortran and C++ but not C that are documented in gcc/Warning-Options.html"?) Dave
On Thu, Apr 30, 2020 at 05:31:22PM -0400, David Malcolm wrote: > On Thu, 2020-04-30 at 23:26 +0200, Jakub Jelinek wrote: > > On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote: > > > Thanks for working on this; sorry for getting these wrong. > > > > > > Is is possible to build gfortran without C and C++? If so, then if > > > I'm > > > > It is possible without C++, but not without C. > > > > E.g. --enable-languages=fortran,go will actually enable > > c,fortran,go,lto. > > *,c,*) > > ;; > > *) > > enable_languages=c,${enable_languages} > > ;; > > So, strictly speaking the #ifdef CL_C isn't needed there, so if you > > want, > > I can drop that #ifdef and only use CL_CXX ifdef. > > Thanks. > > Perhaps a silly question, but does the patch give the right answers if > someone does that? (which I think is equivalent to "Are there any > options labeled with both Fortran and C++ but not C that are documented > in gcc/Warning-Options.html"?) ATM all the -W* options with CL_Fortran in the mask are either CL_C | CL_CXX | CL_Fortran ... | CL_WARNING, or CL_Fortran ... | CL_WARNING (not enabled for anything but Fortran) Jakub
On Thu, 2020-04-30 at 23:38 +0200, Jakub Jelinek wrote: > On Thu, Apr 30, 2020 at 05:31:22PM -0400, David Malcolm wrote: > > On Thu, 2020-04-30 at 23:26 +0200, Jakub Jelinek wrote: > > > On Thu, Apr 30, 2020 at 05:18:13PM -0400, David Malcolm wrote: > > > > Thanks for working on this; sorry for getting these wrong. > > > > > > > > Is is possible to build gfortran without C and C++? If so, > > > > then if > > > > I'm > > > > > > It is possible without C++, but not without C. > > > > > > E.g. --enable-languages=fortran,go will actually enable > > > c,fortran,go,lto. > > > *,c,*) > > > ;; > > > *) > > > enable_languages=c,${enable_languages} > > > ;; > > > So, strictly speaking the #ifdef CL_C isn't needed there, so if > > > you > > > want, > > > I can drop that #ifdef and only use CL_CXX ifdef. > > > > Thanks. > > > > Perhaps a silly question, but does the patch give the right answers > > if > > someone does that? (which I think is equivalent to "Are there any > > options labeled with both Fortran and C++ but not C that are > > documented > > in gcc/Warning-Options.html"?) > > ATM all the -W* options with CL_Fortran in the mask > are either CL_C | CL_CXX | CL_Fortran ... | CL_WARNING, > or CL_Fortran ... | CL_WARNING (not enabled for anything but Fortran) > Thanks. Let's go with your second patch then (CL_C & CL_CXX). Dave
--- gcc/opts.c.jj 2020-04-30 09:37:17.039303011 +0200 +++ gcc/opts.c 2020-04-30 10:27:30.871585618 +0200 @@ -3151,9 +3151,13 @@ get_option_html_page (int option_index) case OPT_Wdate_time: case OPT_Wconversion: - case OPT_Wconversion_extra: + case OPT_Wmaybe_uninitialized: case OPT_Wmissing_include_dirs: case OPT_Wopenmp_simd: + case OPT_Wpedantic: + case OPT_Wreturn_type: + case OPT_Wuninitialized: + case OPT_Wunused: /* These warnings are marked in fortran/lang.opt as "Documented in C" and thus use the common Warning-Options page below. */
Hi! (Sorry to David, apparently I've posted it only privately, not to gcc-patches, so reposting). While testing the --with-documentation-root-url= changes, I run into [Wreturn-type] URL pointing to gfortran documentation where it obviously isn't documented. The following patch updates the list of options to match reality (on the other side -Wconversion-extra is gfortran only option documented in gfortran.texi). Or, perhaps better use the attached patch instead, which doesn't have a hardcoded list and instead uses the flags? I went through options.c and the updated list of options matches exactly the cases where CL_Fortran is set for "-W*" options together with CL_C and/or CL_CXX (ok, there is also -Wall and -Wextra, but hopefully we don't emit [Wall] or [Wextra] for anything). I have successfully bootstrapped/regtested the second patch (CL_C & CL_CXX) on x86_64-linux and i686-linux, ok for trunk, or do you prefer the first one? 2020-04-30 Jakub Jelinek <jakub@redhat.com> * opts.c (get_option_html_page): For OPT_Wconversion_extra look in gfortran documentation rather than in generic, while for OPT_Wmaybe_uninitialized, OPT_Wpedantic, OPT_Wreturn_type, OPT_Wuninitialized and OPT_Wunused look in generic documentation rather than gfortran. Jakub 2020-04-30 Jakub Jelinek <jakub@redhat.com> * opts.c (get_option_html_page): Instead of hardcoding a list of options common between C/C++ and Fortran only use gfortran/ documentation for warnings that have CL_Fortran set but not CL_C or CL_CXX. --- gcc/opts.c.jj 2020-04-30 09:37:17.039303011 +0200 +++ gcc/opts.c 2020-04-30 10:54:20.753713744 +0200 @@ -3141,25 +3141,17 @@ get_option_html_page (int option_index) return "gcc/Static-Analyzer-Options.html"; #ifdef CL_Fortran - if (cl_opt->flags & CL_Fortran) - { - switch (option_index) - { - default: - /* Most Fortran warnings are documented on this page. */ - return "gfortran/Error-and-Warning-Options.html"; - - case OPT_Wdate_time: - case OPT_Wconversion: - case OPT_Wconversion_extra: - case OPT_Wmissing_include_dirs: - case OPT_Wopenmp_simd: - /* These warnings are marked in fortran/lang.opt as - "Documented in C" and thus use the common - Warning-Options page below. */ - break; - } - } + if ((cl_opt->flags & CL_Fortran) != 0 + /* If it is option common to both C/C++ and Fortran, it is documented + in gcc/ rather than gfortran/ docs. */ +#ifdef CL_C + && (cl_opt->flags & CL_C) == 0 +#endif +#ifdef CL_CXX + && (cl_opt->flags & CL_CXX) == 0 +#endif + ) + return "gfortran/Error-and-Warning-Options.html"; #endif return "gcc/Warning-Options.html";