[PATCH 1/3] fortran: defer class wrapper initialization after deallocation [PR92178]
If an actual argument is associated with an INTENT(OUT) dummy, and code
to deallocate it is generated, generate the class wrapper initialization
after the actual argument deallocation.
This is achieved by passing a cleaned up expression to
gfc_conv_class_to_class, so that the class wrapper initialization code
can be isolated and moved independently after the deallocation.
PR fortran/92178
gcc/fortran/ChangeLog:
* trans-expr.cc (gfc_conv_procedure_call): Use a separate gfc_se
struct, initalized from parmse, to generate the class wrapper.
After the class wrapper code has been generated, copy it back
depending on whether parameter deallocation code has been
generated.
gcc/testsuite/ChangeLog:
* gfortran.dg/intent_out_19.f90: New test.
---
gcc/fortran/trans-expr.cc | 18 -
gcc/testsuite/gfortran.dg/intent_out_19.f90 | 22 +
2 files changed, 39 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/intent_out_19.f90
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 7017b652d6e..b7e95e6d04d 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6500,6 +6500,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
else
{
+ bool defer_to_dealloc_blk = false;
if (e->ts.type == BT_CLASS && fsym
&& fsym->ts.type == BT_CLASS
&& (!CLASS_DATA (fsym)->as
@@ -6661,6 +6662,8 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
stmtblock_t block;
tree ptr;
+ defer_to_dealloc_blk = true;
+
gfc_init_block (&block);
ptr = parmse.expr;
if (e->ts.type == BT_CLASS)
@@ -6717,7 +6720,12 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
&& ((CLASS_DATA (fsym)->as
&& CLASS_DATA (fsym)->as->type == AS_ASSUMED_RANK)
|| CLASS_DATA (e)->attr.dimension))
- gfc_conv_class_to_class (&parmse, e, fsym->ts, false,
+ {
+ gfc_se class_se = parmse;
+ gfc_init_block (&class_se.pre);
+ gfc_init_block (&class_se.post);
+
+ gfc_conv_class_to_class (&class_se, e, fsym->ts, false,
fsym->attr.intent != INTENT_IN
&& (CLASS_DATA (fsym)->attr.class_pointer
|| CLASS_DATA
(fsym)->attr.allocatable),
@@ -6727,6 +6735,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
CLASS_DATA (fsym)->attr.class_pointer
|| CLASS_DATA (fsym)->attr.allocatable);
+ parmse.expr = class_se.expr;
+ stmtblock_t *class_pre_block = defer_to_dealloc_blk
+? &dealloc_blk
+: &parmse.pre;
+ gfc_add_block_to_block (class_pre_block, &class_se.pre);
+ gfc_add_block_to_block (&parmse.post, &class_se.post);
+ }
+
if (fsym && (fsym->ts.type == BT_DERIVED
|| fsym->ts.type == BT_ASSUMED)
&& e->ts.type == BT_CLASS
diff --git a/gcc/testsuite/gfortran.dg/intent_out_19.f90
b/gcc/testsuite/gfortran.dg/intent_out_19.f90
new file mode 100644
index 000..03036ed382a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/intent_out_19.f90
@@ -0,0 +1,22 @@
+! { dg-do run }
+!
+! PR fortran/92178
+! Check that if a data reference passed is as actual argument whose dummy
+! has INTENT(OUT) attribute, any other argument depending on the
+! same data reference is evaluated before the data reference deallocation.
+
+program p
+ implicit none
+ class(*), allocatable :: c
+ c = 3
+ call bar (allocated(c), c, allocated (c))
+ if (allocated (c)) stop 14
+contains
+ subroutine bar (alloc, x, alloc2)
+logical :: alloc, alloc2
+class(*), allocatable, intent(out) :: x(..)
+if (allocated (x)) stop 5
+if (.not. alloc) stop 6
+if (.not. alloc2) stop 16
+ end subroutine bar
+end
--
2.40.1
[PATCH 0/3] Fix argument evaluation order [PR92178]
Hello, this is a followup to Harald's recent work [1] on the evaluation order of arguments, when one of them is passed to an intent(out) allocatable dummy and is deallocated before the call. This extends Harald's fix to support: - scalars passed to assumed rank dummies (patch 1), - scalars passed to assumed rank dummies with the data reference depending on its own content (patch 2), - arrays with the data reference depending on its own content (patch 3). There is one (last?) case which is not supported, for which I have opened a separate PR [2]. Regression tested on x86_64-pc-linux-gnu. OK for master? [1] https://gcc.gnu.org/pipermail/fortran/2023-July/059562.html [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110618 Mikael Morin (3): fortran: defer class wrapper initialization after deallocation [PR92178] fortran: Factor data references for scalar class argument wrapping [PR92178] fortran: Reorder array argument evaluation parts [PR92178] gcc/fortran/trans-array.cc | 3 + gcc/fortran/trans-expr.cc | 130 +--- gcc/fortran/trans.cc| 28 + gcc/fortran/trans.h | 8 +- gcc/testsuite/gfortran.dg/intent_out_19.f90 | 22 gcc/testsuite/gfortran.dg/intent_out_20.f90 | 33 + gcc/testsuite/gfortran.dg/intent_out_21.f90 | 33 + 7 files changed, 236 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/intent_out_19.f90 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_20.f90 create mode 100644 gcc/testsuite/gfortran.dg/intent_out_21.f90 -- 2.40.1
[PATCH 2/3] fortran: Factor data references for scalar class argument wrapping [PR92178]
In the case of a scalar actual arg passed to a polymorphic assumed-rank
dummy with INTENT(OUT) attribute, avoid repeatedly evaluating the actual
argument reference by saving a pointer to it. This is non-optimal, but
may also be invalid, because the data reference may depend on its own
content. In that case the expression can't be evaluated after the data
has been deallocated.
There are two ways redundant expressions are generated:
- parmse.expr, which contains the actual argument expression, is
reused to get or set subfields in gfc_conv_class_to_class.
- gfc_conv_class_to_class, to get the virtual table pointer associated
with the argument, generates a new expression from scratch starting
with the frontend expression.
The first part is fixed by saving parmse.expr to a pointer and using
the pointer instead of the original expression.
The second part is fixed by adding a separate field to gfc_se that
is set to the class container expression when the expression to
evaluate is polymorphic. This needs the same field in gfc_ss_info
so that its value can be propagated to gfc_conv_class_to_class which
is modified to use that value. Finally gfc_conv_procedure saves the
expression in that field to a pointer in between to avoid the same
problem as for the first part.
PR fortran/92178
gcc/fortran/ChangeLog:
* trans.h (struct gfc_se): New field class_container.
(struct gfc_ss_info): Ditto.
(gfc_evaluate_data_ref_now): New prototype.
* trans.cc (gfc_evaluate_data_ref_now): Implement it.
* trans-array.cc (gfc_conv_ss_descriptor): Copy class_container
field from gfc_se struct to gfc_ss_info struct.
(gfc_conv_expr_descriptor): Copy class_container field from
gfc_ss_info struct to gfc_se struct.
* trans-expr.cc (gfc_conv_class_to_class): Use class container
set in class_container field if available.
(gfc_conv_variable): Set class_container field on encountering
class variables or components, clear it on encountering
non-class components.
(gfc_conv_procedure_call): Evaluate data ref to a pointer now,
and replace later references by usage of the pointer.
gcc/testsuite/ChangeLog:
* gfortran.dg/intent_out_20.f90: New test.
---
gcc/fortran/trans-array.cc | 3 ++
gcc/fortran/trans-expr.cc | 26
gcc/fortran/trans.cc| 28 +
gcc/fortran/trans.h | 6
gcc/testsuite/gfortran.dg/intent_out_20.f90 | 33 +
5 files changed, 96 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/intent_out_20.f90
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index e7c51bae052..1c2af55d436 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -3271,6 +3271,7 @@ gfc_conv_ss_descriptor (stmtblock_t * block, gfc_ss * ss,
int base)
gfc_add_block_to_block (block, &se.pre);
info->descriptor = se.expr;
ss_info->string_length = se.string_length;
+ ss_info->class_container = se.class_container;
if (base)
{
@@ -7687,6 +7688,8 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
else if (deferred_array_component)
se->string_length = ss_info->string_length;
+ se->class_container = ss_info->class_container;
+
gfc_free_ss_chain (ss);
return;
}
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index b7e95e6d04d..5169fbcd974 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -1266,6 +1266,10 @@ gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e,
gfc_typespec class_ts,
slen = build_zero_cst (size_type_node);
}
+ else if (parmse->class_container != NULL_TREE)
+/* Don't redundantly evaluate the expression if the required information
+ is already available. */
+tmp = parmse->class_container;
else
{
/* Remove everything after the last class reference, convert the
@@ -3078,6 +3082,11 @@ gfc_conv_variable (gfc_se * se, gfc_expr * expr)
return;
}
+ if (sym->ts.type == BT_CLASS
+ && sym->attr.class_ok
+ && sym->ts.u.derived->attr.is_class)
+ se->class_container = se->expr;
+
/* Dereference the expression, where needed. */
se->expr = gfc_maybe_dereference_var (sym, se->expr, se->descriptor_only,
is_classarray);
@@ -3135,6 +3144,15 @@ gfc_conv_variable (gfc_se * se, gfc_expr * expr)
conv_parent_component_references (se, ref);
gfc_conv_component_ref (se, ref);
+
+ if (ref->u.c.component->ts.type == BT_CLASS
+ && ref->u.c.component->attr.class_ok
+ && ref->u.c.component->ts.u.derived->attr.is_class)
+ se->class_container = se->expr;
+ else if (!(ref->u.c.sym->attr.flavor == FL_DERIVED
+
[PATCH 3/3] fortran: Reorder array argument evaluation parts [PR92178]
In the case of an array actual arg passed to a polymorphic array dummy
with INTENT(OUT) attribute, reorder the argument evaluation code to
the following:
- first evaluate arguments' values, and data references,
- deallocate data references associated with an allocatable,
intent(out) dummy,
- create a class container using the freed data references.
The ordering used to be incorrect between the first two items,
when one argument was deallocated before a later argument evaluated
its expression depending on the former argument.
r14-2395-gb1079fc88f082d3c5b583c8822c08c5647810259 fixed it by treating
arguments associated with an allocatable, intent(out) dummy in a
separate, later block. This, however, wasn't working either if the data
reference of such an argument was depending on its own content, as
the class container initialization was trying to use deallocated
content.
This change generates class container initialization code in a separate
block, so that it is moved after the deallocation block without moving
the rest of the argument evaluation code.
This alone is not sufficient to fix the problem, because the class
container generation code repeatedly uses the full expression of
the argument at a place where deallocation might have happened
already. This is non-optimal, but may also be invalid, because the data
reference may depend on its own content. In that case the expression
can't be evaluated after the data has been deallocated.
As in the scalar case previously treated, this is fixed by saving
the data reference to a pointer before any deallocation happens,
and then only refering to the pointer. gfc_reset_vptr is updated
to take into account the already evaluated class container if it's
available.
Contrary to the scalar case, one hunk is needed to wrap the parameter
evaluation in a conditional, to avoid regressing in
optional_class_2.f90. This used to be handled by the class wrapper
construction which wrapped the whole code in a conditional. With
this change the class wrapper construction can't see the parameter
evaluation code, so the latter is updated with an additional handling
for optional arguments.
PR fortran/92178
gcc/fortran/ChangeLog:
* trans.h (gfc_reset_vptr): Add class_container argument.
* trans-expr.cc (gfc_reset_vptr): Ditto. If a valid vptr can
be obtained through class_container argument, bypass evaluation
of e.
(gfc_conv_procedure_call): Wrap the argument evaluation code
in a conditional if the associated dummy is optional. Evaluate
the data reference to a pointer now, and replace later
references with usage of the pointer.
gcc/testsuite/ChangeLog:
* gfortran.dg/intent_out_21.f90: New test.
---
gcc/fortran/trans-expr.cc | 86 -
gcc/fortran/trans.h | 2 +-
gcc/testsuite/gfortran.dg/intent_out_21.f90 | 33
3 files changed, 101 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/intent_out_21.f90
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 5169fbcd974..dbb04f8c434 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -529,24 +529,32 @@ gfc_find_and_cut_at_last_class_ref (gfc_expr *e, bool
is_mold,
}
-/* Reset the vptr to the declared type, e.g. after deallocation. */
+/* Reset the vptr to the declared type, e.g. after deallocation.
+ Use the variable in CLASS_CONTAINER if available. Otherwise, recreate
+ one with E. The generated assignment code is added at the end of BLOCK. */
void
-gfc_reset_vptr (stmtblock_t *block, gfc_expr *e)
+gfc_reset_vptr (stmtblock_t *block, gfc_expr *e, tree class_container)
{
- gfc_symbol *vtab;
- tree vptr;
- tree vtable;
- gfc_se se;
+ tree vptr = NULL_TREE;
- /* Evaluate the expression and obtain the vptr from it. */
- gfc_init_se (&se, NULL);
- if (e->rank)
-gfc_conv_expr_descriptor (&se, e);
- else
-gfc_conv_expr (&se, e);
- gfc_add_block_to_block (block, &se.pre);
- vptr = gfc_get_vptr_from_expr (se.expr);
+ if (class_container != NULL_TREE)
+vptr = gfc_get_vptr_from_expr (class_container);
+
+ if (vptr == NULL_TREE)
+{
+ gfc_se se;
+
+ /* Evaluate the expression and obtain the vptr from it. */
+ gfc_init_se (&se, NULL);
+ if (e->rank)
+ gfc_conv_expr_descriptor (&se, e);
+ else
+ gfc_conv_expr (&se, e);
+ gfc_add_block_to_block (block, &se.pre);
+
+ vptr = gfc_get_vptr_from_expr (se.expr);
+}
/* If a vptr is not found, we can do nothing more. */
if (vptr == NULL_TREE)
@@ -556,6 +564,9 @@ gfc_reset_vptr (stmtblock_t *block, gfc_expr *e)
gfc_add_modify (block, vptr, build_int_cst (TREE_TYPE (vptr), 0));
else
{
+ gfc_symbol *vtab;
+ tree vtable;
+
/* Return the vptr to the address of the declared type. */
vtab = gfc_find_derived_vtab (e->ts.u.derived);
[PATCH] fortran: Release symbols in reversed order [PR106050]
Hello,
I saw the light regarding this PR after Paul posted a comment yesterday.
Regression test in progress on x86_64-pc-linux-gnu.
I plan to push in the next hours.
Mikael
-- >8 --
Release symbols in reversed order wrt the order they were allocated.
This fixes an error recovery ICE in the case of a misplaced
derived type declaration. Such a declaration creates nested
symbols, one for the derived type and one for each type parameter,
which should be immediately released as the declaration is
rejected. This breaks if the derived type is released first.
As the type parameter symbols are in the namespace of the derived
type, releasing the derived type releases the type parameters, so
one can't access them after that, even to release them. Hence,
the type parameters should be released first.
PR fortran/106050
gcc/fortran/ChangeLog:
* symbol.cc (gfc_restore_last_undo_checkpoint): Release symbols
in reverse order.
gcc/testsuite/ChangeLog:
* gfortran.dg/pdt_33.f90: New test.
---
gcc/fortran/symbol.cc| 2 +-
gcc/testsuite/gfortran.dg/pdt_33.f90 | 15 +++
2 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/pdt_33.f90
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 37a9e8fa0ae..4a71d84b3fe 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3661,7 +3661,7 @@ gfc_restore_last_undo_checkpoint (void)
gfc_symbol *p;
unsigned i;
- FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
+ FOR_EACH_VEC_ELT_REVERSE (latest_undo_chgset->syms, i, p)
{
/* Symbol in a common block was new. Or was old and just put in common */
if (p->common_block
diff --git a/gcc/testsuite/gfortran.dg/pdt_33.f90
b/gcc/testsuite/gfortran.dg/pdt_33.f90
new file mode 100644
index 000..0521513f2f8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pdt_33.f90
@@ -0,0 +1,15 @@
+! { dg-do compile }
+!
+! PR fortran/106050
+! The following used to trigger an error recovery ICE by releasing
+! the symbol T before the symbol K which was leading to releasing
+! K twice as it's in T's namespace.
+!
+! Contributed by G. Steinmetz
+
+program p
+ a = 1
+ type t(k) ! { dg-error "Unexpected derived type
declaration" }
+ integer, kind :: k = 4 ! { dg-error "not allowed outside a TYPE
definition" }
+ end type ! { dg-error "Expecting END PROGRAM" }
+end
--
2.40.1
Re: [PATCH] fortran: Release symbols in reversed order [PR106050]
Hi Mikhail,
That's more than OK by me.
Thanks for attacking this PR.
I have a couple more of Steve's orphans waiting to be packaged up -
91960 and 104649. I'll submit them this evening.100607 is closed-fixed
and 103796 seems to be fixed.
Regards
Paul
On Tue, 11 Jul 2023 at 13:08, Mikael Morin via Fortran
wrote:
>
> Hello,
>
> I saw the light regarding this PR after Paul posted a comment yesterday.
>
> Regression test in progress on x86_64-pc-linux-gnu.
> I plan to push in the next hours.
>
> Mikael
>
> -- >8 --
>
> Release symbols in reversed order wrt the order they were allocated.
> This fixes an error recovery ICE in the case of a misplaced
> derived type declaration. Such a declaration creates nested
> symbols, one for the derived type and one for each type parameter,
> which should be immediately released as the declaration is
> rejected. This breaks if the derived type is released first.
> As the type parameter symbols are in the namespace of the derived
> type, releasing the derived type releases the type parameters, so
> one can't access them after that, even to release them. Hence,
> the type parameters should be released first.
>
> PR fortran/106050
>
> gcc/fortran/ChangeLog:
>
> * symbol.cc (gfc_restore_last_undo_checkpoint): Release symbols
> in reverse order.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/pdt_33.f90: New test.
> ---
> gcc/fortran/symbol.cc| 2 +-
> gcc/testsuite/gfortran.dg/pdt_33.f90 | 15 +++
> 2 files changed, 16 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gfortran.dg/pdt_33.f90
>
> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> index 37a9e8fa0ae..4a71d84b3fe 100644
> --- a/gcc/fortran/symbol.cc
> +++ b/gcc/fortran/symbol.cc
> @@ -3661,7 +3661,7 @@ gfc_restore_last_undo_checkpoint (void)
>gfc_symbol *p;
>unsigned i;
>
> - FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
> + FOR_EACH_VEC_ELT_REVERSE (latest_undo_chgset->syms, i, p)
> {
>/* Symbol in a common block was new. Or was old and just put in common
> */
>if (p->common_block
> diff --git a/gcc/testsuite/gfortran.dg/pdt_33.f90
> b/gcc/testsuite/gfortran.dg/pdt_33.f90
> new file mode 100644
> index 000..0521513f2f8
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pdt_33.f90
> @@ -0,0 +1,15 @@
> +! { dg-do compile }
> +!
> +! PR fortran/106050
> +! The following used to trigger an error recovery ICE by releasing
> +! the symbol T before the symbol K which was leading to releasing
> +! K twice as it's in T's namespace.
> +!
> +! Contributed by G. Steinmetz
> +
> +program p
> + a = 1
> + type t(k) ! { dg-error "Unexpected derived type
> declaration" }
> + integer, kind :: k = 4 ! { dg-error "not allowed outside a TYPE
> definition" }
> + end type ! { dg-error "Expecting END PROGRAM" }
> +end
> --
> 2.40.1
>
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Harald, attached is a new version of the patch. This now also respects inquiry-LEN. Btw, there is a potential memory leak in the simplify for inquiry functions. I have added a note into the code. I tried to use a pdt within a derived type as a component. Is that not allowed by the standard? I know, I could hunt in the standard for it, but when someone knows out of his head, he could greatly help me out. Regtests ok on x86_64-linux-gnu/F37. Regards, Andre On Mon, 10 Jul 2023 20:55:29 +0200 Harald Anlauf wrote: > Hi Andre, > > thanks for looking into this! > > While it fixes the original PR, here is a minor extension of the > testcase that ICEs here with your patch: > > program pr102003 >type pdt(n) > integer, len :: n = 8 > character(len=n) :: c >end type pdt >type(pdt(42)) :: p >integer, parameter :: m = len (p% c) >integer, parameter :: n = p% c% len > >if (m /= 42) stop 1 >if (len (p% c) /= 42) stop 2 >print *, p% c% len ! OK >if (p% c% len /= 42) stop 3 ! OK >print *, n ! ICE > end > > I get: > > pdt_33.f03:14:27: > > 14 | integer, parameter :: n = p% c% len >| 1 > Error: non-constant initialization expression at (1) > pdt_33.f03:20:31: > > 20 | print *, n ! ICE >| 1 > internal compiler error: tree check: expected record_type or union_type > or qual_union_type, have integer_type in gfc_conv_component_ref, at > fortran/trans-expr.cc:2757 > 0x84286c tree_check_failed(tree_node const*, char const*, int, char > const*, ...) > ../../gcc-trunk/gcc/tree.cc:8899 > 0xa6d6fb tree_check3(tree_node*, char const*, int, char const*, > tree_code, tree_code, tree_code) > ../../gcc-trunk/gcc/tree.h:3617 > 0xa90847 gfc_conv_component_ref(gfc_se*, gfc_ref*) > ../../gcc-trunk/gcc/fortran/trans-expr.cc:2757 > 0xa91bbc gfc_conv_variable > ../../gcc-trunk/gcc/fortran/trans-expr.cc:3137 > 0xaa8e9c gfc_conv_expr(gfc_se*, gfc_expr*) > ../../gcc-trunk/gcc/fortran/trans-expr.cc:9594 > 0xaa92ae gfc_conv_expr_reference(gfc_se*, gfc_expr*) > ../../gcc-trunk/gcc/fortran/trans-expr.cc:9713 > 0xad67f6 gfc_trans_transfer(gfc_code*) > ../../gcc-trunk/gcc/fortran/trans-io.cc:2607 > 0xa43cb7 trans_code > ../../gcc-trunk/gcc/fortran/trans.cc:2449 > 0xad37c6 build_dt > ../../gcc-trunk/gcc/fortran/trans-io.cc:2051 > 0xa43cd7 trans_code > ../../gcc-trunk/gcc/fortran/trans.cc:2421 > 0xa84711 gfc_generate_function_code(gfc_namespace*) > ../../gcc-trunk/gcc/fortran/trans-decl.cc:7762 > 0x9d9ca7 translate_all_program_units > ../../gcc-trunk/gcc/fortran/parse.cc:6929 > 0x9d9ca7 gfc_parse_file() > ../../gcc-trunk/gcc/fortran/parse.cc:7235 > 0xa40a1f gfc_be_parse_file > ../../gcc-trunk/gcc/fortran/f95-lang.cc:229 > > The fortran-dump confirms that n is not simplified to a constant. > So while you're at it, do you also see a solution to this variant? > > Harald > > > Am 10.07.23 um 17:48 schrieb Andre Vehreschild via Gcc-patches: > > Hi all, > > > > while browsing the pdt meta-bug I came across 102003 and thought to myself: > > Well, that one is easy. How foolish of me... > > > > Anyway, the solution attached prevents a pdt_len (or pdt_kind) expression > > in a function call (e.g. len() or kind()) to mark the whole expression as a > > pdt one. The second part of the patch in simplify.cc then takes care of > > either generating the correct component ref or when a constant expression > > (i.e. gfc_init_expr_flag is set) is required to look this up from the > > actual symbol (not from the type, because there the default value is > > stored). > > > > Regtested ok on x86_64-linux-gnu/Fedora 37. > > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: * expr.cc (find_inquiry_ref): Replace len of pdt_string by constant. (gfc_match_init_expr): Prevent PDT analysis for function calls. (gfc_pdt_find_component_copy_initializer): Get the initializer value for given component. * gfortran.h (gfc_pdt_find_component_copy_initializer): New function. * simplify.cc (gfc_simplify_len): Replace len() of PDT with pdt component ref or constant. gcc/testsuite/ChangeLog: * gfortran.dg/pdt_33.f03: New test. diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index e418f1f3301..23324517ff2 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -1862,6 +1862,13 @@ find_inquiry_ref (gfc_expr *p, gfc_expr **newp) else if (tmp->expr_type == EXPR_CONSTANT) *newp = gfc_get_int_expr (gfc_default_integer_kind, NULL, tmp->value.character.length); + else if (gfc_init_expr_flag + && tmp->ts.u.cl->length->symtree->n.sym->attr.pdt_len) +
[PATCH] Fortran: formal symbol attributes for intrinsic procedures [PR110288]
Dear all,
for intrinsic procedures we derive the typespec of the formal symbol
attributes from the actual arguments. This can have an undesired
effect for character actual arguments, as the argument passing
conventions differ for deferred-length (length is passed by reference)
and otherwise (length is passed by value).
The testcase in the PR nicely demonstrates the issue for
FINDLOC(array,value,...), when either array or value are deferred-length.
We therefore need take care that we do not copy ts.deferred, but
rather set it to false if the formal argument is neither allocatable
or pointer.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
This is actually a 11/12/13/14 regression (and I found a potential
"culprit" in 11-development that touched the call chain in question),
so the patch might finally need backporting as far as seems reasonable.
Thanks,
Harald
From 3b2c523ae31b68fc3b8363b458a55eec53a44365 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Tue, 11 Jul 2023 21:21:25 +0200
Subject: [PATCH] Fortran: formal symbol attributes for intrinsic procedures
[PR110288]
gcc/fortran/ChangeLog:
PR fortran/110288
* symbol.cc (gfc_copy_formal_args_intr): When deriving the formal
argument attributes from the actual ones for intrinsic procedure
calls, take special care of CHARACTER arguments that we do not
wrongly treat them formally as deferred-length.
gcc/testsuite/ChangeLog:
PR fortran/110288
* gfortran.dg/findloc_10.f90: New test.
---
gcc/fortran/symbol.cc| 7 +++
gcc/testsuite/gfortran.dg/findloc_10.f90 | 13 +
2 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/findloc_10.f90
diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 37a9e8fa0ae..90023f0ad73 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -4725,6 +4725,13 @@ gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src,
formal_arg->sym->attr.flavor = FL_VARIABLE;
formal_arg->sym->attr.dummy = 1;
+ /* Do not treat an actual deferred-length character argument wrongly
+ as template for the formal argument. */
+ if (formal_arg->sym->ts.type == BT_CHARACTER
+ && !(formal_arg->sym->attr.allocatable
+ || formal_arg->sym->attr.pointer))
+ formal_arg->sym->ts.deferred = false;
+
if (formal_arg->sym->ts.type == BT_CHARACTER)
formal_arg->sym->ts.u.cl = gfc_new_charlen (gfc_current_ns, NULL);
diff --git a/gcc/testsuite/gfortran.dg/findloc_10.f90 b/gcc/testsuite/gfortran.dg/findloc_10.f90
new file mode 100644
index 000..4d5ecd2306a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/findloc_10.f90
@@ -0,0 +1,13 @@
+! { dg-do run }
+! { dg-options "-fdump-tree-original" }
+! PR fortran/110288 - FINDLOC and deferred-length character arguments
+
+program test
+ character(len=:), allocatable :: array(:)
+ character(len=:), allocatable :: value
+ array = ["bb", "aa"]
+ value = "aa"
+ if (findloc (array, value, dim=1) /= 2) stop 1
+end program test
+
+! { dg-final { scan-tree-dump "_gfortran_findloc2_s1 \\(.*, \\.array, \\.value\\)" "original" } }
--
2.35.3
Re: [PATCH] Fortran: formal symbol attributes for intrinsic procedures [PR110288]
On Tue, Jul 11, 2023 at 09:39:31PM +0200, Harald Anlauf via Fortran wrote: > Dear all, > > for intrinsic procedures we derive the typespec of the formal symbol > attributes from the actual arguments. This can have an undesired > effect for character actual arguments, as the argument passing > conventions differ for deferred-length (length is passed by reference) > and otherwise (length is passed by value). > > The testcase in the PR nicely demonstrates the issue for > FINDLOC(array,value,...), when either array or value are deferred-length. > > We therefore need take care that we do not copy ts.deferred, but > rather set it to false if the formal argument is neither allocatable > or pointer. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > > This is actually a 11/12/13/14 regression (and I found a potential > "culprit" in 11-development that touched the call chain in question), > so the patch might finally need backporting as far as seems reasonable. > OK. Backports are OK as well. -- Steve
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Andre,
this looks much better now!
This looks mostly good to me, except for a typo in the testcase:
+ if (p% ci% len /= 42) stop 4
There is no component "ci", only "c". The testsuite would fail.
Regarding the memleak: replacing
// TODO: Fix leaking expr tmp, when simplify is done twice.
tmp = gfc_copy_expr (*newp);
by
if (inquiry->next)
{
gfc_free_expr (tmp);
tmp = gfc_copy_expr (*newp);
}
or rather
if (inquiry->next)
gfc_replace_expr (tmp, *newp);
at least shrinks the leak a bit. (Untested otherwise).
OK with one of the above changes (provided it survives regtesting).
Thanks for the patch!
Harald
Am 11.07.23 um 18:23 schrieb Andre Vehreschild via Gcc-patches:
Hi Harald,
attached is a new version of the patch. This now also respects inquiry-LEN.
Btw, there is a potential memory leak in the simplify for inquiry functions. I
have added a note into the code.
I tried to use a pdt within a derived type as a component. Is that not allowed
by the standard? I know, I could hunt in the standard for it, but when someone
knows out of his head, he could greatly help me out.
Regtests ok on x86_64-linux-gnu/F37.
Regards,
Andre
On Mon, 10 Jul 2023 20:55:29 +0200
Harald Anlauf wrote:
Hi Andre,
thanks for looking into this!
While it fixes the original PR, here is a minor extension of the
testcase that ICEs here with your patch:
program pr102003
type pdt(n)
integer, len :: n = 8
character(len=n) :: c
end type pdt
type(pdt(42)) :: p
integer, parameter :: m = len (p% c)
integer, parameter :: n = p% c% len
if (m /= 42) stop 1
if (len (p% c) /= 42) stop 2
print *, p% c% len ! OK
if (p% c% len /= 42) stop 3 ! OK
print *, n ! ICE
end
I get:
pdt_33.f03:14:27:
14 | integer, parameter :: n = p% c% len
| 1
Error: non-constant initialization expression at (1)
pdt_33.f03:20:31:
20 | print *, n ! ICE
| 1
internal compiler error: tree check: expected record_type or union_type
or qual_union_type, have integer_type in gfc_conv_component_ref, at
fortran/trans-expr.cc:2757
0x84286c tree_check_failed(tree_node const*, char const*, int, char
const*, ...)
../../gcc-trunk/gcc/tree.cc:8899
0xa6d6fb tree_check3(tree_node*, char const*, int, char const*,
tree_code, tree_code, tree_code)
../../gcc-trunk/gcc/tree.h:3617
0xa90847 gfc_conv_component_ref(gfc_se*, gfc_ref*)
../../gcc-trunk/gcc/fortran/trans-expr.cc:2757
0xa91bbc gfc_conv_variable
../../gcc-trunk/gcc/fortran/trans-expr.cc:3137
0xaa8e9c gfc_conv_expr(gfc_se*, gfc_expr*)
../../gcc-trunk/gcc/fortran/trans-expr.cc:9594
0xaa92ae gfc_conv_expr_reference(gfc_se*, gfc_expr*)
../../gcc-trunk/gcc/fortran/trans-expr.cc:9713
0xad67f6 gfc_trans_transfer(gfc_code*)
../../gcc-trunk/gcc/fortran/trans-io.cc:2607
0xa43cb7 trans_code
../../gcc-trunk/gcc/fortran/trans.cc:2449
0xad37c6 build_dt
../../gcc-trunk/gcc/fortran/trans-io.cc:2051
0xa43cd7 trans_code
../../gcc-trunk/gcc/fortran/trans.cc:2421
0xa84711 gfc_generate_function_code(gfc_namespace*)
../../gcc-trunk/gcc/fortran/trans-decl.cc:7762
0x9d9ca7 translate_all_program_units
../../gcc-trunk/gcc/fortran/parse.cc:6929
0x9d9ca7 gfc_parse_file()
../../gcc-trunk/gcc/fortran/parse.cc:7235
0xa40a1f gfc_be_parse_file
../../gcc-trunk/gcc/fortran/f95-lang.cc:229
The fortran-dump confirms that n is not simplified to a constant.
So while you're at it, do you also see a solution to this variant?
Harald
Am 10.07.23 um 17:48 schrieb Andre Vehreschild via Gcc-patches:
Hi all,
while browsing the pdt meta-bug I came across 102003 and thought to myself:
Well, that one is easy. How foolish of me...
Anyway, the solution attached prevents a pdt_len (or pdt_kind) expression
in a function call (e.g. len() or kind()) to mark the whole expression as a
pdt one. The second part of the patch in simplify.cc then takes care of
either generating the correct component ref or when a constant expression
(i.e. gfc_init_expr_flag is set) is required to look this up from the
actual symbol (not from the type, because there the default value is
stored).
Regtested ok on x86_64-linux-gnu/Fedora 37.
Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de
--
Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Patch, Fortran] Allow ref'ing PDT's len() in parameter-initializer [PR102003]
Hi Andre, I forgot to answer your other question: Am 11.07.23 um 18:23 schrieb Andre Vehreschild via Gcc-patches: I tried to use a pdt within a derived type as a component. Is that not allowed by the standard? I know, I could hunt in the standard for it, but when someone knows out of his head, he could greatly help me out. You mean something like the following is rejected with a strange error: type pdt(n) integer, len :: n = 8 character(len=n) :: c end type pdt type t type(pdt(42)) :: q end type t type(t) :: u end pr102003.f90:1:10: 1 | type pdt(n) | 1 Error: Cannot convert TYPE(Pdtpdt) to TYPE(pdt) at (1) ISTR that there is an existing PR...
