[Fortran] PR49791 - Fix legacy namelist support

This patch partially undoes the change done in PR 46010.

For that patch, first (comment 7)

+              || !dtp->u.p.ionml->touched)

was added - and after not solving the problem, the complete change was 
(comment 9 and committed version)

+		      || !dtp->u.p.ionml->touched
+		      || dtp->u.p.ionml->type == BT_DERIVED)



As the current PR 49791 shows, the "touched" line causes the test case 
to fail. Well, my simple solution was to remove the "touch" line and 
keep only the BT_DERIVED line.

I have not the slightest idea what the "touch" does (while it is clear 
to me why the BT_DERIVED helped with the old PR 46010). Thus, I don't 
know whether there are any side effects. At least there are none which 
the test suite covers.

Build and regtested on x86-64-linux. OK for the trunk?

To which branch versions do we want to backport? The patch for PR 46010 
was applied end of last year and affects the branches 4.4, 4.5 and 4.6. 
As it is a regression, I am inclined to apply it to all versions 
starting from 4.4.

If the patch makes sense, I think it should be have a relatively low 
regression risk: It is close to the before October 2010 version but it 
continues to fix PR 49791. Thus, there should be no problem for 
backporting in this regard.

Tobias

On Thu, Jul 21, 2011 at 12:30:53AM +0200, Tobias Burnus wrote:
> 
> Build and regtested on x86-64-linux. OK for the trunk?
> 

The patch looks incomplete.  Where is gfc_warning("Extension: expanding
namelist with nonstandard array syntax")?

Steve Kargl wrote:
> The patch looks incomplete. Where is gfc_warning("Extension: expanding 
> namelist with nonstandard array syntax")? 

There exists *no* gfc_warning in libgfortran. And as it is not a 
compiled file but file read by the compiled program, there is no 
possibility to diagnose this at compile time.

Additionally, the code in libgfortran only supports this vendor 
extension if one uses -std=gnu or -std=legacy. For, e.g.,  -std=f2008 
the extended read support is silently disabled. -- You could have seen 
this in the patch.

As pointed out in the bug report, nearly all compilers do support this 
vendor extension - and it is even documented in gfortran's manual: 
http://gcc.gnu.org/onlinedocs/gfortran/Extensions-to-namelist.html

Tobias

On Thu, Jul 21, 2011 at 07:59:36AM +0200, Tobias Burnus wrote:
> Steve Kargl wrote:
> >The patch looks incomplete. Where is gfc_warning("Extension: expanding 
> >namelist with nonstandard array syntax")? 
> 
> There exists *no* gfc_warning in libgfortran. And as it is not a 
> compiled file but file read by the compiled program, there is no 
> possibility to diagnose this at compile time.

There is generate_warning().  If one can disable this 
extension vai -std=f95, then one should be able to 
emit a warning.

> Additionally, the code in libgfortran only supports this vendor 
> extension if one uses -std=gnu or -std=legacy. For, e.g.,  -std=f2008 
> the extended read support is silently disabled. -- You could have seen 
> this in the patch.

I read the patch.  I disagree with it.  I think the extention,
which was removed over a year ago, should not be restored.

On 07/21/2011 08:13 AM, Steve Kargl wrote:
> There is generate_warning(). If one can disable this extension vai 
> -std=f95, then one should be able to emit a warning

But one does not have to. While for the compiler, it might be okay and 
often is useful to emit warnings, having warnings printed from the 
library is much less useful. Either the output gets drowned in all the 
other output such that no one sees it - or it breaks scripts which do 
not expect this extra output. Especially, the programmer might not use 
input which causes the library to emit warnings - and the user may not 
have the source code to fix it.

> I think the extention, which was removed over a year ago, should not 
> be restored

And I disagree. The extension itself was not removed - there are even 
testsuite testcases which test for it.* However, as it turned out, there 
exists namelists which are valid according to the documented GNU 
extension but are rejected. - That's not new given how many constructs 
namelists allow.

Additionally, it is not a year ago but not even 9 months - and less than 
4 month since 4.6.0 which was the first GCC release with the bug. You 
cannot expect that every user uses unstable versions or immediately 
updates to a released version. Having someone using 4.6.0 after less 
than 4 months is already very timely.

Additionally, as mentioned before, except for NAG all 7 of the compilers 
I tested plus gfortran 4.1 to 4.6 work (with 4.4 to 4.6 failing after 
2010-10-26/2010-11-03). Furthermore, it is documented in gfortran's 
manual and it is not a very invasive patch.

Tobias

* For instance namelist_24.f90

On 07/20/2011 03:30 PM, Tobias Burnus wrote:
> This patch partially undoes the change done in PR 46010.
>
> For that patch, first (comment 7)
>
> + || !dtp->u.p.ionml->touched)
>
> was added - and after not solving the problem, the complete change was (comment
> 9 and committed version)
>
> + || !dtp->u.p.ionml->touched
> + || dtp->u.p.ionml->type == BT_DERIVED)
>

The "touched" is not needed and in fact if we are not reading derived types, it 
is not relevant. I should have taken it out in the first place and I suspect it 
tripped up the test case because "touched" uninitialized is set to zero.

I would say OK for trunk and then backport in a few weeks.  If you feel like 
being conservative you could use

|| (dtp->u.p.ionml->type == BT_DERIVED && !dtp->u.p.ionml->touched)) but I do 
not think it is necessary.

Regards,

Jerry