Re: [fortran, 4.5] Implement errmsg for [de]allocate

[Tobias Burnus <burnus at net-b dot de>]

Steve Kargl wrote:
> On Wed, Dec 03, 2008 at 12:07:09PM -0800, Steve Kargl wrote:
>   
>> On Wed, Dec 03, 2008 at 08:28:48PM +0100, Tobias Burnus wrote:
>>     
>>> Steve Kargl wrote:
>>>       
>>>> The attached patch implements the F2003 feature of ERRMSG= in
>>>> the [DE]ALLOCATE statements.  The current patch makes use of
>>>> the infrastructure for STAT= to determine if a failure occurs.
>>>>         
The patch looks OK. Can you make sure that PRs for the missing bits exist?
Thanks for the patch!

Tobias


>>>>   
>>>>         
>>> Somehow it does not work if one additionally requests a STAT= variable:
>>>
>>>   deallocate(x, stat=i, errmsg=err)
>>>                      1
>>> Error: Syntax error in DEALLOCATE statement at (1)
>>>
>>> (Ditto for: errmsg=..., stat=...)
>>>       
>> I may have misinterpreted the 'or' in the BNF:
>>
>> R623 allocate-stmt is ALLOCATE ( [ type-spec :: ] allocation-list
>>                         [, alloc-opt-list ] )
>> R624 alloc-opt     is STAT = stat-variable
>>                    or ERRMSG = errmsg-variable
>>                    or SOURCE = source-expr
>>
>> alloc-opt-list is not defined but C630 suggests that a combination of
>> the alloc-opts can appear.
>>
>> C630 (R624) No alloc-opt shall appear more than once in a given alloc-opt-list.
>>
>> It's easy to fix.  I'll work out it this weekend.
>>
>>     
>>> Additionally: Can we add a -Wsurprising which checks for the presence of
>>> STAT= when ERRMSG= is given? NAG f95 prints:
>>>
>>> Warning: hfjgf.f90, line 6: ERRMSG= is useless without STAT=
>>>
>>>       
>>>>   deallocate(x, x, errmsg=err)
>>>>   print *, err
>>>>         
>>> There are several issues:
>>> a) "deallocate(x, x)" is invalid, which can be compile-time diagnosable
>>> (as e.g. ifort does), better place it in two lines
>>>       
>> OK.  gfortran doesn't currently catch at compile time.
>>
>> REMOVE:kargl[125] gfc4x -o z b.f90
>> REMOVE:kargl[126] ./z
>> At line 4 of file b.f90
>> Fortran runtime error: Attempt to DEALLOCATE unallocated 'i'
>> REMOVE:kargl[127] cat b.f90
>> program a
>>    integer, allocatable :: i(:)
>>    allocate(i(4))
>>    deallocate(i, i)
>> end program a
>>
>>     
>>> b) "print *, err" is invalid - it always prints uninitialized value (in
>>> case of success). In case of a failure, it does not print anything as
>>> DEALLOCATE aborts
>>>       
>> The code in the email was a quick hack to show the current error
>> message.  The testcase is completely legal.
>>
>>     
>>> "If an error condition occurs during execution of an ALLOCATE statement
>>> that does not contain the STAT= specifier, execution of the program is
>>> terminated."
>>>       
>> Hmmm, that's going to complicate the trans-*c code.  In fact, this
>> pretty makes errmsg useless without stat, so your -Wsurprising
>> should probably be a plain old warning.
>>
>>     
>>> I will review the patch itself later.
>>>
>>>       
>> Wait for an update.  I have a 5 hour flight home
>> tomorrow night.
>>     
>
> It took a little longer than 5 hours. :(
>
> The attached patch addresses many of the issues raised by Tobias
> as well as some that I've identified while reading the F2003
> standard.  Specifically, this patch implements the F2003 feature
> ERRMSG in the ALLOCATE and DEALLOCATE statments, and it adds
> additional compile time error checking for allocate-objects,
> stat-variable, and errmsg-variable.
>
> Note, this patch does not implement checking of allocate-objects,
> stat-variable, and errmsg-variable if these involve a derived type.
> For example, 
>
>    program c
>    type a
>      integer, allocatable :: i(:)
>    end type a
>    type(a) b
>    allocate(b%i(2))
>    deallocate(b%i, b%i)  ! Invalid, not diagnosed.
>    end program c
>
> troutmask:sgk[224] gfc4x -o z c.f90
> troutmask:sgk[225] ./z
> At line 10 of file c.f90
> Fortran runtime error: Attempt to DEALLOCATE unallocated 'b'
>
> This is the behavior one sees in 4.2.x, 4.3.x, and trunk.
>
> There is also an issue with array references in STAT= and
> ERRMSG= variables.  Consider,
>
>   program b
>   integer, allocatable :: i(:)
>   integer :: k(3) = 42
>   deallocate(i, stat=k)    ! Invalid, not diagnosed. Causes ICE!
>   print '(3(I0,1X))', k
>   deallocate(i, stat=k(3)) ! Valid
>   print '(3(I0,1X))', k
>   end program
>
> troutmask:sgk[230] gfc4x -o z b.f90
> b.f90: In function 'b':
> b.f90:1: internal compiler error: Segmentation fault: 11
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>
> Again, this is the behavior one sees in 4.2.x, 4.3.x, and trunk.
>
> These issues will require walking data structures and would
> make the 863 line diff much larger and harder to review (ie.,
> a multi-dimensional array of a nested derived type is going
> to be nasty).
>
> This patch also does not implement the parsing of the SOURCE=
> alloc-opt for the ALLOCATE() statement.  Given the number
> of constraints involving SOURCE, the addition of an optional
> type spec, the fact that gfc_code needs to grow another member,
> and the length of the current diff, I felt implementation of
> SOURCE should be done in a self-contained patch.
>
> Regression tests run on x86_64-unknown-freebsd8.0
>
>                 === gfortran Summary ===
>
> # of expected passes            28976
> # of unexpected successes       31
> # of expected failures          19
> # of unsupported tests          42
>
>
> 2008-12-10  Steven G. Kargl  <kargls@comcast.net>
>
> 	* gfortran.dg/alloc_alloc_expr_1.f90: Adjust for new error message.
> 	* gfortran.dg/allocate_alloc_opt_1.f90: New test.
> 	* gfortran.dg/allocate_alloc_opt_2.f90: Ditto.
> 	* gfortran.dg/allocate_alloc_opt_3.f90: Ditto.
> 	* gfortran.dg/deallocate_alloc_opt_1.f90: Ditto.
> 	* gfortran.dg/deallocate_alloc_opt_2.f90: Ditto.
> 	* gfortran.dg/deallocate_alloc_opt_3.f90: Ditto.
>
>
> 2008-12-10  Steven G. Kargl  <kargls@comcast.net>
>
> 	* trans-stmt.c(gfc_trans_allocate): Add translation of ERRMSG.
> 	(gfc_trans_deallocate): Add translation of ERRMSG.  Remove stale
> 	comments.  Minor whitespace cleanup.
> 	* resolve.c(is_scalar_expr_ptr): Whitespace cleanup.
> 	(resolve_deallocate_expr (gfc_expr *e): Update error message.
> 	(resolve_allocate_expr):  Remove dead code.  Update error message.
> 	Move error checking to ...
> 	(resolve_allocate_deallocate): ... here.  Add additional error
> 	checking for STAT, ERRMSG, and allocate-objects.
> 	* match.c(gfc_match_allocate,gfc_match_deallocate):  Parse ERRMSG.
> 	Check for redundant uses of STAT and ERRMSG.  Reword error message
> 	and add checking for pointer, allocatable, and proc_pointer attributes.
>
>