| Message ID | a4470f1d-a946-a5bf-7934-5ef8e88ac3d9@netcologne.de |
|---|---|
| State | New |
| Headers | show |
| Series | [fortran] Fix character length in constructors | expand |
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
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.
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.
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
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"?
Am 20.02.2018 um 20:10 schrieb Janne Blomqvist:
> Shouldn't the second one be "stop 2"?
Corrected, r257859.
Regards
Thomas
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;
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.