| Message ID | trinity-6d934a50-8304-4704-bce4-36a2afbc687e-1664911631690@3c-app-gmx-bs14 |
|---|---|
| State | New |
| Headers | show |
| Series | Fortran: reject procedures and procedure pointers as output item [PR107074] | expand |
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
>
>
From 3b15fe83830c1e75339114e0241e9d2158393017 Mon Sep 17 00:00:00 2001 From: Harald Anlauf <anlauf@gmx.de> Date: Tue, 4 Oct 2022 21:19:21 +0200 Subject: [PATCH] Fortran: reject procedures and procedure pointers as output item [PR107074] gcc/fortran/ChangeLog: PR fortran/107074 * trans-io.cc (transfer_expr): A procedure or a procedure pointer cannot be output items. gcc/testsuite/ChangeLog: PR fortran/107074 * gfortran.dg/pr107074.f90: New test. --- gcc/fortran/trans-io.cc | 14 ++++++++++++++ gcc/testsuite/gfortran.dg/pr107074.f90 | 11 +++++++++++ 2 files changed, 25 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr107074.f90 diff --git a/gcc/fortran/trans-io.cc b/gcc/fortran/trans-io.cc index 9f86815388c..c4e1537eed6 100644 --- a/gcc/fortran/trans-io.cc +++ b/gcc/fortran/trans-io.cc @@ -2430,6 +2430,20 @@ transfer_expr (gfc_se * se, gfc_typespec * ts, tree addr_expr, break; + case BT_PROCEDURE: + if (code->expr1 + && code->expr1->symtree + && code->expr1->symtree->n.sym) + { + if (code->expr1->symtree->n.sym->attr.proc_pointer) + gfc_error ("Procedure pointer at %C cannot be an output item"); + else + gfc_error ("Procedure at %C cannot be an output item"); + return; + } + /* If a PROCEDURE item gets through to here, fall through and ICE. */ + gcc_fallthrough (); + case_bt_struct: case BT_CLASS: if (gfc_bt_struct (ts->type) || ts->type == BT_CLASS) diff --git a/gcc/testsuite/gfortran.dg/pr107074.f90 b/gcc/testsuite/gfortran.dg/pr107074.f90 new file mode 100644 index 00000000000..a09088c2e9d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr107074.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR fortran/107074 - ICE: Bad IO basetype (8) +! Contributed by G.Steinmetz + +program p + implicit none + integer, external :: a + procedure(real), pointer :: b + print *, merge (a, a, .true.) ! { dg-error "Procedure" } + print *, merge (b, b, .true.) ! { dg-error "Procedure pointer" } +end -- 2.35.3
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