Fortran: reject procedures and procedure pointers as output item [PR107074]

  Dear all,

when looking at output item lists we didn't catch procedures
and procedure pointers and ran into a gfc_internal_error().
Such items are not allowed by the Fortran standard, e.g. for
procedure pointers there is

C1233 (R1217) An expression that is an output-item shall not
              have a value that is a procedure pointer.

Attached patch generates an error instead.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald
  

Hello

Le 04/10/2022 à 21:27, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> when looking at output item lists we didn't catch procedures
> and procedure pointers and ran into a gfc_internal_error().
> Such items are not allowed by the Fortran standard, e.g. for
> procedure pointers there is
> 
> C1233 (R1217) An expression that is an output-item shall not
>                have a value that is a procedure pointer.
> 
> Attached patch generates an error instead.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
Please move the check to resolve_transfer in resolve.cc.

Strangely, the patch doesn't seem to fix the problem on the testcase 
here.  There is an outer parenthese expression preventing the condition 
you added from triggering.  Can you double check?

If we take the standard to the letter, only output items are forbidden, 
so a check is missing for writing context.  I don't know how it can work 
for input items though, so maybe not worth it.  In any case, the error 
shouldn't mention output items in reading context.

Here is a variant of the testcase with procedure pointer components, 
that fails differently but can probably be caught as well.

program p
   implicit none
   type :: t
     procedure(f), pointer, nopass :: b
   end type t
   type(t) :: a

   interface
     real function f()
     end function f
   end interface

   print *, merge (a%b, a%b, .true.)
end
  

Hi Mikael,

> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
> Von: "Mikael Morin" <morin-mikael@orange.fr>
> Please move the check to resolve_transfer in resolve.cc.

I have done this, see attached updated patch.

Regtests cleanly on x86_64-pc-linux-gnu.

> Strangely, the patch doesn't seem to fix the problem on the testcase
> here.  There is an outer parenthese expression preventing the condition
> you added from triggering.  Can you double check?

You are right: I had a one-liner in my worktree from PR105371 that
fixes an issue with gfc_simplify_merge and that seems to help here.
It is now included.

> If we take the standard to the letter, only output items are forbidden,
> so a check is missing for writing context.  I don't know how it can work
> for input items though, so maybe not worth it.  In any case, the error
> shouldn't mention output items in reading context.
>
> Here is a variant of the testcase with procedure pointer components,
> that fails differently but can probably be caught as well.
>
> program p
>    implicit none
>    type :: t
>      procedure(f), pointer, nopass :: b
>    end type t
>    type(t) :: a
>
>    interface
>      real function f()
>      end function f
>    end interface
>
>    print *, merge (a%b, a%b, .true.)
> end

I hadn't thought about this, and found a solution that also fixes this
one.  Great example!  This is now an additional test.

OK for mainline?

And thanks for your comments!

Harald
  

Le 05/10/2022 à 22:40, Harald Anlauf a écrit :
> Hi Mikael,
> 
>> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
>> Von: "Mikael Morin" <morin-mikael@orange.fr>
>> Please move the check to resolve_transfer in resolve.cc.
> 
> I have done this, see attached updated patch.
> 
> Regtests cleanly on x86_64-pc-linux-gnu.
> 
>> Strangely, the patch doesn't seem to fix the problem on the testcase
>> here.  There is an outer parenthese expression preventing the condition
>> you added from triggering.  Can you double check?
> 
> You are right: I had a one-liner in my worktree from PR105371 that
> fixes an issue with gfc_simplify_merge and that seems to help here.
> It is now included.
> 
The rest looks good, but I'm not sure about your one-liner.
I will try to come with a real test later, but in principle, if you have 
a call to FOO(MERGE(A,A,.TRUE.)) you can't simplify it to FOO(A) as 
writes to the argument in FOO should not overwrite the content of A. 
The dummy should be associated with a temporary value, not to A.
  

Le 06/10/2022 à 10:37, Mikael Morin a écrit :
> Le 05/10/2022 à 22:40, Harald Anlauf a écrit :
>> Hi Mikael,
>>
>>> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
>>> Von: "Mikael Morin" <morin-mikael@orange.fr>
>>> Please move the check to resolve_transfer in resolve.cc.
>>
>> I have done this, see attached updated patch.
>>
>> Regtests cleanly on x86_64-pc-linux-gnu.
>>
>>> Strangely, the patch doesn't seem to fix the problem on the testcase
>>> here.  There is an outer parenthese expression preventing the condition
>>> you added from triggering.  Can you double check?
>>
>> You are right: I had a one-liner in my worktree from PR105371 that
>> fixes an issue with gfc_simplify_merge and that seems to help here.
>> It is now included.
>>
> The rest looks good, but I'm not sure about your one-liner.
> I will try to come with a real test later, but in principle, if you have 
> a call to FOO(MERGE(A,A,.TRUE.)) you can't simplify it to FOO(A) as 
> writes to the argument in FOO should not overwrite the content of A. The 
> dummy should be associated with a temporary value, not to A.
> 
Here is a test that your patch breaks.
Admittedly it's rejected if A has the INTENT(INOUT) attribute, but 
without it, I think it's allowed.

program p
   integer :: b
   b = 1
   call foo(merge(b,b,.true.))
   if (b /= 1) stop 1
contains
   subroutine foo(a)
     integer :: a
     if (a == 1) a = 42
   end subroutine foo
end program p
  

Hi Mikael,

I definitely agree that we need a temporary for the result of
MERGE(a,a,.true.), I just haven't found out how to do that.

The reason for the bad one-liner was that in gfc_simplify_merge

	result = gfc_get_parentheses (result);

actually does have issues, in that the subsequent

      gfc_simplify_expr (result, 1);

seems to fail in interesting cases (as in PR105371).

So that is something to look into...

Cheers,
Harald

> Gesendet: Donnerstag, 06. Oktober 2022 um 22:32 Uhr
> Von: "Mikael Morin" <morin-mikael@orange.fr>
> An: "Harald Anlauf" <anlauf@gmx.de>
> Cc: "gcc-patches" <gcc-patches@gcc.gnu.org>, "fortran" <fortran@gcc.gnu.org>
> Betreff: Re: [PATCH, v2] Fortran: reject procedures and procedure pointers as IO element [PR107074]
>
> Le 06/10/2022 à 10:37, Mikael Morin a écrit :
> > Le 05/10/2022 à 22:40, Harald Anlauf a écrit :
> >> Hi Mikael,
> >>
> >>> Gesendet: Mittwoch, 05. Oktober 2022 um 12:34 Uhr
> >>> Von: "Mikael Morin" <morin-mikael@orange.fr>
> >>> Please move the check to resolve_transfer in resolve.cc.
> >>
> >> I have done this, see attached updated patch.
> >>
> >> Regtests cleanly on x86_64-pc-linux-gnu.
> >>
> >>> Strangely, the patch doesn't seem to fix the problem on the testcase
> >>> here.  There is an outer parenthese expression preventing the condition
> >>> you added from triggering.  Can you double check?
> >>
> >> You are right: I had a one-liner in my worktree from PR105371 that
> >> fixes an issue with gfc_simplify_merge and that seems to help here.
> >> It is now included.
> >>
> > The rest looks good, but I'm not sure about your one-liner.
> > I will try to come with a real test later, but in principle, if you have 
> > a call to FOO(MERGE(A,A,.TRUE.)) you can't simplify it to FOO(A) as 
> > writes to the argument in FOO should not overwrite the content of A. The 
> > dummy should be associated with a temporary value, not to A.
> > 
> Here is a test that your patch breaks.
> Admittedly it's rejected if A has the INTENT(INOUT) attribute, but 
> without it, I think it's allowed.
> 
> program p
>    integer :: b
>    b = 1
>    call foo(merge(b,b,.true.))
>    if (b /= 1) stop 1
> contains
>    subroutine foo(a)
>      integer :: a
>      if (a == 1) a = 42
>    end subroutine foo
> end program p
> 
>