On Wed, Nov 7, 2018 at 5:32 PM Jakub Jelinek wrote: > > On Wed, Nov 07, 2018 at 05:05:13PM -0500, Fritz Reese wrote: > > --- a/gcc/fortran/options.c > +++ b/gcc/fortran/options.c > @@ -32,6 +32,20 @@ along with GCC; see the file COPYING3. If not see > > gfc_option_t gfc_option; > > +#define _expand(m) m > > I think it would be better to avoid names like _expand, too generic > name and starts with underscore, name it e.g. SET_BITFLAG_1 or something > similar. And it isn't mentioned in the ChangeLog. I find "expand" a more helpful name than "set_bitflag_1" since it describes what the macro does. However, I don't think it makes too much of a difference so I'll follow your preference (but I'll use SET_BITFLAG2 since then the definition line fits in 80 characters). > > @@ -62,14 +75,30 @@ set_dec_flags (int value) > } > > What about the > /* Allow legacy code without warnings. */ > gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL > | GFC_STD_GNU | GFC_STD_LEGACY; > gfc_option.warn_std &= ~(GFC_STD_LEGACY | GFC_STD_F95_DEL); > that is done for value, shouldn't set_dec_flags remove those > flags again? Maybe not the allow_std ones, because those are set already by > default, perhaps just the warn_std flags? > Sure. I wasn't convinced about this and how it might interplay with -std= so I left it alone, but I suppose it makes sense to unsuppress the warnings when disabling -fdec. > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/array_temporaries_5.f90 > @@ -0,0 +1,20 @@ > +! { dg-do run } > +! { dg-options "-fcheck-array-temporaries -fno-check-array-temporaries" } > +! > +! PR fortran/87919 > +! > +! Ensure -fno-check-array-temporaries disables array temporary checking. > +! Copied from array_temporaries_2.f90. > > For tests where you expect no errors and that are just copies of other > testcases, perhaps > include 'array_temporaries_2.f90' > or similar instead? > Strictly speaking it's not an exact copy because it omits the final { dg-output } check for the runtime warning, since the warning is supposed to occur in array_temporaries_2.f90 but not in the new case array_temporaries_5.f90. I assumed include would propagate the { dg-output } check -- upon actually testing this, it appears that is not the case. I find this misleading at a glance, but it works, so I don't mind this with an extra comment line. Attached is a new patch which incorporates your recommendations. Here's the diff between the two, followed by the new changelog: diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c index e703cad96fd..167621905bc 100644 --- a/gcc/fortran/options.c +++ b/gcc/fortran/options.c @@ -32,8 +32,6 @@ along with GCC; see the file COPYING3. If not see gfc_option_t gfc_option; -#define _expand(m) m - #define SET_FLAG(flag, condition, on_value, off_value) \ do \ { \ @@ -43,8 +41,10 @@ gfc_option_t gfc_option; flag = (off_value); \ } while (0) +#define SET_BITFLAG2(m) m + #define SET_BITFLAG(flag, condition, value) \ - _expand (SET_FLAG (flag, condition, (flag | (value)), (flag & ~(value)))) + SET_BITFLAG2 (SET_FLAG (flag, condition, (flag | (value)), (flag & ~(value)))) /* Set flags that control warnings and errors for different @@ -66,15 +66,17 @@ set_default_std_flags (void) static void set_dec_flags (int value) { + /* Allow legacy code without warnings. + Nb. We do not unset the allowed standards with value == 0 because + they are set by default in set_default_std_flags. */ if (value) - { - /* Allow legacy code without warnings. */ - gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL - | GFC_STD_GNU | GFC_STD_LEGACY; - gfc_option.warn_std &= ~(GFC_STD_LEGACY | GFC_STD_F95_DEL); - } + gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL + | GFC_STD_GNU | GFC_STD_LEGACY; + + SET_BITFLAG (gfc_option.warn_std, !value, GFC_STD_LEGACY); + SET_BITFLAG (gfc_option.warn_std, !value, GFC_STD_F95_DEL); - /* Set other DEC compatibility extensions. */ + /* Set (or unset) other DEC compatibility extensions. */ SET_BITFLAG (flag_dollar_ok, value, value); SET_BITFLAG (flag_cray_pointer, value, value); SET_BITFLAG (flag_dec_structure, value, value); @@ -855,3 +871,7 @@ gfc_get_option_string (void) } #undef SET_BITFLAG +#undef SET_BITFLAG2 #undef SET_FLAG -#undef _expand diff --git a/gcc/testsuite/gfortran.dg/array_temporaries_5.f90 b/gcc/testsuite/gfortran.dg/array_temporaries_5.f90 index 0070a935933..dd147ba38ed 100644 --- a/gcc/testsuite/gfortran.dg/array_temporaries_5.f90 +++ b/gcc/testsuite/gfortran.dg/array_temporaries_5.f90 @@ -4,17 +4,7 @@ ! PR fortran/87919 ! ! Ensure -fno-check-array-temporaries disables array temporary checking. -! Copied from array_temporaries_2.f90. ! -program test - implicit none - integer :: a(3,3) - call foo(a(:,1)) ! OK, no temporary created - call foo(a(1,:)) ! BAD, temporary var created -contains - subroutine foo(x) - integer :: x(3) - x = 5 - end subroutine foo -end program test +! Note that 'include' drops the dg-output check from the original test case. +include 'array_temporaries_2.f90' >>>>>> ChangeLog <<<<<< PR fortran/87919 Fix handling -fno-* prefix for init-local-zero, check-array-temporaries and dec. gcc/fortran/ * options.c (SET_FLAG, SET_BITFLAG, SET_BITFLAG2): New macros. (set_dec_flags): Set/unset DEC and std flags according to value. (set_init_local_zero): New helper for -finit-local-zero flag group. (gfc_init_options): Fix disabling of init flags, array temporaries check, and dec flags when value is zero (from -fno-*). gcc/testsuiste/ * gfortran.dg/array_temporaries_5.f90: New test. * gfortran.dg/dec_bitwise_ops_3.f90: Ditto. * gfortran.dg/dec_d_lines_3.f: Ditto. * gfortran.dg/dec_exp_4.f90: Ditto. * gfortran.dg/dec_exp_5.f90: Ditto. * gfortran.dg/dec_io_7.f90: Ditto. * gfortran.dg/dec_structure_24.f: Ditto. * gfortran.dg/dec_structure_25.f: Ditto. * gfortran.dg/dec_structure_26.f: Ditto. * gfortran.dg/dec_structure_27.f: Ditto. * gfortran.dg/dec_type_print_3.f90: Ditto. * gfortran.dg/init_flag_20.f90: Ditto.