*ping* [patch, fortran] PR 30146, errors for INTENT(OUT) and INTENT(INOUT) for DO loop variables

[Thomas Koenig]

Hi Steven,

> On Sat, Nov 10, 2012 at 3:00 PM, Thomas Koenig wrote:
>> I wrote:
>>
>>> after the dicsussion on c.l.f, it become clear that passing a DO loop
>>> variable to an INTENT(OUT) or INTENT(INOUT) dummy argument is an error.
>>> The attached patch throws an error for both cases.
>
> But should we really isse an error for INTENT(INOUT)? IMHO a warning
> suffices, with maybe an error only for strict (i.e. non-GNU) standard
> settings.

This was the result of a discussion on c.l.f.  The summary can be found
http://groups.google.com/group/comp.lang.fortran/msg/7107f24b8980fad3?hl=de

Basically, passing an index variable to an INTENT(INOUT) variable
violates a requirement on the program, and than an error would be
the best course of action.

>>> I chose to issue the errors as a front-end pass because we cannot check
>>> for formal arguments during parsing (where the other checks are
>>> implemented).
>>>
>>> Regression-tested.  OK for trunk?
>>
>>
>> Ping ** 1.4285 ?
>
> You don't have to list do_list twice in the ChangeLog, you probably
> wanted one of those to be do_level ;-)

OK.

>
>>> +  do_list = XNEWVEC(gfc_code *, do_size);
>
> Taste nit: Why not just toss do_list, do_level, and do_size around as
> a function argument, instead of making them global variable? Just
> define a struct containing them and pass it around via the "data"
> argument for gfc_code_walker should work, I think.

The problem is with do_level.  This could be incremented in do_warn,
but we only know when to decrement it in gfc_code_walker (because there
is no EXEC_ENDDO). So, we need a static variable in any case.

The rest is a question of taste. If we need one static variable, I think
we might as well use some other static variables.  The only alternative
I thought about was using a VEC, but frankly the documentation on that
left me baffled as to how to implement this.


> IMHO names like "do_warn" and "do_list" are not very descriptive, if
> not to say confusing. do_* names are used elsewhere in the compiler
> for functions that perform ("do") a task, whereas your do_* functions
> are for the Fortran DO construct. I'd prefer different names.

Changed to doloop_*.

>
>>> +   to an INTENt(OUT) or INTENT(INOUT) dummy variable.  */
>
> s/INTENt/INTENT/

Fixed.

>
>>> +  /* Withot a formal arglist, there is only unknown INTENT,
>
> s/Withot/Without/
>
>
>>> +      for (i=0; i<do_level; i++)
>
> for (i = 0; i < do_level; i++)
>
>
>>> +			      "inside loop  beginning at %L as INTENT(OUT) "
>
> Extraneous space after loop.

Fixed.

> How do you handle OPTIONAL args?

As far as I have been able to determine, they work:

ig25@linux-fd1f:~/Krempel/Do> cat optional.f90
module opt
   implicit none
contains
   subroutine opt_in(a,b)
   integer, intent(in), optional :: a
   integer, intent(out) :: b
   end subroutine opt_in
end module opt
program main
   use opt
   implicit none
   integer :: i
   do i=1,10
     call opt_in(b=i)
   end do
end program main
ig25@linux-fd1f:~/Krempel/Do> gfortran optional.f90
optional.f90:14.18:

     call opt_in(b=i)
                   1
optional.f90:13.11:

   do i=1,10
            2
Fehler: Variable 'i' at (1) set to undefined value inside loop 
beginning at (2) as INTENT(OUT) argument to subroutine 'opt_in'

Or were you thinking of another case?

Attached is the new version of the patch, regression-tested.

Thanks for the review!

OK for trunk?

	Thomas

2012-11-11  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/30146
         * frontend-passes.c (do_warn):  New function.
         (doloop_list):  New static variable.
         (doloop_size):  New static variable.
         (doloop_level):  New static variable.
         (gfc_run_passes): Call doloop_warn.
         (doloop_code):  New function.
         (doloop_function):  New function.
         (gfc_code_walker):  Keep track of DO level.

2012-11-11  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/30146
         * gfortran.dg/do_check_6.f90:  New test.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: p6.diff
Type: text/x-patch
Size: 5838 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20121111/90ff8906/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: do_check_6.f90
Type: text/x-fortran
Size: 1161 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20121111/90ff8906/attachment-0001.bin>