This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[gfortran] Fix PR31266: Wrong string length in substring references to components



Hi,


I had hoped to fix PR30871 together with this one, but unfortunately
the scalarizer is not up to par for that problem.  Therefore I
present you a patch only for PR31266.

The problem was that resolution of component references would copy the
gfc_typespec of the component to the total expression, which is of
course wrong if we have a substring reference at the end of the chain.
As an example,
 var%comp(1:2)
would be given the string length of the component %comp, not the
length of the reference (i.e. 2).  The fix is of course to no longer
copy the string lengths when the component type is applied to the
expression.

I decided to omit the copying only if a substring reference follows.
I.e. for something like var%comp the string length of the whole
expression would be explicit.  This has the advantage that
LEN(var%comp) can be evaluated in the frontend.  It is noteworthy that
the scalarizer manages to determine the string length even of the
substring reference, i.e. LEN(var%comp(1:2)) would appear unevaluated
in the -fdump-parse-tree dump, but it would be reduced to the value 2
in the -fdump-tree-original dump.  If someone understands where this
happens, I think it would be a good idea to move this evaluation
further to the front, so that e.g. the truncation warnings could be
given in more cases.

Less importantly, this patch also rewords the error message when
string truncation is detected.  It now no longer uses the abbreviation
"rhs".  I'm not allto happy about the way the lengths are given inside
the parentheses, though I exchanged the order to match the physical
order in the sourcecode.  If anybody has a suggestion how to present
this information clearer, I would welcome it.  Maybe "(5 vs. 2)" if
that wouldn't look so much like sports journalism?

Built and tested on i386-darwin.  New testcase included.  Ok?  This
patch is a prerequisite for fixing PR30871, so if anybody wants to
have a go at it, this is an excellent time.

Cheers,
- Tobi
Hi,

I had hoped to fix PR30871 together with this one, but unfortunately
the scalarizer is not up to par for that testcase.  Therefore I
present you a patch only for PR31266.

The problem was that resolution of component references would copy the
gfc_typespec of the component to the total expression, which is of
course wrong if we have a substring reference at the end of the chain.
As an example,
 var%comp(1:2)
would be given the string length of the component %comp, not the
length of the reference (i.e. 2).  The fix is of course to no longer
copy the string lengths when the component type is applied to the
expression.

I decided to omit the copying only if a substring reference follows.
I.e. for something like var%comp the string length of the whole
expression would be explicit.  This has the advantage that
LEN(var%comp) can be evaluated in the frontend.  It is noteworthy that
the scalarizer manages to determine the string length even of the
substring reference, i.e. LEN(var%comp(1:2)) would appear unevaluated
in the -fdump-parse-tree dump, but it would be reduced to the value 2
in the -fdump-tree-original dump.  If someone understands where this
happens, I think it would be a good idea to move this evaluation
further to the front, so that e.g. the truncation warnings could be
given in more cases.

Less importantly, this patch also rewords the error message when
string truncation is detected.  It now no longer uses the abbreviation
"rhs".  I'm not allto happy about the way the lengths are given inside
the parentheses, though I exchanged the order to match the physical
order in the sourcecode.  If anybody has a suggestion how to present
this information clearer, I would welcome it.  Maybe "(5 vs. 2)" if
that wouldn't look so much like sports journalism?

Built and tested on i386-darwin.  New testcase included.  Ok?  This
patch is a prerequisite for fixing PR30871, so if anybody wants to
have a go at it, this is an excellent time.

Cheers,
- Tobi

	PR fortran/31266
fortran/
	* primary.c (gfc_variable_attr): Don't copy string length if it
	doesn't make sense.
	* resolve.c (resolve_code): Clarify error message.
testsuite/
	* gfortran.dg/char_assign_1.f90: New.

Index: fortran/primary.c
===================================================================
--- fortran/primary.c	(revision 123625)
+++ fortran/primary.c	(working copy)
@@ -1844,7 +1844,14 @@ gfc_variable_attr (gfc_expr *expr, gfc_t
       case REF_COMPONENT:
 	gfc_get_component_attr (&attr, ref->u.c.component);
 	if (ts != NULL)
-	  *ts = ref->u.c.component->ts;
+	  {
+	    *ts = ref->u.c.component->ts;
+	    /* Don't set the string length if a substring reference
+	       follows.  */
+	    if (ts->type == BT_CHARACTER
+		&& ref->next && ref->next->type == REF_SUBSTRING)
+		ts->cl = NULL;
+	  }
 
 	pointer = ref->u.c.component->pointer;
 	allocatable = ref->u.c.component->allocatable;
Index: fortran/resolve.c
===================================================================
--- fortran/resolve.c	(revision 123625)
+++ fortran/resolve.c	(working copy)
@@ -5135,8 +5135,9 @@ resolve_code (gfc_code *code, gfc_namesp
 		rlen = mpz_get_si (code->expr2->ts.cl->length->value.integer);
 
 	      if (rlen && llen && rlen > llen)
-		gfc_warning_now ("rhs of CHARACTER assignment at %L will be "
-				 "truncated (%d/%d)", &code->loc, rlen, llen);
+		gfc_warning_now ("CHARACTER expression will be truncated "
+				 "in assignment (%d/%d) at %L",
+				 llen, rlen, &code->loc);
 	    }
 
 	  if (gfc_pure (NULL))
Index: testsuite/gfortran.dg/char_assign_1.f90
===================================================================
--- testsuite/gfortran.dg/char_assign_1.f90	(revision 0)
+++ testsuite/gfortran.dg/char_assign_1.f90	(revision 0)
@@ -0,0 +1,23 @@
+! { dg-do run }
+! { dg-options "-Wcharacter-truncation" }
+! Tests the fix for PR31266: references to CHARACTER
+! components lead to the wrong length being assigned to substring
+! expressions.
+type data
+   character(len=5) :: c
+end type data
+type(data), dimension(5), target :: y
+character(len=2), dimension(5) :: p
+character(len=3), dimension(5) :: q
+
+y(:)%c = "abcdef" ! { dg-warning "in assignment \\(5/6\\)" }
+p(1) = y(1)%c(3:)
+if (p(1).ne."cd") call abort()
+
+p(1) = y(1)%c  ! { dg-warning "in assignment \\(2/5\\)" }
+if (p(1).ne."ab") call abort()
+
+q = "xyz"
+p = q ! { dg-warning "CHARACTER expression will be truncated in assignment \\(2/3\\)" }
+if (any (p.ne.q(:)(1:2))) call abort()
+end

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]