[fortran] Fix character length in constructors

Message ID a4470f1d-a946-a5bf-7934-5ef8e88ac3d9@netcologne.de
State New
Headers show
Series
  • [fortran] Fix character length in constructors
Related show

Commit Message

Thomas Koenig Feb. 19, 2018, 10:41 p.m.
Hello world,

when putting in a seemingly innocent simplification for PR 56342,
I caused a regression in PR 82823, in PACK. The root cause of
this one turned out to be PR 48890, in which structure
constructors containing characters were not handled correctly
if the lengths did not match.

The attached patch fixes that.

Regression-tested. OK for trunk?

Regards

	Thomas

2018-02-19  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/48890
         PR fortran/83823
         * primary.c (gfc_convert_to_structure_constructor):
         For a constant string constructor, make sure the length
         is correct.

2018-02-19  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/48890
         PR fortran/83823
         * gfortran.dg/structure_constructor_14.f90: New test.

Comments

Steve Kargl Feb. 19, 2018, 10:51 p.m. | #1
On Mon, Feb 19, 2018 at 11:41:30PM +0100, Thomas Koenig wrote:
> 

> Regression-tested. OK for trunk?

> 


OK with the fix suggested below.


> Index: primary.c

> ===================================================================

> --- primary.c	(Revision 257788)

> +++ primary.c	(Arbeitskopie)

> @@ -2879,6 +2879,38 @@ gfc_convert_to_structure_constructor (gfc_expr *e,

>        if (!this_comp)

>  	goto cleanup;

>  

> +      /* For a constant string constructor, make sure the length is correct;

> +	 truncate of fill with blanks if needed.  */


truncate or fill

-- 
Steve
Janne Blomqvist Feb. 20, 2018, 7:51 a.m. | #2
On Tue, Feb 20, 2018 at 12:41 AM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hello world,

>

> when putting in a seemingly innocent simplification for PR 56342,

> I caused a regression in PR 82823, in PACK. The root cause of

> this one turned out to be PR 48890, in which structure

> constructors containing characters were not handled correctly

> if the lengths did not match.

>

> The attached patch fixes that.

>

> Regression-tested. OK for trunk?


It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().
mpz_get_si() returns a long, which is a 32-bit type on win64.

Otherwise Ok with the spelling fix suggested by Steve.

-- 
Janne Blomqvist
Steve Kargl Feb. 20, 2018, 3:29 p.m. | #3
On Tue, Feb 20, 2018 at 09:51:14AM +0200, Janne Blomqvist wrote:
> On Tue, Feb 20, 2018 at 12:41 AM, Thomas Koenig <tkoenig@netcologne.de> wrote:

> > Hello world,

> >

> > when putting in a seemingly innocent simplification for PR 56342,

> > I caused a regression in PR 82823, in PACK. The root cause of

> > this one turned out to be PR 48890, in which structure

> > constructors containing characters were not handled correctly

> > if the lengths did not match.

> >

> > The attached patch fixes that.

> >

> > Regression-tested. OK for trunk?

> 

> It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().

> mpz_get_si() returns a long, which is a 32-bit type on win64.

> 

> Otherwise Ok with the spelling fix suggested by Steve.

> 


Good catch.  I don't know how I forgot that you've
spent a lot time fixing int, long, size_t, ssize_t
issues.

-- 
Steve
Thomas Koenig Feb. 20, 2018, 6:59 p.m. | #4
Am 20.02.2018 um 08:51 schrieb Janne Blomqvist:

> It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().

> mpz_get_si() returns a long, which is a 32-bit type on win64.

> 

> Otherwise Ok with the spelling fix suggested by Steve.


Committed as r257856.

Thanks to you and Steve for the review and the heads_up about
gfc_mpz_get_hwi.

I also changed the "call abort" to "STOP 1" at the last second
(old habits are hard to break, it seems :-)

Regards

	Thomas
Janne Blomqvist Feb. 20, 2018, 7:10 p.m. | #5
On Tue, Feb 20, 2018 at 8:59 PM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Am 20.02.2018 um 08:51 schrieb Janne Blomqvist:

>

>> It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().

>> mpz_get_si() returns a long, which is a 32-bit type on win64.

>>

>> Otherwise Ok with the spelling fix suggested by Steve.

>

>

> Committed as r257856.

>

> Thanks to you and Steve for the review and the heads_up about

> gfc_mpz_get_hwi.

>

> I also changed the "call abort" to "STOP 1" at the last second

> (old habits are hard to break, it seems :-)


Shouldn't the second one be "stop 2"?


-- 
Janne Blomqvist
Thomas Koenig Feb. 20, 2018, 7:54 p.m. | #6
Am 20.02.2018 um 20:10 schrieb Janne Blomqvist:
> Shouldn't the second one be "stop 2"?


Corrected, r257859.

Regards

	Thomas

Patch

Index: primary.c
===================================================================
--- primary.c	(Revision 257788)
+++ primary.c	(Arbeitskopie)
@@ -2879,6 +2879,38 @@  gfc_convert_to_structure_constructor (gfc_expr *e,
       if (!this_comp)
 	goto cleanup;
 
+      /* For a constant string constructor, make sure the length is correct;
+	 truncate of fill with blanks if needed.  */
+      if (this_comp->ts.type == BT_CHARACTER && !this_comp->attr.allocatable
+	  && this_comp->ts.u.cl && this_comp->ts.u.cl->length
+	  && this_comp->ts.u.cl->length->expr_type == EXPR_CONSTANT
+	  && actual->expr->expr_type == EXPR_CONSTANT)
+	{
+	  ptrdiff_t c, e;
+	  c = mpz_get_si (this_comp->ts.u.cl->length->value.integer);
+	  e = actual->expr->value.character.length;
+
+	  if (c != e)
+	    {
+	      ptrdiff_t i, to;
+	      gfc_char_t *dest;
+	      dest = gfc_get_wide_string (c + 1);
+
+	      to = e < c ? e : c;
+	      for (i = 0; i < to; i++)
+		dest[i] = actual->expr->value.character.string[i];
+	      
+	      for (i = e; i < c; i++)
+		dest[i] = ' ';
+
+	      dest[c] = '\0';
+	      free (actual->expr->value.character.string);
+
+	      actual->expr->value.character.length = c;
+	      actual->expr->value.character.string = dest;
+	    }
+	}
+
       comp_tail->val = actual->expr;
       if (actual->expr != NULL)
 	comp_tail->where = actual->expr->where;