Message ID | CAGkQGi+Rq08DYLx8C2yXjUX8m2-3NUuMjtpExrbWdpb3anEF=w@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | [fortran] PR58618 - Wrong code with character substring and ASSOCIATE | expand |
Hi Paul, Paul Richard Thomas wrote: > This problem concerned associate targets being substrings. It turns > out that they are returned as pointer types (with a different cast for > unity based substrings ***sigh***) and so can be assigned directly to > the associate name. The patch quite simply removed the condition that > such targets be allocatable, pointer or dummy. > I noticed in the course of working up the testcase that > character (:), pointer :: ptr => NULL() > character (6), target :: tgt = 'lmnopq' > ptr => tgt > print *, len (ptr), ptr > end > ICEs on the NULL initialization of the pointer but works fine if this > is removed. Has this already been posted as a PR? I leave it to Dominique to search for a PR; otherwise, I believe the attach patch fixes the issue. – It just needs someone to package it with a test case, regtest and commit it. > Bootstrapped and regtested on FC28/x86_64 - OK for trunk? OK – thanks for the fix. Tobias > 2018-10-17 Paul Thomas <pault@gcc.gnu.org> > > PR fortran/58618 > * trans-stmt.c (trans_associate_var): All strings that return > as pointer types can be assigned directly to the associate > name so remove 'attr' and the condition that uses it. > > 2018-10-17 Paul Thomas <pault@gcc.gnu.org> > > PR fortran/58618 > * gfortran.dg/associate_45.f90 : New test. diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index b0c12e5fc38..88f9f570725 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -1762,7 +1762,8 @@ gfc_get_symbol_decl (gfc_symbol * sym) gfc_finish_var_decl (length, sym); if (!sym->attr.associate_var && TREE_CODE (length) == VAR_DECL - && sym->value && sym->value->ts.u.cl->length) + && sym->value && sym->value->expr_type != EXPR_NULL + && sym->value->ts.u.cl->length) { gfc_expr *len = sym->value->ts.u.cl->length; DECL_INITIAL (length) = gfc_conv_initializer (len, &len->ts, @@ -1772,7 +1773,7 @@ gfc_get_symbol_decl (gfc_symbol * sym) DECL_INITIAL (length)); } else - gcc_assert (!sym->value); + gcc_assert (!sym->value || sym->value->expr_type == EXPR_NULL); } gfc_finish_var_decl (decl, sym);
I do not think that there will be a PR for the ICE. This is a regression introduced by my patch for PR70149 (September 30th). A patch is attached. I will commit it as 'obvious' as soon as it has finished regtesting. I will also commit the patch for PR58618 shortly afterwards. Thanks for the review. Paul On Wed, 17 Oct 2018 at 22:17, Tobias Burnus <burnus@net-b.de> wrote: > > Hi Paul, > > Paul Richard Thomas wrote: > > This problem concerned associate targets being substrings. It turns > > out that they are returned as pointer types (with a different cast for > > unity based substrings ***sigh***) and so can be assigned directly to > > the associate name. The patch quite simply removed the condition that > > such targets be allocatable, pointer or dummy. > > I noticed in the course of working up the testcase that > > character (:), pointer :: ptr => NULL() > > character (6), target :: tgt = 'lmnopq' > > ptr => tgt > > print *, len (ptr), ptr > > end > > ICEs on the NULL initialization of the pointer but works fine if this > > is removed. Has this already been posted as a PR? > > > I leave it to Dominique to search for a PR; otherwise, I believe the > attach patch fixes the issue. – It just needs someone to package it with > a test case, regtest and commit it. > > > > Bootstrapped and regtested on FC28/x86_64 - OK for trunk? > > OK – thanks for the fix. > > Tobias > > > 2018-10-17 Paul Thomas <pault@gcc.gnu.org> > > > > PR fortran/58618 > > * trans-stmt.c (trans_associate_var): All strings that return > > as pointer types can be assigned directly to the associate > > name so remove 'attr' and the condition that uses it. > > > > 2018-10-17 Paul Thomas <pault@gcc.gnu.org> > > > > PR fortran/58618 > > * gfortran.dg/associate_45.f90 : New test.
Patch for the PR70149 regression committed as revision 265263. Likewise the patch for PR58618 has been committed as revision 265264. Cheers Paul On Wed, 17 Oct 2018 at 22:17, Tobias Burnus <burnus@net-b.de> wrote: > > Hi Paul, > > Paul Richard Thomas wrote: > > This problem concerned associate targets being substrings. It turns > > out that they are returned as pointer types (with a different cast for > > unity based substrings ***sigh***) and so can be assigned directly to > > the associate name. The patch quite simply removed the condition that > > such targets be allocatable, pointer or dummy. > > I noticed in the course of working up the testcase that > > character (:), pointer :: ptr => NULL() > > character (6), target :: tgt = 'lmnopq' > > ptr => tgt > > print *, len (ptr), ptr > > end > > ICEs on the NULL initialization of the pointer but works fine if this > > is removed. Has this already been posted as a PR? > > > I leave it to Dominique to search for a PR; otherwise, I believe the > attach patch fixes the issue. – It just needs someone to package it with > a test case, regtest and commit it. > > > > Bootstrapped and regtested on FC28/x86_64 - OK for trunk? > > OK – thanks for the fix. > > Tobias > > > 2018-10-17 Paul Thomas <pault@gcc.gnu.org> > > > > PR fortran/58618 > > * trans-stmt.c (trans_associate_var): All strings that return > > as pointer types can be assigned directly to the associate > > name so remove 'attr' and the condition that uses it. > > > > 2018-10-17 Paul Thomas <pault@gcc.gnu.org> > > > > PR fortran/58618 > > * gfortran.dg/associate_45.f90 : New test.
Index: gcc/fortran/trans-stmt.c =================================================================== *** gcc/fortran/trans-stmt.c (revision 265231) --- gcc/fortran/trans-stmt.c (working copy) *************** trans_associate_var (gfc_symbol *sym, gf *** 1656,1662 **** bool need_len_assign; bool whole_array = true; gfc_ref *ref; - symbol_attribute attr; gcc_assert (sym->assoc); e = sym->assoc->target; --- 1656,1661 ---- *************** trans_associate_var (gfc_symbol *sym, gf *** 1916,1924 **** } } - attr = gfc_expr_attr (e); if (sym->ts.type == BT_CHARACTER && e->ts.type == BT_CHARACTER - && (attr.allocatable || attr.pointer || attr.dummy) && POINTER_TYPE_P (TREE_TYPE (se.expr))) { /* These are pointer types already. */ --- 1915,1921 ---- *************** trans_associate_var (gfc_symbol *sym, gf *** 1926,1933 **** } else { ! tmp = TREE_TYPE (sym->backend_decl); ! tmp = gfc_build_addr_expr (tmp, se.expr); } gfc_add_modify (&se.pre, sym->backend_decl, tmp); --- 1923,1930 ---- } else { ! tmp = TREE_TYPE (sym->backend_decl); ! tmp = gfc_build_addr_expr (tmp, se.expr); } gfc_add_modify (&se.pre, sym->backend_decl, tmp); Index: gcc/testsuite/gfortran.dg/associate_45.f90 =================================================================== *** gcc/testsuite/gfortran.dg/associate_45.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/associate_45.f90 (working copy) *************** *** 0 **** --- 1,38 ---- + ! { dg-do run } + ! + ! Test the fix for PR58618 by checking that substring associate targets + ! work correctly. + ! + ! Contributed by Vladimir Fuka <vladimir.fuka@gmail.com> + ! + character(5) :: s(2) = ['abcde','fghij'] + character (6), pointer :: ptr => NULL() + character (6), target :: tgt = 'lmnopq' + + associate (x=>s(2)(3:4)) + if (x .ne. 'hi') stop 1 + x = 'uv' + end associate + if (any (s .ne. ['abcde','fguvj'])) stop 2 + + ! Unity based substrings are cast differently. */ + associate (x=>s(1)(1:4)) + if (x .ne. 'abcd') stop 3 + x(2:3) = 'wx' + end associate + if (any (s .ne. ['awxde','fguvj'])) stop 4 + + ! Make sure that possible misidentifications do not occur. + ptr => tgt + associate (x=>ptr) + if (x .ne. 'lmnopq') stop 5 + x(2:3) = 'wx' + end associate + if (tgt .ne. 'lwxopq') stop 6 + + associate (x=>ptr(5:6)) + if (x .ne. 'pq') stop 7 + x = 'wx' + end associate + if (tgt .ne. 'lwxowx') stop 8 + end