[PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
Dear all,
The attached patch fixes PR105658 by forcing an array temporary to be
created. This is required when passing an array component, but this
didn't happen if the dummy argument was an unlimited polymorphic type.
The problem bit of code is in `gfc_conv_expr_descriptor`, near L7828:
subref_array_target = (is_subref_array (expr)
&& (se->direct_byref
|| expr->ts.type == BT_CHARACTER));
need_tmp = (gfc_ref_needs_temporary_p (expr->ref)
&& !subref_array_target);
where `need_tmp` is being evaluated to 0. The logic here isn't clear
to me, and this function is used in several places, which is why I
went with setting `parmse.force_tmp = 1` in `gfc_conv_procedure_call`
and using the same conditional as the later branch for the
non-polymorphic case (near the call to `gfc_conv_subref_array_arg`)
If this patch is ok, please could someone commit it for me? This is my
first patch for GCC, so apologies in advance if the commit message is
missing something.
Tested on x86_64-pc-linux-gnu.
The bug is present in gfortran back to 4.9, so should it also be backported?
Cheers,
Peter
PR fortran/105658
gcc/fortran/ChangeLog
* trans-expr.cc (gfc_conv_procedure_call): When passing an
array component reference of intrinsic type to a procedure
with an unlimited polymorphic dummy argument, a temporary
should be created.
gcc/testsuite/ChangeLog
* gfortran.dg/PR105658.f90: New test.
---
gcc/fortran/trans-expr.cc | 8
gcc/testsuite/gfortran.dg/PR105658.f90 | 25 +
2 files changed, 33 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index a0593b76f18..7fd3047c4e9 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6439,6 +6439,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
CLASS object for the unlimited polymorphic formal. */
gfc_find_vtab (&e->ts);
gfc_init_se (&parmse, se);
+ /* The actual argument is a component reference to an array
+ of derived types, so we need to force creation of a
+ temporary */
+ if (e->expr_type == EXPR_VARIABLE
+ && is_subref_array (e)
+ && !(fsym && fsym->attr.pointer))
+ parmse.force_tmp = 1;
+
gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts);
}
diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90
b/gcc/testsuite/gfortran.dg/PR105658.f90
new file mode 100644
index 000..407ee25f77c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/PR105658.f90
@@ -0,0 +1,25 @@
+! { dg-do compile }
+! { dg-options "-Warray-temporaries" }
+! Test fix for incorrectly passing array component to unlimited
polymorphic procedure
+
+module test_PR105658_mod
+ implicit none
+ type :: foo
+integer :: member1
+integer :: member2
+ end type foo
+contains
+ subroutine print_poly(array)
+class(*), dimension(:), intent(in) :: array
+select type(array)
+type is (integer)
+ print*, array
+end select
+ end subroutine print_poly
+
+ subroutine do_print(thing)
+type(foo), dimension(3), intent(in) :: thing
+call print_poly(thing%member1) ! { dg-warning "array temporary" }
+ end subroutine do_print
+
+end module test_PR105658_mod
--
2.43.0
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
Hi Harald,
Thanks for your help, please see the updated and signed-off patch below.
> (I am not entirely sure whether we need to exclude pointer and
> allocatable attributes here explicitly, given the constraints
> in F2023:15.5.2.6, but other may have an opinion, too.
> The above should be safe anyway.)
I've included them in the patch here, but it does seem to work fine
without checking those attributes here -- and invalid code is still
caught with that change.
It also occurred to me that array temporaries aren't _required_ here
(for arrays of derived type components), but in the general case with
a type with differently sized components, the stride wouldn't be a
multiple of the component's type's size. Is it possible in principle
to have an arbitrary stride?
Cheers,
Peter
>From 907a104facfc7f35f48ebcfa9ef5f8f5430d4d3c Mon Sep 17 00:00:00 2001
From: Peter Hill
Date: Thu, 15 Feb 2024 16:58:33 +
Subject: [PATCH] Fortran: fix passing array component ref to polymorphic
procedures
PR fortran/105658
gcc/fortran/ChangeLog
* trans-expr.cc (gfc_conv_intrinsic_to_class): When passing an
array component reference of intrinsic type to a procedure
with an unlimited polymorphic dummy argument, a temporary
should be created.
gcc/testsuite/ChangeLog
* gfortran.dg/PR105658.f90: New test.
Signed-off-by: Peter Hill
---
gcc/fortran/trans-expr.cc | 9 +
gcc/testsuite/gfortran.dg/PR105658.f90 | 50 ++
2 files changed, 59 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index a0593b76f18..004081aa6c3 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -1019,6 +1019,14 @@ gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e,
tmp = gfc_typenode_for_spec (&class_ts);
var = gfc_create_var (tmp, "class");
+ /* Force a temporary for component or substring references */
+ if (unlimited_poly
+ && class_ts.u.derived->components->attr.dimension
+ && !class_ts.u.derived->components->attr.allocatable
+ && !class_ts.u.derived->components->attr.class_pointer
+ && is_subref_array (e))
+parmse->force_tmp = 1;
+
/* Set the vptr. */
ctree = gfc_class_vptr_get (var);
@@ -6439,6 +6447,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
CLASS object for the unlimited polymorphic formal. */
gfc_find_vtab (&e->ts);
gfc_init_se (&parmse, se);
+
gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts);
}
diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90
b/gcc/testsuite/gfortran.dg/PR105658.f90
new file mode 100644
index 000..8aacecf806e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/PR105658.f90
@@ -0,0 +1,50 @@
+! { dg-do compile }
+! { dg-options "-Warray-temporaries" }
+! Test fix for incorrectly passing array component to unlimited
polymorphic procedure
+
+module test_PR105658_mod
+ implicit none
+ type :: foo
+ integer :: member1
+ integer :: member2
+ end type foo
+contains
+ subroutine print_poly(array)
+ class(*), dimension(:), intent(in) :: array
+ select type(array)
+ type is (integer)
+ print*, array
+ type is (character(*))
+ print *, array
+ end select
+ end subroutine print_poly
+
+ subroutine do_print(thing)
+ type(foo), dimension(3), intent(in) :: thing
+ type(foo), parameter :: y(3) = [foo(1,2),foo(3,4),foo(5,6)]
+ integer :: i, j, uu(5,6)
+
+ call print_poly(thing%member1) ! { dg-warning "array temporary" }
+ call print_poly(y%member2) ! { dg-warning "array temporary" }
+ call print_poly(y(1::2)%member2) ! { dg-warning "array temporary" }
+
+ ! The following array sections work without temporaries
+ uu = reshape([(((10*i+j),i=1,5),j=1,6)],[5,6])
+ print *, uu(2,2::2)
+ call print_poly (uu(2,2::2)) ! no temp needed!
+ print *, uu(1::2,6)
+ call print_poly (uu(1::2,6)) ! no temp needed!
+ end subroutine do_print
+
+ subroutine do_print2(thing2)
+ class(foo), dimension(:), intent(in) :: thing2
+ call print_poly (thing2% member2) ! { dg-warning "array temporary" }
+ end subroutine do_print2
+
+ subroutine do_print3 ()
+ character(3) :: c(3) = ["abc","def","ghi"]
+ call print_poly (c(1::2)) ! no temp needed!
+ call print_poly (c(1::2)(2:3)) ! { dg-warning "array temporary" }
+ end subroutine do_print3
+
+end module test_PR105658_mod
--
2.43.0
Re: [PATCH] Fortran: fix check for non-optional arrays passed to elemental
On Thu, 27 Feb 2025 at 18:09, Jerry D wrote: > > On 2/27/25 7:38 AM, Peter Hill wrote: > > Dear all, > > > > The attached patch fixes an ICE in gfc_resolve_code when passing an > > optional array to an elemental procedure with `-pedantic` enabled. > > PR95446 added the original check, this patch fixes the case where the > > other actual argument is an array literal (or something else other > > than a variable). The ICE is present since 11.1, so this could be > > backported? > > > > Cheers, > > Peter > > > > Hi Peter, was there a PR associated with this one? > > Jerry > > --- snip --- Hi Jerry, Nope, I couldn't find one -- should I have created one first? Cheers, Peter
Re: [PATCH] Fortran: fix check for non-optional arrays passed to elemental
No problem, created PR119054 with a reproducer and some details. Cheers, Peter On Thu, 27 Feb 2025 at 20:45, Jerry D wrote: > > On 2/27/25 12:33 PM, Peter Hill wrote: > > On Thu, 27 Feb 2025 at 18:09, Jerry D wrote: > >> > >> On 2/27/25 7:38 AM, Peter Hill wrote: > >>> Dear all, > >>> > >>> The attached patch fixes an ICE in gfc_resolve_code when passing an > >>> optional array to an elemental procedure with `-pedantic` enabled. > >>> PR95446 added the original check, this patch fixes the case where the > >>> other actual argument is an array literal (or something else other > >>> than a variable). The ICE is present since 11.1, so this could be > >>> backported? > >>> > >>> Cheers, > >>> Peter > >>> > >> > >> Hi Peter, was there a PR associated with this one? > >> > >> Jerry > >> > >> --- snip --- > > > > Hi Jerry, > > > > Nope, I couldn't find one -- should I have created one first? > > > > Cheers, > > Peter > > Yes please with a testcase that illustrates the problem. This way we > capture some hsitory,if you dont mind. > > Jerry
[PATCH] Fortran: fix check for non-optional arrays passed to elemental
Dear all,
The attached patch fixes an ICE in gfc_resolve_code when passing an
optional array to an elemental procedure with `-pedantic` enabled.
PR95446 added the original check, this patch fixes the case where the
other actual argument is an array literal (or something else other
than a variable). The ICE is present since 11.1, so this could be
backported?
Cheers,
Peter
gcc/fortran/Changelog
* resolve.cc (resolve_elemental_actual): When checking other
actual arguments to elemental procedures, don't check
attributes of literals and function calls
gcc/testsuite/Changelog
* gfortran.dg/pr95446.f90: Expand test case to literals and
function calls
Signed-off-by: Peter Hill
---
gcc/fortran/resolve.cc| 4 +++-
gcc/testsuite/gfortran.dg/pr95446.f90 | 14 ++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 6a83a7967a8..bf602389d5b 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -2429,7 +2429,9 @@ resolve_elemental_actual (gfc_expr *expr, gfc_code *c)
for (a = arg0; a; a = a->next)
if (a != arg
&& a->expr->rank == arg->expr->rank
- && !a->expr->symtree->n.sym->attr.optional)
+ && (a->expr->expr_type != EXPR_VARIABLE
+ || (a->expr->expr_type == EXPR_VARIABLE
+ && !a->expr->symtree->n.sym->attr.optional)))
{
t = true;
break;
diff --git a/gcc/testsuite/gfortran.dg/pr95446.f90
b/gcc/testsuite/gfortran.dg/pr95446.f90
index 86e1019d7af..0787658813a 100644
--- a/gcc/testsuite/gfortran.dg/pr95446.f90
+++ b/gcc/testsuite/gfortran.dg/pr95446.f90
@@ -22,6 +22,20 @@ program elemental_optional
end function outer
+ function outer_literal(o) result(l)
+integer, intent(in), optional :: o(5)
+integer :: l(5)
+
+l = inner(o, [1,2,3,4,5])
+ end function outer_literal
+
+ function outer_func(o) result(l)
+integer, intent(in), optional :: o(5)
+integer :: l(5)
+
+l = inner(o, outer())
+ end function outer_func
+
elemental function inner(a,b) result(x)
integer, intent(in), optional :: a
integer, intent(in) :: b
--
2.48.1
