Re: [PATCH, v2] Fortran: restrictions on integer arguments to SYSTEM_CLOCK [PR112609]

[Steve Kargl <sgk@troutmask.apl.washington.edu>]

Harald, 

Sorry about delayed response.  Got side-tracked by Family this weekend.

On Sun, Nov 19, 2023 at 09:46:46PM +0100, Harald Anlauf wrote:
> 
> On 11/19/23 01:04, Steve Kargl wrote:
> > On Sat, Nov 18, 2023 at 11:12:55PM +0100, Harald Anlauf wrote:
> > > Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> > > 
> > 
> > Not in its current form.
> > 
> > >   {
> > > +  int first_int_kind = -1;
> > > +  bool f2023 = ((gfc_option.allow_std & GFC_STD_F2023) != 0
> > > +		&& (gfc_option.allow_std & GFC_STD_GNU) == 0);
> > > +
> > 
> > If you use the gfc_notify_std(), then you should not need the
> > above check on GFC_STD_GNU as it should include GFC_STD_F2023.
> 
> this is actually the question (and problem).  For all new features,
> -std=gnu shall include everything allowed by -std=f2023.

Yes.

> Here we have the problem that the testcase is valid F2018 and is
> silently accepted by gfortran-13 for -std=gnu and -std=f2018.

F2023 is the Fortran standard and supercedes previous Fortran standards.
If there is a conflict between the standing standard and an old standard,
then the standing standard should take precedence unless one specifically
uses, for example, -std=f2018.

After 20+ years of contributing to gfortran, I've come to believe
that the default -std= should be the current standard, and -std=gnu
should be deprecated.  All GNU extensions should require an option
to active.  For example,

   write(*,*), 'hello'
   end

   gfortran12 -o z a.f90
   a.f90:1:10:

   1 | write(*,*), 'hello'
     |           1
   Warning: Legacy Extension: Comma before i/o item list at (1)

This should be an error unless the -fallow-write-stmt-comma is used.
The option would simply degrade the error to a warning.  Why, you ask?
To encourage people to write standard conforming code.  Unfortunately,
that horse has left the barn.

> I prefer to keep it that way also for gfortran-14, and apply the
> new restrictions only for -std=f2023.  Do we agree on this?

If gfortran wants to maintain the status quo for 14, then
it should probably remove the -std=f2023 patch and wait for
the branch to 15.  

> Now that should happen for -std=gnu -pedantic (-w)?

-pedantic is not a very effective option and should be ignored.
 
> I have thought some more and came up with the revised attached
> patch, which still has the above condition.  It now marks the
> diagnostics as GNU extensions beyond F2023 for -std=f2023.
> 
> The mask f2023 in the above form suppresses new warnings even
> for -pedantic; one would normally use -w to suppress them.
> 
> Now if you remove the second part of the condition, we will
> regress on testcases system_clock_1.f90 and system_clock_3.f90
> because they would emit GNU extension warnings because the
> testsuite runs with -pedantic.

It seems that the solution is to fix the code in the testsuite.
With -std=f2023 or -std=gnu, the code should error.  With
-std=f2018 (or older?), the code should compile.

It's been too long for my memory, doesn't the use of
'{ dg-options "-std=f023" }' supercede the set of predefined options
such as -pedantic?

> The options I see:
> 
> - use patch-V1 (although diagnostics are better in V2),
> 
> - use patch-V2,
> 
> - use patch-V2, but enable -pedantic warnings for previously
>   valid code, and adjust the failing testcases

I suppose that this last one is the best option.
> 
> - ???
> 
> > Elsewhere in the FE, gfortran uses gfc_notify_std() to enforce
> > requirements of a Fortran standard.  The above would be
> > 
> >        if (count->ts.kind < gfc_default_integer_kind
> >            && gfc_notify_std (GFC_STD_F2023, "COUNT argument to SYSTEM_CLOCK "
> >                               "at %L must have kind of at least default integer",
> >                               &count->where))
> 
> I tried this first, and it did not do the job.
> 
> The logic in gfc_notify_std is:
> 
>   estd = std & ~gfc_option.allow_std;  /* Standard to error about.  */
>   error = (estd != 0);
>   if (error)
>     msg = notify_std_msg (estd);
> ...
> 
> So for -std=f2023 we get estd=0, error=false, and *NO* error.
> For -std=f2018 we get error=true and an error message.
> This is the opposite of what is needed.
> 
> Can you please try yourself?
> 

I was afraid you were going to say this.  :-) :-) 

The holidays draw near.  I can probably find the time to
poke at gfortran.

> > Note, gfc_notify_std() should add the 'Fortran 2023: ' string,
> > if not, that should be fixed.
> 
> This I did fix.

Thanks.

> > Of course, I seldom provide patches if others don't have a comment
> > then do as you like.
> 
> Thanks for your feedback!

No, thank you for continuing to peck away at gfortran issue.

-- 
steve