fortran: Separate associate character lengths earlier [PR104570]

Message ID 4cce238d-5fdc-4a94-908b-9a1399193493@orange.fr
State New
Headers
Series fortran: Separate associate character lengths earlier [PR104570] |

Commit Message

Mikael Morin March 14, 2022, 6:28 p.m. UTC
  Hello,

this workarounds the regression I introduced with the fix for pr104228.
The problem comes from the evaluation of character length for the 
associate target (y) in the testcase.
The expression is non-scalar which is not supported at that point, as no 
scalarization setup is made there.

My initial direction to fix this was trying to modify the evaluation of 
expressions so that the rank was ignored in the context of character 
length evaluation.
That’s the patch I attached to the PR.
The patch I’m proposing here tries instead to avoid the need to evaluate 
character length through sharing of gfc_charlen between symbols and 
expressions.
I slightly prefer the latter direction, though I’m not sure it’s more 
robust.

At least it passes regression testing, which proves some basic level of 
goodness.
OK for master?
Even if PR104570 is a 12 regression only, the PR104228 fix is targeted 
at all the open branches so this one as well.
OK for 11/10/9?
  

Comments

Harald Anlauf March 16, 2022, 9:24 p.m. UTC | #1
Hi Mikael,

Am 14.03.22 um 19:28 schrieb Mikael Morin:
> Hello,
>
> this workarounds the regression I introduced with the fix for pr104228.
> The problem comes from the evaluation of character length for the
> associate target (y) in the testcase.
> The expression is non-scalar which is not supported at that point, as no
> scalarization setup is made there.
>
> My initial direction to fix this was trying to modify the evaluation of
> expressions so that the rank was ignored in the context of character
> length evaluation.
> That’s the patch I attached to the PR.
> The patch I’m proposing here tries instead to avoid the need to evaluate
> character length through sharing of gfc_charlen between symbols and
> expressions.
> I slightly prefer the latter direction, though I’m not sure it’s more
> robust. > At least it passes regression testing, which proves some basic level of
> goodness.
> OK for master?

I tried a few minor variations of the testcase, and they
also seemed to work.  The patch looks simple enough, so
OK from my side.

> Even if PR104570 is a 12 regression only, the PR104228 fix is targeted
> at all the open branches so this one as well.
> OK for 11/10/9?

Yes, after giving it a short time on master to "burn in".

Thanks for the patch!

Harald
  

Patch

From af11a90c730a57be86b94331f31a611b31276b83 Mon Sep 17 00:00:00 2001
From: Mikael Morin <mikael@gcc.gnu.org>
Date: Sun, 13 Mar 2022 22:22:55 +0100
Subject: [PATCH] fortran: Separate associate character lengths earlier
 [PR104570]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change workarounds an ICE in the evaluation of the character length of an
array expression referencing an associate variable; the code is not prepared
to see a non-scalar expression as it doesn’t initialize the scalarizer.

Before this change, associate length symbols get a new gfc_charlen at resolution
stage to unshare them from the associate expression, so that at translation
stage it is a decl specific to the associate symbol that is initialized, not the
decl of some other symbol.  This reinitialization of gfc_charlen happens after
expressions referencing the associate symbol have been parsed, so that those
expressions retain the original gfc_charlen they have copied from the symbol.
At translation stage, the gfc_charlen for the associate symbol is setup with the
decl holding the actual length value, but the expressions have retained the
original gfc_charlen without any decl.  So they need to evaluate the character
length, and this is where the ICE happens.

This change moves the reinitialization of gfc_charlen earlier at parsing stage,
so that at resolution stage the gfc_charlen can be retained as it’s already not
shared with any other symbol, and the expressions which now share their
gfc_charlen with the symbol are automatically updated when the length decl is
setup at translation stage.  There is no need any more to evaluate the character
length as it has all the required information, and the ICE doesn’t happen.

The first resolve.cc hunk is necessary to avoid regressing on the
associate_35.f90 testcase.

	PR fortran/104228
	PR fortran/104570

gcc/fortran/ChangeLog:

	* parse.cc (parse_associate): Use a new distinct gfc_charlen if the
	copied type has one whose length is not known to be constant.
	* resolve.cc (resolve_assoc_var): Reset charlen if it’s shared with
	the associate target regardless of the expression type.
	Don’t reinitialize charlen if it’s deferred.

gcc/testsuite/ChangeLog:

	* gfortran.dg/associate_58.f90: New test.
---
 gcc/fortran/parse.cc                       | 18 ++++++++++++++++++
 gcc/fortran/resolve.cc                     |  9 ++++++---
 gcc/testsuite/gfortran.dg/associate_58.f90 | 21 +++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/associate_58.f90

diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index db918291b10..e6e915d2a5e 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -4924,6 +4924,24 @@  parse_associate (void)
 	 in case of association to a derived-type.  */
       sym->ts = a->target->ts;
 
+      /* Don’t share the character length information between associate
+	 variable and target if the length is not a compile-time constant,
+	 as we don’t want to touch some other character length variable when
+	 we try to initialize the associate variable’s character length
+	 variable.
+	 We do it here rather than later so that expressions referencing the
+	 associate variable will automatically have the correctly setup length
+	 information.  If we did it at resolution stage the expressions would
+	 use the original length information, and the variable a new different
+	 one, but only the latter one would be correctly initialized at
+	 translation stage, and the former one would need some additional setup
+	 there.  */
+      if (sym->ts.type == BT_CHARACTER
+	  && sym->ts.u.cl
+	  && !(sym->ts.u.cl->length
+	       && sym->ts.u.cl->length->expr_type == EXPR_CONSTANT))
+	sym->ts.u.cl = gfc_new_charlen (gfc_current_ns, NULL);
+
       /* Check if the target expression is array valued.  This cannot always
 	 be done by looking at target.rank, because that might not have been
 	 set yet.  Therefore traverse the chain of refs, looking for the last
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 266e41e25b1..3f59448647e 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -9226,7 +9226,7 @@  resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
       if (!sym->ts.u.cl)
 	sym->ts.u.cl = target->ts.u.cl;
 
-      if (sym->ts.deferred && target->expr_type == EXPR_VARIABLE
+      if (sym->ts.deferred
 	  && sym->ts.u.cl == target->ts.u.cl)
 	{
 	  sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
@@ -9245,8 +9245,11 @@  resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
 		|| sym->ts.u.cl->length->expr_type != EXPR_CONSTANT)
 		&& target->expr_type != EXPR_VARIABLE)
 	{
-	  sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
-	  sym->ts.deferred = 1;
+	  if (!sym->ts.deferred)
+	    {
+	      sym->ts.u.cl = gfc_new_charlen (sym->ns, NULL);
+	      sym->ts.deferred = 1;
+	    }
 
 	  /* This is reset in trans-stmt.cc after the assignment
 	     of the target expression to the associate name.  */
diff --git a/gcc/testsuite/gfortran.dg/associate_58.f90 b/gcc/testsuite/gfortran.dg/associate_58.f90
new file mode 100644
index 00000000000..9c24f35c0d8
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associate_58.f90
@@ -0,0 +1,21 @@ 
+! { dg-do compile }
+!
+! PR fortran/104570
+! The following used to cause an ICE because the string length
+! evaluation of the (y) expression was not prepared to handle
+! a non-scalar expression.
+
+program p
+   character(:), allocatable :: x(:)
+   x = ['abc']
+   call s
+contains
+   subroutine s
+      associate (y => x)
+         associate (z => (y))
+            print *, z
+         end associate
+      end associate
+   end
+end
+
-- 
2.35.1