[PATCH] fortran: Compare non-constant bound expressions. [PR105379]

[PATCH] fortran: Compare non-constant bound expressions. [PR105379]

[Mikael Morin]

[-- Attachment #1: Type: text/plain, Size: 125 bytes --]

Hello,

this fixes a regression caused by my recent PR103662 patch.

Regression tested on x86_64-pc-linux-gnu. OK for master?

[-- Attachment #2: 0001-fortran-Compare-non-constant-bound-expressions.-PR10.patch --]
[-- Type: text/x-patch, Size: 5034 bytes --]

From d7309a471c42e51e84c37d5d4a3fd5bb0ed67405 Mon Sep 17 00:00:00 2001
From: Mikael Morin <mikael@gcc.gnu.org>
Date: Mon, 25 Apr 2022 19:47:04 +0200
Subject: [PATCH] fortran: Compare non-constant bound expressions. [PR105379]

Starting with r12-8235-gfa5cd7102da676dcb1757b1288421f5f3439ae0e,
class descriptor types are compared to detect duplicate declarations.

This caused ICEs as the comparison of array spec supported only constant
explicit bounds, but dummy class variable descriptor types can have a
_data field with non-constant array spec bounds.

This change adds support for non-constant bounds.  For that,
gfc_dep_compare_expr is used.  It does probably more than strictly
necessary, but using it avoids rewriting a specific comparison function,
making mistakes and forgetting cases.

	PR fortran/103662
	PR fortran/105379

gcc/fortran/ChangeLog:

	* array.cc (compare_bounds): Use bool as return type.
	Support non-constant expressions.
	(gfc_compare_array_spec): Update call to compare_bounds.

gcc/testsuite/ChangeLog:

	* gfortran.dg/class_dummy_8.f90: New test.
	* gfortran.dg/class_dummy_9.f90: New test.
---
 gcc/fortran/array.cc                        | 27 ++++++++++++---------
 gcc/testsuite/gfortran.dg/class_dummy_8.f90 | 20 +++++++++++++++
 gcc/testsuite/gfortran.dg/class_dummy_9.f90 | 20 +++++++++++++++
 3 files changed, 56 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/class_dummy_8.f90
 create mode 100644 gcc/testsuite/gfortran.dg/class_dummy_9.f90

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index 90ea812d699..bbdb5b392fc 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -957,23 +957,28 @@ gfc_copy_array_spec (gfc_array_spec *src)
 }
 
 
-/* Returns nonzero if the two expressions are equal.  Only handles integer
-   constants.  */
+/* Returns nonzero if the two expressions are equal.
+   We should not need to support more than constant values, as that’s what is
+   allowed in derived type component array spec.  However, we may create types
+   with non-constant array spec for dummy variable class container types, for
+   which the _data component holds the array spec of the variable declaration.
+   So we have to support non-constant bounds as well.  */
 
-static int
+static bool
 compare_bounds (gfc_expr *bound1, gfc_expr *bound2)
 {
   if (bound1 == NULL || bound2 == NULL
-      || bound1->expr_type != EXPR_CONSTANT
-      || bound2->expr_type != EXPR_CONSTANT
       || bound1->ts.type != BT_INTEGER
       || bound2->ts.type != BT_INTEGER)
     gfc_internal_error ("gfc_compare_array_spec(): Array spec clobbered");
 
-  if (mpz_cmp (bound1->value.integer, bound2->value.integer) == 0)
-    return 1;
-  else
-    return 0;
+  /* What qualifies as identical bounds?  We could probably just check that the
+     expressions are exact clones.  We avoid rewriting a specific comparison
+     function and re-use instead the rather involved gfc_dep_compare_expr which
+     is just a bit more permissive, as it can also detect identical values for
+     some mismatching expressions (extra parenthesis, swapped operands, unary
+     plus, etc).  It probably only makes a difference in corner cases.  */
+  return gfc_dep_compare_expr (bound1, bound2) == 0;
 }
 
 
@@ -1006,10 +1011,10 @@ gfc_compare_array_spec (gfc_array_spec *as1, gfc_array_spec *as2)
   if (as1->type == AS_EXPLICIT)
     for (i = 0; i < as1->rank + as1->corank; i++)
       {
-	if (compare_bounds (as1->lower[i], as2->lower[i]) == 0)
+	if (!compare_bounds (as1->lower[i], as2->lower[i]))
 	  return 0;
 
-	if (compare_bounds (as1->upper[i], as2->upper[i]) == 0)
+	if (!compare_bounds (as1->upper[i], as2->upper[i]))
 	  return 0;
       }
 
diff --git a/gcc/testsuite/gfortran.dg/class_dummy_8.f90 b/gcc/testsuite/gfortran.dg/class_dummy_8.f90
new file mode 100644
index 00000000000..0976a725866
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/class_dummy_8.f90
@@ -0,0 +1,20 @@
+! { dg-do compile }
+!
+! PR fortran/105379
+! Type comparison of class containers used to trigger an ICE when one of the
+! class containers had a non-constant array spec.
+!
+! Contributed by Gerhard Steinmetz <gscfq@t-online.de>.
+
+program p
+   type t
+   end type
+contains
+   subroutine s1(x)
+      class(t) :: x(3)
+   end
+   subroutine s2(n, x)
+      integer :: n
+      class(t) :: x(n)
+   end
+end
diff --git a/gcc/testsuite/gfortran.dg/class_dummy_9.f90 b/gcc/testsuite/gfortran.dg/class_dummy_9.f90
new file mode 100644
index 00000000000..0fd98c05be2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/class_dummy_9.f90
@@ -0,0 +1,20 @@
+! { dg-do compile }
+!
+! PR fortran/105379
+! Type comparison of class containers used to trigger an ICE when one of the
+! class containers had a non-constant array spec.
+!
+! Contributed by Gerhard Steinmetz <gscfq@t-online.de>.
+
+program p
+   type t
+   end type
+   integer :: m = 3
+contains
+   subroutine s1(x)
+      class(t) :: x(3)
+   end
+   subroutine s3(x)
+      class(t) :: x(m)
+   end
+end
-- 
2.35.1

Re: [PATCH] fortran: Compare non-constant bound expressions. [PR105379]

[Thomas Koenig]

Hi Mikael,

> this fixes a regression caused by my recent PR103662 patch.
> 
> Regression tested on x86_64-pc-linux-gnu. OK for master?

OK.  Good to see that a bit of optimization can also sneak in with
a bug fix :-)

Best regards

	Thomas