Hi Paul,
this looks good to me and is OK for mainline. When it has survived a
week or two, backporting at least to 14-branch (ideally before 14.2
release) would be a good thing!
Regarding the following excerpt of the testcase:
+! Commented out lines give implicit type warnings with gfortran and nagfor
+! character(len = len (d)) :: line4 (len (line3))
+ character(len = len (line3)) :: line4 (len (line3))
+! character(len = size(len4, 1)) :: line5
I guess the last commented line should have referred to size(line4, 1),
right? The lexical distance of len4 and line4 can be small after
long coding...
The first commented line gives no warning here, but is simply
inconsistent with a test later on, since len (d) < len (line3).
What exactly was the issue?
***
A minor nit: while you were fixing whitespace issues in the source,
you missed an indent with spaces here:
@@ -857,12 +873,26 @@ gfc_defer_symbol_init (gfc_symbol * sym)
/* Find the first dummy arg seen after us, or the first
non-dummy arg.
This is a circular list, so don't go past the head. */
while (p != head
- && (!p->attr.dummy || p->dummy_order > sym->dummy_order))
+ && (!p->attr.dummy || decl_order (p, sym)))
{
At least on my side there is no tab...
(It is fine in a similar code later on.)
***
Finally a big thanks for the patch!
Harald
Am 13.06.24 um 23:43 schrieb Paul Richard Thomas:
Hi Both,
Thanks for the highly constructive comments. I think that I have
incorporated them fully in the attached.
OK for mainline and ...?
Paul
On Mon, 10 Jun 2024 at 08:19, Andre Vehreschild <ve...@gmx.de> wrote:
Hi Paul,
while looking at your patch I see calls to gfc_add_init_cleanup (...,
back),
while the function signature is gfc_add_init_cleanup (..., bool front).
This
slightly confuses me. I would at least expect to see
gfc_add_init_cleanup(...,
!back) calls. Just to get the semantics right.
Then I wonder why not doing:
diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index bafe8cbc5bc..97ace8c778e 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -2497,3 +2497,63 @@ gfc_omp_expr_prefix_same (gfc_expr *lexpr, gfc_expr
*rexpr)
return true;
}
+
+
+/* gfc_function_dependency returns true for non-dummy symbols with
dependencies
+ on an old-fashioned function result (ie. proc_name =
proc_name->result).
+ This is used to ensure that initialization code appears after the
function
+ result is treated and that any mutual dependencies between these
symbols are
+ respected. */
+
+static bool
+dependency_fcn (gfc_expr *e, gfc_symbol *sym,
+ int *f ATTRIBUTE_UNUSED)
+{
+ return (e && e->expr_type == EXPR_VARIABLE
+ && e->symtree
+ && e->symtree->n.sym == sym);
+}
Instead of the multiple if-statements?
+
+bool
+gfc_function_dependency (gfc_symbol *sym, gfc_symbol *proc_name)
+{
+ bool front = false;
+
+ if (proc_name && proc_name->attr.function
+ && proc_name == proc_name->result
+ && !(sym->attr.dummy || sym->attr.result))
+ {
+ if (sym->as && sym->as->type == AS_EXPLICIT)
+ {
+ for (int dim = 0; dim < sym->as->rank; dim++)
+ {
+ if (sym->as->lower[dim]
+ && sym->as->lower[dim]->expr_type != EXPR_CONSTANT)
+ front = gfc_traverse_expr (sym->as->lower[dim], proc_name,
+ dependency_fcn, 0);
+ if (front)
+ break;
+ if (sym->as->upper[dim]
+ && sym->as->upper[dim]->expr_type != EXPR_CONSTANT)
+ front = gfc_traverse_expr (sym->as->upper[dim], proc_name,
+ dependency_fcn, 0);
+ if (front)
+ break;
+ }
+ }
+
+ if (sym->ts.type == BT_CHARACTER
+ && sym->ts.u.cl && sym->ts.u.cl->length
+ && sym->ts.u.cl->length->expr_type != EXPR_CONSTANT)
+ front = gfc_traverse_expr (sym->ts.u.cl->length, proc_name,
+ dependency_fcn, 0);
This can overwrite a previous front == true, right? Is this intended?
+ }
+ return front;
+ }
The rest - besides the front-back confusion - looks fine to me. Thanks for
the
patch.
Regards,
Andre
On Sun, 9 Jun 2024 07:14:39 +0100
Paul Richard Thomas <paul.richard.tho...@gmail.com> wrote:
Hi All,
The attached fixes a problem that, judging by the comments, has been
looked
at periodically over the last ten years but just looked to be too
fiendishly complicated to fix. This is not in small part because of the
confusing ordering of dummies in the tlink chain and the unintuitive
placement of all deferred initializations to the front of the init chain
in
the wrapped block.
The result of the existing ordering is that the initialization code for
non-dummy variables that depends on the function result occurs before any
initialization code for the function result itself. The fix ensures that:
(i) These variables are placed correctly in the tlink chain, respecting
inter-dependencies; and (ii) The dependent initializations are placed at
the end of the wrapped block init chain. The details appear in the
comments in the patch. It is entirely possible that a less clunky fix
exists but I failed to find it.
OK for mainline?
Regards
Paul
--
Andre Vehreschild * Email: vehre ad gmx dot de
