This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, libfortran, 3/3] Update file position lazily
On Saturday 29 October 2011 10:09:07 Janne Blomqvist wrote:
> On Sat, Oct 29, 2011 at 01:48, Mikael Morin <mikael.morin@sfr.fr> wrote:
> > On Tuesday 18 October 2011 17:11:24 Janne Blomqvist wrote:
> >> Also, I think I've found a small standards conformance bug. From F2008
> >> (N1830) 9.10.2.23 (page 256): "... ASIS if the connection was opened
> >> without changing its position." and "If the ïle has been repositioned
> >> since the connection, the scalar-default-char-variable
> >> is assigned a processor-dependent value, which shall not be REWIND
> >> unless the ïle is positioned at its initial
> >> point and shall not be APPEND unless the ïle is positioned so that its
> >> endïle record is the next record or at its
> >> terminal point if it has no endïle record.
> >> "
> >>
> >> If my understanding of the above is correct, returning ASIS is
> >> incorrent unless the position is unchanged since the OPEN statement.
> >> Currently we return ASIS by default if it's neither REWIND nor APPEND.
> >> So the patch changes the implementation to return the
> >> processor-dependent value UNSPECIFIED in this case.
> >
> > If my reading is correct, returning ASIS is as valid as returning
> > UNSPECIFIED ("processor-dependent"). I have a preference for UNSPECIFIED
> > and see your patch as OK, but shouldn't it be avoided if it breaks
> > backwards compatibility?
>
> My thinking was that the first sentence I quoted would prohibit ASIS
> even though it's not explicitly forbidden in the second quoted
> sentence. Fixing the implementation would thus be correcting a
> standards-conformance bug.
Well, the first sentence does impose the value to be ASIS in case the position
hasn't changed, but it does not impose the value not to be ASIS in case the
position has changed.
On the other hand, let's think about the use cases: if we are returning ASIS
even if the position has changed, we can't use that value reliably to tell
that the position hasn't changed.
Thus, I think your patch is OK with the following changes.
>
> FWIW, it seems ifort 12.0 uses "UNDEFINED" in this case; I suppose a
> case could be made for using the same. Comments?
Let's go for UNDEFINED then.
>
> > I'm also afraid of testsuite changes of the following kind.
> > Was there no reason for the "-std=legacy"?
> >
> > diff --git a/gcc/testsuite/gfortran.dg/inquire_5.f90
> > b/gcc/testsuite/gfortran.dg/inquire_5.f90
> > index fe107a1..064f96d 100644
> > --- a/gcc/testsuite/gfortran.dg/inquire_5.f90
> > +++ b/gcc/testsuite/gfortran.dg/inquire_5.f90
> > @@ -1,11 +1,10 @@
> > ! { dg-do run { target fd_truncate } }
> > -! { dg-options "-std=legacy" }
> > !
>
> I changed the declaration of "chr" from "character*20" to
> "character(len=20)" which made std=legacy unnecessary. As the testcase
> doesn't test any legacy functionality per se, I though this change
> would slightly simplify it. See also
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40881
>
> which mass-added std=legacy to a number of testcases (including this
> one) as a result of some frontend warnings changes.
OK for that part.
>
> @@ -31,7 +30,8 @@
> write(7,*)'this is another record'
> backspace(7)
> inquire(7,position=chr)
> - if (chr.NE.'ASIS') CALL ABORT
> + if (chr.eq.'ASIS' .or. chr .eq. 'REWIND' &
> + .or. chr .eq. 'APPEND') CALL ABORT
I think it's better to keep the more restrictive:
if (chr.NE.'UNDEFINED') CALL ABORT
Just in case we have a memory leak some day, which makes us return some junk
here.
Please give the other some time to comment on our discussion before
committing.
Thanks for the patch.
Mikael