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

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

[Harald Anlauf]

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

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.


[-- Attachment #2: patch-pr89077-part2 --]
[-- Type: text/plain, Size: 1413 bytes --]

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.  */

[-- Attachment #3: patch-pr89077-part2-testcase --]
[-- Type: text/plain, Size: 1134 bytes --]

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

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

[Thomas Koenig]

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

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

[Harald Anlauf]

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

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

[-- Attachment #2: patch-pr89077-part2-v2 --]
[-- Type: text/plain, Size: 1468 bytes --]

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.  */