Message ID | 4E27571D.6030007@net-b.de |
---|---|
State | New |
Headers | show |
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
2011-07-21 Tobias Burnus <burnus@net-b.de> PR fortran/49791 * io/list_read.c (nml_parse_qualifier): Remove check to enabled extended read for another case. 2011-07-21 Tobias Burnus <burnus@net-b.de> PR fortran/49791 * gfortran.dg/namelist_72.f: New. diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c index c88f232..01272d0 100644 --- a/libgfortran/io/list_read.c +++ b/libgfortran/io/list_read.c @@ -2213,7 +2213,6 @@ nml_parse_qualifier (st_parameter_dt *dtp, descriptor_dimension *ad, do not allow excess data to be processed. */ if (is_array_section == 1 || !(compile_options.allow_std & GFC_STD_GNU) - || !dtp->u.p.ionml->touched || dtp->u.p.ionml->type == BT_DERIVED) ls[dim].end = ls[dim].start; else --- /dev/null 2011-07-19 07:59:35.374731880 +0200 +++ gcc/gcc/testsuite/gfortran.dg/namelist_72.f 2011-07-21 00:10:23.000000000 +0200 @@ -0,0 +1,33 @@ +! { dg-do run } +! +! PR fortran/49791 +! +! Contributed by Elliott Sales de Andrade +! + program namelist_test + + dimension xpos(5000), ypos(5000) + namelist /geometry/ xpos, ypos + + xpos = -huge(xpos) + ypos = -huge(ypos) + + open(unit=4,file='geometry.in') + write(4,'(a)') '$geometry' + write(4,'(a)') ' xpos(1)= 0.00, 0.10, 0.20, 0.30, 0.40,' + write(4,'(a)') ' ypos(1)= 0.50, 0.60, 0.70, 0.80, 0.90,' + write(4,'(a)') '$end' + + close(4) + + open (unit=4,file='geometry.in',status='old',form='formatted') + read (4,geometry) + close(4, status='delete') + + !print *, 'xpos', xpos(1:10), 'ypos', ypos(1:10) + + if (any (xpos(1:5) /= [0.00, 0.10, 0.20, 0.30, 0.40]))call abort() + if (any (ypos(1:5) /= [0.50, 0.60, 0.70, 0.80, 0.90]))call abort() + if (any (xpos(6:) /= -huge(xpos))) call abort () + if (any (ypos(6:) /= -huge(ypos))) call abort () + end