Bug 30146

Summary: Redefining do-variable in excecution cycle
Product: gcc Reporter: Tobias Burnus <burnus>
Component: fortranAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: enhancement CC: dfranke, gcc-bugs, P.Schaffnit, tkoenig
Priority: P3 Keywords: diagnostic
Version: 4.3.0   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2007-01-22 21:29:40

Description Tobias Burnus 2006-12-11 14:05:08 UTC
I think the following program is wrong as one uses a passes a do variable to an intent(inout) (or intent(out) subroutine).

However, neither g95, sunf95 nor NAG f95 complain at compile time. Using nagf95 -C=all I get at run time:
Actual argument for INTENT(INOUT) dummy argument I is an expression
(I get this whether or not I set "i" in the suboutine.)

Am I missing something?

From the standard ("8.1.6.4.2 The execution cycle"):
"Except for the incrementation of the DO variable that occurs in step (3), the DO variable shall neither be redefined nor become undefined while the DO construct is active."

Once could implement this by setting a flag to the do-variable on for the excecution cycle, and removing the flag on exit. Then one could check the result in resolve.c. Or look at gfc_check_do_variable in parse.c.


I don't know whether the output should be a warning or an error, but the following program runs "2 4 6" with nagf95, ifort and sunf95, "2 4 6 8 10" with g95 and "2 4 ... 142434 ..." with gfortran.


program main
  implicit none
  integer :: i
  do i  = 1,5,1
    call mysub(i)
    print *, i
  end do
contains
  subroutine mysub(i)
   integer,intent(inout) :: i
   i = i + 1
  end subroutine mysub
end program main
Comment 1 Tobias Schl├╝ter 2006-12-17 20:56:47 UTC
As long as the callee doesn't change the value it's probably valid, so this would need some kind of interprocedural analysis to give a correct error. A warning may be in place, though. This shouldn't be to hard, we check that LHS' are not DO variables, so similar code is already there.
Comment 2 Tobias Burnus 2006-12-17 21:20:13 UTC
> As long as the callee doesn't change the value it's probably valid, so this
> would need some kind of interprocedural analysis to give a correct error.

I think it is formally seen invalid as you have INTENT([IN]OUT) set.

Actually, I just retested with NAG f95 and it gives:
Error: test.f90, line 5: Active DO variable I passed to INTENT(OUT) argument I number 1 of MYSUB

We could nontheless consider of giving a warning only, if you think the case of INTENT(OUT) with no assignment is much more common than a wrongly used do-variable.

By the way, if no value is assigned to an INTENT(OUT) variable, gfortran should follow the other compilers and issue a warning:
"INTENT(OUT) dummy argument I never set"
"INTENT(OUT) variable 'i' at (1) is never set"
"A dummy argument with an explicit INTENT(OUT) declaration is not given an explicit value."
Comment 3 Michael Richmond 2007-03-21 16:15:11 UTC
The following program causes an infinite loop in gfortran, but not in other fortrans.

PROGRAM do_index_in_call
DO i = 1, 1
CALL SUB(i)
ENDDO
END PROGRAM do_index_in_call
SUBROUTINE sub(i)
i = 2
END
Comment 4 Tobias Burnus 2007-03-21 19:07:41 UTC
(In reply to comment #3)
> The following program causes an infinite loop in gfortran, but not in other
> fortrans.

The program is plainly invalid as one redefines the DO thus your Fortran compiler is free to do what ever it wants. In addition, your standard violation cannot be detected at compile time.

gfortran implements it as:
  while(1) {
   if(i == 1) break;
  }
This of cause fails if you change i. As the program is invalid and as I don't see any possibility to fix this in gfortran, I regard the problem of comment 3 as INVALID/WONTFIX.

For the original example, comment 0, I still think outputting a warning or an error would be ok. (difference: the original example uses "INTENT(OUT)", the comment 3 example uses no INTENT - besides, an interface or a host/use association would be needed too.)
Comment 5 Daniel Franke 2009-04-09 16:03:55 UTC
(In reply to comment #4)
> This of cause fails if you change i. As the program is invalid and as I don't
> see any possibility to fix this in gfortran, I regard the problem of comment 3
> as INVALID/WONTFIX.

In the meantime we got -fwhole-file to check for interface mismatches. The subroutine sub() in #3 does not specify INTENT for its dummy argument 'i', so it's INOUT by default. Thus, if this code is compiled with -fwhole-file, gfortran can/should raise an error?!

 * * *

I'm unsure how to implement this. The check for assignments to the loop-variable, e.g.
  DO i = 1, ...
    i = <expr>
  END DO
is done during matching, using gfc_state_stack (see parse.c, gfc_check_do_variable). This is by far too early for argument checking.

Generally, checks for INTENTs are done in interface.c(check_intents), called by interface.c (gfc_procedure_use). This seems to be the place to add the check for loop-variables. However, as it is valid to use the same variable as a loop-variable and not, e.g.
  INTEGER :: i
  CALL set(i, 3)         ! valid
  DO i = ...
    CALL set(i, 3)       ! invalid
  END DO
  CALL set(i, 3)         ! valid
  CONTAINS
    SUBROUTINE set(j, n)
      INTEGER, INTENT(out) :: j
      INTEGER, INTENT(in)  :: n
      j = n
    END SUBROUTINE

we can not unconditionally add an attribute, say, is_loop_variable, to the symbol of 'i' as it sometimes is not. One could temporarily flag the symbol as loop-variable in resolve.c (gfc_resolve_iterator), but I'm lost where/how to reset this flag afterwards.

Thoughts? Pointers?
Comment 6 Thomas Koenig 2012-11-25 17:24:14 UTC
Author: tkoenig
Date: Sun Nov 25 17:24:09 2012
New Revision: 193793

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193793
Log:
2012-11-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/30146
	* frontend-passes.c (doloop_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-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

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


Added:
    trunk/gcc/testsuite/gfortran.dg/do_check_7.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/frontend-passes.c
    trunk/gcc/testsuite/ChangeLog
Comment 7 Thomas Koenig 2012-11-25 18:29:00 UTC
I think this can be counted as fixed with the addition
of -fcheck=do and the recent fixes for INTENT(OUT) and
INTENT(INOUT) dummy arguments.

Please reopen if you think othrwise.