| Message ID | CAGkQGi+Lh_2w+BP6c-2h_ru-5R91DF3=CK8+VvQE0ELUwTOMpQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [fortran] PR91717 - ICE on concatenating deferred-length character and character literal | expand |
On Wed, Sep 11, 2019 at 06:44:50AM +0100, Paul Richard Thomas wrote: > =================================================================== > *** gcc/testsuite/gfortran.dg/dependency_55.f90 (nonexistent) > --- gcc/testsuite/gfortran.dg/dependency_55.f90 (working copy) > *************** > *** 0 **** > --- 1,17 ---- > + ! { dg-do run } > + ! > + ! Test the fix for PR91717 in which the concatenation operation ICEd. > + ! > + ! Contributed by Damian Rouson <damian@sourceryinstitute.org> > + ! > + type core > + character (len=:), allocatable :: msg > + end type > + > + type(core) :: my_core > + > + my_core%msg = my_core%msg//"my message is: " my_core%msg is undefined on the RHS. This is invalid Fortran. Not sure whether your patch is correct or not.
Hi Steve,
Being an allocatable component, this code appears on entry into scope:
my_core.msg = 0B;
my_core._msg_length = 0;
{
Thus, according to the standard, my_core%msg is defined.
If you follow the backtrace from the ICE, you will see that something
like the patch is required. It is important that the dependency is
detected for this assignment even though no array references are
involved. There is certainly the need for a temporary, which the patch
generates.
Cheers
Paul
On Wed, 11 Sep 2019 at 06:50, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
>
> On Wed, Sep 11, 2019 at 06:44:50AM +0100, Paul Richard Thomas wrote:
> > ===================================================================
> > *** gcc/testsuite/gfortran.dg/dependency_55.f90 (nonexistent)
> > --- gcc/testsuite/gfortran.dg/dependency_55.f90 (working copy)
> > ***************
> > *** 0 ****
> > --- 1,17 ----
> > + ! { dg-do run }
> > + !
> > + ! Test the fix for PR91717 in which the concatenation operation ICEd.
> > + !
> > + ! Contributed by Damian Rouson <damian@sourceryinstitute.org>
> > + !
> > + type core
> > + character (len=:), allocatable :: msg
> > + end type
> > +
> > + type(core) :: my_core
> > +
> > + my_core%msg = my_core%msg//"my message is: "
>
> my_core%msg is undefined on the RHS. This is invalid
> Fortran. Not sure whether your patch is correct or not.
>
> --
> Steve
On Wed, Sep 11, 2019 at 11:27:56PM +0100, Paul Richard Thomas wrote: > Hi Steve, > > Being an allocatable component, this code appears on entry into scope: > > my_core.msg = 0B; > my_core._msg_length = 0; > { > > Thus, according to the standard, my_core%msg is defined. > I'll politely defer to the Fortran standard. 5.4.10 Allocatable variables The allocation status of an allocatable variable is either allocated or unallocated. An allocatable variable becomes allocated as described in 9.7.1.3. It becomes unallocated as described in 9.7.3.2. An unallocated allocatable variable shall not be referenced or defined. 7.5.4.6 Default initialization for components Allocatable components are always initialized to unallocated. In your testcase, you have an unallocatable allocatable entity referenced on the RHS in the first line that involves my_core%msg. From the Fortran standard's point of view, I believe the code is nonconforming. type core character (len=:), allocatable :: msg end type type(core) :: my_core print *, allocated(my_core%msg) ! Comment out to avoid ICE. ! my_core%msg on RHS is unallocated in next line. ! my_core%msg = my_core%msg//"my message is: ! my_core%msg = my_core%msg//"Hello!" ! ! if (my_core%msg .ne. "my message is: Hello!") stop 1 end % gfcx -o z a.f90 && ./z F > detected for this assignment even though no array references are > involved. There is certainly the need for a temporary, which the patch > generates. > Your patch may fix the ICE, but if the code compiles and execute. You are getting the "right" answer fortuitiouly. Of course, I could be wrong.
Hi Steve, I have inserted a my_core%msg = '"" so that it is initialised. The reallocation on assignment explicitly deals with an unallocated entity or one of its allocatable components by allocation, rather than reallocation. I realise that my explanation of the patch might be a bit sparse. The ultimate caller is frontend_passes.c(realloc_string_callback), which looks for overlap even for scalar cases. The ICE came about because there are no array references in the expressions being compared. Flagging that there are identical component chains in this case and returning 1 from gfc_dep_resolver covers this case. OK to commit? Cheers Paul realloc_string_callback) On Thu, 12 Sep 2019 at 00:05, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > > On Wed, Sep 11, 2019 at 11:27:56PM +0100, Paul Richard Thomas wrote: > > Hi Steve, > > > > Being an allocatable component, this code appears on entry into scope: > > > > my_core.msg = 0B; > > my_core._msg_length = 0; > > { > > > > Thus, according to the standard, my_core%msg is defined. > > > > I'll politely defer to the Fortran standard. > > 5.4.10 Allocatable variables > > The allocation status of an allocatable variable is either allocated > or unallocated. An allocatable variable becomes allocated as described > in 9.7.1.3. It becomes unallocated as described in 9.7.3.2. > > An unallocated allocatable variable shall not be referenced or defined. > > 7.5.4.6 Default initialization for components > > Allocatable components are always initialized to unallocated. > > In your testcase, you have an unallocatable allocatable entity > referenced on the RHS in the first line that involves my_core%msg. > From the Fortran standard's point of view, I believe the code is > nonconforming. > > type core > character (len=:), allocatable :: msg > end type > > type(core) :: my_core > > print *, allocated(my_core%msg) > > ! Comment out to avoid ICE. > ! my_core%msg on RHS is unallocated in next line. > ! my_core%msg = my_core%msg//"my message is: > ! my_core%msg = my_core%msg//"Hello!" > ! > ! if (my_core%msg .ne. "my message is: Hello!") stop 1 > end > > % gfcx -o z a.f90 && ./z > F > > > detected for this assignment even though no array references are > > involved. There is certainly the need for a temporary, which the patch > > generates. > > > > Your patch may fix the ICE, but if the code compiles and > execute. You are getting the "right" answer fortuitiouly. > > Of course, I could be wrong. > > -- > Steve
On Thu, Sep 12, 2019 at 09:55:20AM +0100, Paul Richard Thomas wrote: > > OK to commit? > Yes.
Thanks - committed as r275696. Paul On Thu, 12 Sep 2019 at 15:09, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > > On Thu, Sep 12, 2019 at 09:55:20AM +0100, Paul Richard Thomas wrote: > > > > OK to commit? > > > > Yes. > > -- > Steve
Index: gcc/fortran/dependency.c =================================================================== *** gcc/fortran/dependency.c (revision 275323) --- gcc/fortran/dependency.c (working copy) *************** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 2096,2101 **** --- 2096,2102 ---- int m; gfc_dependency fin_dep; gfc_dependency this_dep; + bool same_component = false; this_dep = GFC_DEP_ERROR; fin_dep = GFC_DEP_ERROR; *************** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 2115,2120 **** --- 2116,2123 ---- components. */ if (lref->u.c.component != rref->u.c.component) return 0; + + same_component = true; break; case REF_SUBSTRING: *************** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 2280,2285 **** --- 2283,2292 ---- if (lref || rref) return 1; + /* This can result from concatenation of assumed length string components. */ + if (same_component && fin_dep == GFC_DEP_ERROR) + return 1; + /* If we haven't seen any array refs then something went wrong. */ gcc_assert (fin_dep != GFC_DEP_ERROR); Index: gcc/testsuite/gfortran.dg/dependency_55.f90 =================================================================== *** gcc/testsuite/gfortran.dg/dependency_55.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/dependency_55.f90 (working copy) *************** *** 0 **** --- 1,17 ---- + ! { dg-do run } + ! + ! Test the fix for PR91717 in which the concatenation operation ICEd. + ! + ! Contributed by Damian Rouson <damian@sourceryinstitute.org> + ! + type core + character (len=:), allocatable :: msg + end type + + type(core) :: my_core + + my_core%msg = my_core%msg//"my message is: " + my_core%msg = my_core%msg//"Hello!" + + if (my_core%msg .ne. "my message is: Hello!") stop 1 + end
Hi All, I nearly committed this patch as 'obvious' but noticed a fair number of changes in 10-branch in dependency analysis. This is in fact a 10-regression. I'll wait for an OK. Bootstrapped and regtested on FC30/x86_64 - OK to commit? Paul 2019-09-11 Paul Thomas <pault@gcc.gnu.org> PR fortran/91717 * dependency.c (gfc_dep_resolver): Flag identical components and exit with return value 1 if set and no array refs. 2019-09-11 Paul Thomas <pault@gcc.gnu.org> PR fortran/91717 * gfortran.dg/dependency_55.f90 : New test.