Fortran: function results never have the ALLOCATABLE attribute [PR109500]

Message ID trinity-28f2e76d-4032-4ca5-8666-7faf6caf6c05-1682020919629@3c-app-gmx-bs34
State Accepted
Headers
Series Fortran: function results never have the ALLOCATABLE attribute [PR109500] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Harald Anlauf April 20, 2023, 8:01 p.m. UTC
  Dear all,

Fortran 2018 added a clarification that the *result* of a function
whose result *variable* has the ALLOCATABLE attribute is a *value*
that itself does not have the ALLOCATABLE attribute.

For those interested: there was a thread on the J3 mailing list
some time ago (for links see the PR).

The patch which implements a related check was co-authored with
Steve and regtested by him.  Testcase verified against NAG.

OK for mainline (gcc-14)?

Thanks,
Harald & Steve
  

Comments

Mikael Morin April 22, 2023, 9:25 a.m. UTC | #1
Hello,

Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> Fortran 2018 added a clarification that the *result* of a function
> whose result *variable* has the ALLOCATABLE attribute is a *value*
> that itself does not have the ALLOCATABLE attribute.
> 
> For those interested: there was a thread on the J3 mailing list
> some time ago (for links see the PR).
> 
> The patch which implements a related check was co-authored with
> Steve and regtested by him.  Testcase verified against NAG.
> 
> OK for mainline (gcc-14)?
> 
Looks good in principle, but I think the real fix should be in the 
gfc_expr_attr function, which copies all the attributes (including 
allocatable) in the EXPR_FUNCTION case.  How would the testsuite react 
if that attribute was cleared there?  Is your patch still needed if 
gfc_expr_attr is fixed?

Mikael
  
Li, Pan2 via Gcc-patches April 22, 2023, 1:52 p.m. UTC | #2
On Sat, Apr 22, 2023 at 11:25:41AM +0200, Mikael Morin wrote:
> 
> Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit :
> > Dear all,
> > 
> > Fortran 2018 added a clarification that the *result* of a function
> > whose result *variable* has the ALLOCATABLE attribute is a *value*
> > that itself does not have the ALLOCATABLE attribute.
> > 
> > For those interested: there was a thread on the J3 mailing list
> > some time ago (for links see the PR).
> > 
> > The patch which implements a related check was co-authored with
> > Steve and regtested by him.  Testcase verified against NAG.
> > 
> > OK for mainline (gcc-14)?
> > 
> Looks good in principle, but I think the real fix should be in the
> gfc_expr_attr function, which copies all the attributes (including
> allocatable) in the EXPR_FUNCTION case.  How would the testsuite react if
> that attribute was cleared there?  Is your patch still needed if
> gfc_expr_attr is fixed?

You may be correct that something can be done elsewhere.
I do note that a function result can be allocatable
(within the funciton body).  The issue only arises when
argument association is done, which is done where Harald
and I have the patch.  Do we know that the function will
be an actual argument associated with an allocatable 
dummy argument when gfc_expr_attr is invoked?
  
Mikael Morin April 22, 2023, 3:17 p.m. UTC | #3
Le 22/04/2023 à 15:52, Steve Kargl a écrit :
> On Sat, Apr 22, 2023 at 11:25:41AM +0200, Mikael Morin wrote:
>>
>> Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit :
>>> Dear all,
>>>
>>> Fortran 2018 added a clarification that the *result* of a function
>>> whose result *variable* has the ALLOCATABLE attribute is a *value*
>>> that itself does not have the ALLOCATABLE attribute.
>>>
>>> For those interested: there was a thread on the J3 mailing list
>>> some time ago (for links see the PR).
>>>
>>> The patch which implements a related check was co-authored with
>>> Steve and regtested by him.  Testcase verified against NAG.
>>>
>>> OK for mainline (gcc-14)?
>>>
>> Looks good in principle, but I think the real fix should be in the
>> gfc_expr_attr function, which copies all the attributes (including
>> allocatable) in the EXPR_FUNCTION case.  How would the testsuite react if
>> that attribute was cleared there?  Is your patch still needed if
>> gfc_expr_attr is fixed?
> 
> You may be correct that something can be done elsewhere.
> I do note that a function result can be allocatable
> (within the funciton body).  The issue only arises when
> argument association is done, which is done where Harald
> and I have the patch.  Do we know that the function will
> be an actual argument associated with an allocatable
> dummy argument when gfc_expr_attr is invoked?
> 
No, there is no context information in gfc_expr_attr, but the result 
should not be dependent on context anyway.

You are probably right that the impact of this bug is limited to the 
case of argument association, not as broad as I thought.  Yet we should 
not keep gfc_expr_attr returning an allocatable attribute for function 
expressions in any case.
  
Li, Pan2 via Gcc-patches April 22, 2023, 4:54 p.m. UTC | #4
On Sat, Apr 22, 2023 at 05:17:30PM +0200, Mikael Morin wrote:
> Le 22/04/2023 à 15:52, Steve Kargl a écrit :
> > On Sat, Apr 22, 2023 at 11:25:41AM +0200, Mikael Morin wrote:
> > > 
> > > Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit :
> > > > Dear all,
> > > > 
> > > > Fortran 2018 added a clarification that the *result* of a function
> > > > whose result *variable* has the ALLOCATABLE attribute is a *value*
> > > > that itself does not have the ALLOCATABLE attribute.
> > > > 
> > > > For those interested: there was a thread on the J3 mailing list
> > > > some time ago (for links see the PR).
> > > > 
> > > > The patch which implements a related check was co-authored with
> > > > Steve and regtested by him.  Testcase verified against NAG.
> > > > 
> > > > OK for mainline (gcc-14)?
> > > > 
> > > Looks good in principle, but I think the real fix should be in the
> > > gfc_expr_attr function, which copies all the attributes (including
> > > allocatable) in the EXPR_FUNCTION case.  How would the testsuite react if
> > > that attribute was cleared there?  Is your patch still needed if
> > > gfc_expr_attr is fixed?
> > 
> > You may be correct that something can be done elsewhere.
> > I do note that a function result can be allocatable
> > (within the funciton body).  The issue only arises when
> > argument association is done, which is done where Harald
> > and I have the patch.  Do we know that the function will
> > be an actual argument associated with an allocatable
> > dummy argument when gfc_expr_attr is invoked?
> > 
> No, there is no context information in gfc_expr_attr, but the result should
> not be dependent on context anyway.
> 
> You are probably right that the impact of this bug is limited to the case of
> argument association, not as broad as I thought.  Yet we should not keep
> gfc_expr_attr returning an allocatable attribute for function expressions in
> any case.

I suspect we're stuck in a catch-22 situation.  The symbol is
marked as allocatable,

function foo()
   integer, allocatable :: foo
   foo = 42   !<--- So that this isn't rejected
end

but when the function is actually referenced in an expression
the result is normally used, and symbol is still marked as
allocatable.
  
Harald Anlauf April 22, 2023, 6:19 p.m. UTC | #5
Hi Mikael,

Am 22.04.23 um 11:25 schrieb Mikael Morin:
> Hello,
>
> Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit :
>> Dear all,
>>
>> Fortran 2018 added a clarification that the *result* of a function
>> whose result *variable* has the ALLOCATABLE attribute is a *value*
>> that itself does not have the ALLOCATABLE attribute.
>>
>> For those interested: there was a thread on the J3 mailing list
>> some time ago (for links see the PR).
>>
>> The patch which implements a related check was co-authored with
>> Steve and regtested by him.  Testcase verified against NAG.
>>
>> OK for mainline (gcc-14)?
>>
> Looks good in principle, but I think the real fix should be in the
> gfc_expr_attr function, which copies all the attributes (including
> allocatable) in the EXPR_FUNCTION case.  How would the testsuite react
> if that attribute was cleared there?  Is your patch still needed if
> gfc_expr_attr is fixed?

you mean like the following?

diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
index 00d35a71770..7517efc5414 100644
--- a/gcc/fortran/primary.cc
+++ b/gcc/fortran/primary.cc
@@ -2775,6 +2775,7 @@ gfc_expr_attr (gfc_expr *e)
               attr.pointer = CLASS_DATA (sym)->attr.class_pointer;
               attr.allocatable = CLASS_DATA (sym)->attr.allocatable;
             }
+         attr.allocatable = 0;
         }
        else if (e->value.function.isym
                && e->value.function.isym->transformational

While this leads to a rejection of the testcase, I see regressions
e.g. on allocatable_function_1.f90 and allocatable_function_8.f90
because the function result from a previous invocation does not get
freed, and on a subsequent function reference the result variable
should always be unallocated.

Not sure if the "catch-22" Steve mentions is a good characterization,
but a function reference with assignment of the result to an
(allocatable) variable, like

   integer, allocatable  :: p
   p = f()

is semantically different from an ordinary assignment to an
allocatable variable, where the r.h.s. is an allocatable variable,
because the function result variable *must* be deallocated after
the assignment, whereas an ordinary variable on the r.h.s must
remain unaltered.

So I guess it is much less risky to approach the issue by not
allowing argument association to an allocatable dummy for an
actual argument that is a function reference.  (I initially had
an even stricter idea to allow only an allocatable *variable*
for the actual argument, but did not check the lengthy text
on argument association).

> Mikael
>

Harald
  
Mikael Morin April 22, 2023, 6:43 p.m. UTC | #6
Le 22/04/2023 à 20:19, Harald Anlauf a écrit :
> Hi Mikael,
> 
> Am 22.04.23 um 11:25 schrieb Mikael Morin:
>> Hello,
>>
>> Le 20/04/2023 à 22:01, Harald Anlauf via Fortran a écrit :
>>> Dear all,
>>>
>>> Fortran 2018 added a clarification that the *result* of a function
>>> whose result *variable* has the ALLOCATABLE attribute is a *value*
>>> that itself does not have the ALLOCATABLE attribute.
>>>
>>> For those interested: there was a thread on the J3 mailing list
>>> some time ago (for links see the PR).
>>>
>>> The patch which implements a related check was co-authored with
>>> Steve and regtested by him.  Testcase verified against NAG.
>>>
>>> OK for mainline (gcc-14)?
>>>
>> Looks good in principle, but I think the real fix should be in the
>> gfc_expr_attr function, which copies all the attributes (including
>> allocatable) in the EXPR_FUNCTION case.  How would the testsuite react
>> if that attribute was cleared there?  Is your patch still needed if
>> gfc_expr_attr is fixed?
> 
> you mean like the following?
> 
> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
> index 00d35a71770..7517efc5414 100644
> --- a/gcc/fortran/primary.cc
> +++ b/gcc/fortran/primary.cc
> @@ -2775,6 +2775,7 @@ gfc_expr_attr (gfc_expr *e)
>                attr.pointer = CLASS_DATA (sym)->attr.class_pointer;
>                attr.allocatable = CLASS_DATA (sym)->attr.allocatable;
>              }
> +         attr.allocatable = 0;
>          }
>         else if (e->value.function.isym
>                 && e->value.function.isym->transformational
> 
Yes, like this.

> While this leads to a rejection of the testcase, I see regressions
> e.g. on allocatable_function_1.f90 and allocatable_function_8.f90
> because the function result from a previous invocation does not get
> freed, and on a subsequent function reference the result variable
> should always be unallocated.
> 
Meh!

> Not sure if the "catch-22" Steve mentions is a good characterization,
> but a function reference with assignment of the result to an
> (allocatable) variable, like
> 
>    integer, allocatable  :: p
>    p = f()
> 
> is semantically different from an ordinary assignment to an
> allocatable variable, where the r.h.s. is an allocatable variable,
> because the function result variable *must* be deallocated after
> the assignment, whereas an ordinary variable on the r.h.s must
> remain unaltered.
> 
> So I guess it is much less risky to approach the issue by not
> allowing argument association to an allocatable dummy for an
> actual argument that is a function reference.  (I initially had
> an even stricter idea to allow only an allocatable *variable*
> for the actual argument, but did not check the lengthy text
> on argument association).
> 
OK, let's go with your patch as originally submitted then.

Thanks.
  
Li, Pan2 via Gcc-patches April 24, 2023, 6:31 p.m. UTC | #7
On Sat, Apr 22, 2023 at 08:43:53PM +0200, Mikael Morin wrote:
> > 
> OK, let's go with your patch as originally submitted then.
> 

Mikael, thanks for looking at the original patch and
suggested an alternative location to attempt to fix
the bug.  Halrald, thanks for following up on Mikael's
suggestion.
  

Patch

From 2cebc8f9e7b399b7747c9ad0392831de91851b5b Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Thu, 20 Apr 2023 21:47:34 +0200
Subject: [PATCH] Fortran: function results never have the ALLOCATABLE
 attribute [PR109500]

Fortran 2018 8.5.3 (ALLOCATABLE attribute) explains in Note 1 that the
result of referencing a function whose result variable has the ALLOCATABLE
attribute is a value that does not itself have the ALLOCATABLE attribute.

gcc/fortran/ChangeLog:

	PR fortran/109500
	* interface.cc (gfc_compare_actual_formal): Reject allocatable
	functions being used as actual argument for allocable dummy.

gcc/testsuite/ChangeLog:

	PR fortran/109500
	* gfortran.dg/allocatable_function_11.f90: New test.

Co-authored-by: Steven G. Kargl <kargl@gcc.gnu.org>
---
 gcc/fortran/interface.cc                      | 12 +++++++
 .../gfortran.dg/allocatable_function_11.f90   | 36 +++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/allocatable_function_11.f90

diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc
index e9843e9549c..968ee193c07 100644
--- a/gcc/fortran/interface.cc
+++ b/gcc/fortran/interface.cc
@@ -3638,6 +3638,18 @@  gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
 	  goto match;
 	}

+      if (a->expr->expr_type == EXPR_FUNCTION
+	  && a->expr->value.function.esym
+	  && f->sym->attr.allocatable)
+	{
+	  if (where)
+	    gfc_error ("Actual argument for %qs at %L is a function result "
+		       "and the dummy argument is ALLOCATABLE",
+		       f->sym->name, &a->expr->where);
+	  ok = false;
+	  goto match;
+	}
+
       /* Check intent = OUT/INOUT for definable actual argument.  */
       if (!in_statement_function
 	  && (f->sym->attr.intent == INTENT_OUT
diff --git a/gcc/testsuite/gfortran.dg/allocatable_function_11.f90 b/gcc/testsuite/gfortran.dg/allocatable_function_11.f90
new file mode 100644
index 00000000000..1a2831e186f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocatable_function_11.f90
@@ -0,0 +1,36 @@ 
+! { dg-do compile }
+! PR fortran/109500 - check F2018:8.5.3 Note 1
+!
+! The result of referencing a function whose result variable has the
+! ALLOCATABLE attribute is a value that does not itself have the
+! ALLOCATABLE attribute.
+
+program main
+  implicit none
+  integer, allocatable  :: p
+  procedure(f), pointer :: pp
+  pp => f
+  p = f()
+  print *, allocated (p)
+  print *, is_allocated (p)
+  print *, is_allocated (f())  ! { dg-error "is a function result" }
+  print *, is_allocated (pp()) ! { dg-error "is a function result" }
+  call s (p)
+  call s (f())  ! { dg-error "is a function result" }
+  call s (pp()) ! { dg-error "is a function result" }
+
+contains
+  subroutine s(p)
+    integer, allocatable :: p
+  end subroutine s
+
+  function f()
+    integer, allocatable :: f
+    allocate (f, source=42)
+  end function
+
+  logical function is_allocated(p)
+    integer, allocatable :: p
+    is_allocated = allocated(p)
+  end function
+end program
--
2.35.3