[PR,fortran/89077,part,2] - ICE using * as len specifier for character parameter

Message ID 5C5DE86B.7060009@gmx.de
State New
Headers show
Series
  • [PR,fortran/89077,part,2] - ICE using * as len specifier for character parameter
Related show

Commit Message

Harald Anlauf Feb. 8, 2019, 8:36 p.m.
The attached patch attempts a substring length simplification
so that more complex expressions are handled in initialization
expressions.  Thanks to Thomas König for the suggestion.

Regtested on x86_64-pc-linux-gnu.

(The PR still has other wrong-code issue to be addressed separately.)

OK for trunk?  And for backports to 8/7?

Thanks,
Harald


2019-02-08  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/89077
	* resolve.c (gfc_resolve_substring_charlen): Check substring
	length for constantness prior to general calculation of length.

2019-02-08  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/89077
	* gfortran.dg/substr_simplify.f90: New test.

Index: gcc/testsuite/gfortran.dg/substr_simplify.f90
===================================================================
--- gcc/testsuite/gfortran.dg/substr_simplify.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/substr_simplify.f90	(working copy)
@@ -0,0 +1,20 @@
+! { dg-do run }
+!
+! Test fixes for substring simplications derived from
+! PR fortran/89077 - ICE using * as len specifier for character parameter
+
+program test
+  implicit none
+  integer :: i
+  character(*), parameter :: s = 'abcdef', y = 'efcdab'
+  character(6), save      :: t = transfer ([(s(i:i),  i=1,len(s)  )], s)
+  character(*), parameter :: u = transfer ([(s(i:i+2),i=1,len(s),3)], s)
+  character(6), save      :: v = transfer ([(s(i:i+2),i=1,len(s),3)], s)
+  character(*), parameter :: w = transfer ([(y(i:i+1),i=len(s)-1,1,-2)], s)
+  character(6), save      :: x = transfer ([(y(i:i+1),i=len(s)-1,1,-2)], s)
+  if (len (t) /= len (s) .or. t /= s) stop 1
+  if (len (u) /= len (s) .or. u /= s) stop 2
+  if (len (v) /= len (s) .or. v /= s) stop 3
+  if (len (w) /= len (s) .or. w /= s) stop 4
+  if (len (x) /= len (s) .or. x /= s) stop 5
+end

Comments

Thomas Koenig Feb. 9, 2019, 1:03 p.m. | #1
Hi Harald,

> OK for trunk?  And for backports to 8/7?


I played around with your patch and found a few problems with
ICEs, but these were all pre-existing as far as I could determine;
I have submitted PR89266 for what I discovered.

I a bit concerned about the case of like a(i:i-2) returning negative
lengths. Could you maybe set the length to zero if it is
calculated to be negative?

I'd say that the patch is OK for trunk with that change, even though
we are technically in regression-only mode. It is fairly localized,
and should not have ill effects.  Regarding backport to the other
open branches - well, it does not fix a regression, so I'd be
inclined not to backport it.

Regards

	Thomas
Harald Anlauf Feb. 9, 2019, 5:30 p.m. | #2
Committed to trunk as rev. 268726. after adding a comment that a check
for negative substring length is already present.  The updated version
is attached.

Thanks for the review.  Will not backport unless requested.

Harald

On 02/08/19 21:36, Harald Anlauf wrote:
> The attached patch attempts a substring length simplification

> so that more complex expressions are handled in initialization

> expressions.  Thanks to Thomas König for the suggestion.

> 

> Regtested on x86_64-pc-linux-gnu.

> 

> (The PR still has other wrong-code issue to be addressed separately.)

> 

> OK for trunk?  And for backports to 8/7?

> 

> Thanks,

> Harald

> 

> 

> 2019-02-08  Harald Anlauf  <anlauf@gmx.de>

> 

> 	PR fortran/89077

> 	* resolve.c (gfc_resolve_substring_charlen): Check substring

> 	length for constantness prior to general calculation of length.

> 

> 2019-02-08  Harald Anlauf  <anlauf@gmx.de>

> 

> 	PR fortran/89077

> 	* gfortran.dg/substr_simplify.f90: New test.

> 



-- 
Harald Anlauf
Dieburger Str. 17
60386 Frankfurt
Tel.: (069) 4014 8318
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 268725)
+++ gcc/fortran/resolve.c	(working copy)
@@ -4965,6 +4965,7 @@
   gfc_ref *char_ref;
   gfc_expr *start, *end;
   gfc_typespec *ts = NULL;
+  mpz_t diff;
 
   for (char_ref = e->ref; char_ref; char_ref = char_ref->next)
     {
@@ -5016,12 +5017,26 @@
       return;
     }
 
-  /* Length = (end - start + 1).  */
-  e->ts.u.cl->length = gfc_subtract (end, start);
-  e->ts.u.cl->length = gfc_add (e->ts.u.cl->length,
-				gfc_get_int_expr (gfc_charlen_int_kind,
-						  NULL, 1));
+  /* Length = (end - start + 1).
+     Check first whether it has a constant length.  */
+  if (gfc_dep_difference (end, start, &diff))
+    {
+      gfc_expr *len = gfc_get_constant_expr (BT_INTEGER, gfc_charlen_int_kind,
+					     &e->where);
 
+      mpz_add_ui (len->value.integer, diff, 1);
+      mpz_clear (diff);
+      e->ts.u.cl->length = len;
+      /* The check for length < 0 is handled below */
+    }
+  else
+    {
+      e->ts.u.cl->length = gfc_subtract (end, start);
+      e->ts.u.cl->length = gfc_add (e->ts.u.cl->length,
+				    gfc_get_int_expr (gfc_charlen_int_kind,
+						      NULL, 1));
+    }
+
   /* F2008, 6.4.1:  Both the starting point and the ending point shall
      be within the range 1, 2, ..., n unless the starting point exceeds
      the ending point, in which case the substring has length zero.  */

Patch

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 268658)
+++ gcc/fortran/resolve.c	(working copy)
@@ -4965,6 +4965,7 @@ 
   gfc_ref *char_ref;
   gfc_expr *start, *end;
   gfc_typespec *ts = NULL;
+  mpz_t diff;
 
   for (char_ref = e->ref; char_ref; char_ref = char_ref->next)
     {
@@ -5016,12 +5017,25 @@ 
       return;
     }
 
-  /* Length = (end - start + 1).  */
-  e->ts.u.cl->length = gfc_subtract (end, start);
-  e->ts.u.cl->length = gfc_add (e->ts.u.cl->length,
-				gfc_get_int_expr (gfc_charlen_int_kind,
-						  NULL, 1));
+  /* Length = (end - start + 1).
+     Check first whether it has a constant length.  */
+  if (gfc_dep_difference (end, start, &diff))
+    {
+      gfc_expr *len = gfc_get_constant_expr (BT_INTEGER, gfc_charlen_int_kind,
+					     &e->where);
 
+      mpz_add_ui (len->value.integer, diff, 1);
+      mpz_clear (diff);
+      e->ts.u.cl->length = len;
+    }
+  else
+    {
+      e->ts.u.cl->length = gfc_subtract (end, start);
+      e->ts.u.cl->length = gfc_add (e->ts.u.cl->length,
+				    gfc_get_int_expr (gfc_charlen_int_kind,
+						      NULL, 1));
+    }
+
   /* F2008, 6.4.1:  Both the starting point and the ending point shall
      be within the range 1, 2, ..., n unless the starting point exceeds
      the ending point, in which case the substring has length zero.  */