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

Hi Steve,

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.

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

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?

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

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.

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

- ???

> 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?

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

This I did fix.

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

Thanks for your feedback!

Harald

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.

Hello,

Le 20/11/2023 à 20:02, Steve Kargl a écrit :
> 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.
> 
Harald, you mentioned the lack of GFC_STD_F2023_DEL feature group in 
your first message, but I don't quite understand why you didn't add one. 
  It seems to me the most natural way to do this.

>> 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?
> 
I suggest we emit a warning by default, error with -std=f2023 (I agree 
with Steve that we should push towards strict f2023 conformance), and no 
diagnostic with -std=gnu or -std=f2018 or lower.

> 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.

Agreed, these seem to explicitly test mismatching kinds, so add an 
option to prevent error.

Mikael