[Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Daniel Kraft]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

Hi,

as suggested by Tobias, this patch relaxes the "used before typed" check 
to allow code like this:

SUBROUTINE (n)
   INTEGER :: arr(n + 1)
   INTEGER :: n

which is strictly speaking invalid but used in real world code and poses 
no problem to implement.  At the moment, a slightly stricter variant is 
implemented in gfortran which allows only arr(n) (in -std=gnu) for 
not-yet-typed n, and this patch extends this permission for basic 
arithmetic expressions like the one above.

Regression test running on GNU/Linux-x86-32.  Ok for trunk if no 
regressions?

Yours,
Daniel

-- 
Done:  Arc-Bar-Cav-Rog-Sam-Val-Wiz
To go: Hea-Kni-Mon-Pri-Ran-Tou

[-- Attachment #2: patch.changelog --]
[-- Type: text/plain, Size: 304 bytes --]

2008-10-12  Daniel Kraft  <d@domob.eu>

	PR fortran/37688
	* expr.c (gfc_expr_check_typed): Extend permission of untyped
	expressions to both top-level variable and basic arithmetic expressions.

2008-10-12  Daniel Kraft  <d@domob.eu>

	PR fortran/37688
	* gfortran.dg/used_before_typed_6.f90: New test.

[-- Attachment #3: patch.diff --]
[-- Type: text/plain, Size: 2570 bytes --]

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 141001)
+++ gcc/fortran/expr.c	(working copy)
@@ -3429,9 +3429,11 @@ gfc_expr_set_symbols_referenced (gfc_exp
 
 /* Walk an expression tree and check each variable encountered for being typed.
    If strict is not set, a top-level variable is tolerated untyped in -std=gnu
-   mode; this is for things in legacy-code like:
+   mode as is a basic arithmetic expression using those; this is for things in
+   legacy-code like:
 
      INTEGER :: arr(n), n
+     INTEGER :: arr(n + 1), n
 
    The namespace is needed for IMPLICIT typing.  */
 
@@ -3458,9 +3460,26 @@ gfc_expr_check_typed (gfc_expr* e, gfc_n
 {
   bool error_found;
 
-  /* If this is a top-level variable, do the check with strict given to us.  */
-  if (!strict && e->expr_type == EXPR_VARIABLE && !e->ref)
-    return gfc_check_symbol_typed (e->symtree->n.sym, ns, strict, e->where);
+  /* If this is a top-level variable or EXPR_OP, do the check with strict given
+     to us.  */
+  if (!strict)
+    {
+      if (e->expr_type == EXPR_VARIABLE && !e->ref)
+	return gfc_check_symbol_typed (e->symtree->n.sym, ns, strict, e->where);
+
+      if (e->expr_type == EXPR_OP)
+	{
+	  gfc_try t = SUCCESS;
+
+	  gcc_assert (e->value.op.op1);
+	  t = gfc_expr_check_typed (e->value.op.op1, ns, strict);
+
+	  if (t == SUCCESS && e->value.op.op2)
+	    t = gfc_expr_check_typed (e->value.op.op2, ns, strict);
+
+	  return t;
+	}
+    }
 
   /* Otherwise, walk the expression and do it strictly.  */
   check_typed_ns = ns;
Index: gcc/testsuite/gfortran.dg/used_before_typed_6.f90
===================================================================
--- gcc/testsuite/gfortran.dg/used_before_typed_6.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/used_before_typed_6.f90	(revision 0)
@@ -0,0 +1,20 @@
+! { dg-do compile }
+! { dg-options "-std=gnu" }
+
+! Allow legacy code to work even if not only a single symbol is used as
+! expression but a basic arithmetic expression.
+
+SUBROUTINE test (n, m)
+  IMPLICIT NONE
+
+  ! These should go fine.
+  INTEGER :: arr1(n + 1) ! { dg-bogus "used before it is typed" }
+  INTEGER :: arr2(n / (2 * m**5)) ! { dg-bogus "used before it is typed" }
+
+  ! These should fail for obvious reasons.
+  INTEGER :: arr3(n * 1.1) ! { dg-error "must be of INTEGER type" }
+  INTEGER :: arr4(REAL (m)) ! { dg-error "used before it is typed" }
+  INTEGER :: arr5(SIN (m)) ! { dg-error "used before it is typed" }
+
+  INTEGER :: n, m
+END SUBROUTINE test

Re: [Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Tobias Burnus]

Daniel Kraft wrote:
> Regression test running on GNU/Linux-x86-32.  Ok for trunk if no
> regressions?
OK. Thanks for the patch.

Tobias

Re: [Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Paul Richard Thomas]

Dear All,

Which other compilers accept this feature and why is it so
unreasonable to place declarations in standard conforming order?

I am completely against bending the standard on the grounds that
something is "used in real world code".   As very least, do a standard
check on it and preferably don't do it at all.

Paul

On Sun, Oct 12, 2008 at 11:25 AM, Tobias Burnus <burnus@net-b.de> wrote:
> Daniel Kraft wrote:
>> Regression test running on GNU/Linux-x86-32.  Ok for trunk if no
>> regressions?
> OK. Thanks for the patch.
>
> Tobias
>

Re: [Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Daniel Kraft]

Paul,

> Which other compilers accept this feature and why is it so
> unreasonable to place declarations in standard conforming order?
> 
> I am completely against bending the standard on the grounds that
> something is "used in real world code".   As very least, do a standard
> check on it and preferably don't do it at all.

I agree here, but I also believe the patch in question is a "reasonable" 
extension to the "used before typed" permission already there; on the 
other hand, however, I think we could get rid of it entirely, but I'm 
not a user of any legacy code and at the time we worked that one out, I 
got the impression that at least

INTEGER :: arr(n), n

is something used over and over that should be accepted by all means 
(but maybe my impression was wrong).  Given this invalid code is already 
accepted, I think it is better to extend this permission like done here 
to accept even more legacy code (if possible without other problems) (or 
disable the permission entirely).

I'm not sure what you mean by the standard check, but with -std=f95 and 
friends this code is of course rejected, both now and with the patch.

Thanks for your comments though, as a non-legacy-code user I base my 
opinion there just on input I get from all the others ;)

Daniel

-- 
Done:  Arc-Bar-Cav-Rog-Sam-Val-Wiz
To go: Hea-Kni-Mon-Pri-Ran-Tou

Re: [Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Paul Richard Thomas]

Daniel,


>
> I'm not sure what you mean by the standard check, but with -std=f95 and
> friends this code is of course rejected, both now and with the patch.

Ah, does it:-)  OK then!

> Thanks for your comments though, as a non-legacy-code user I base my opinion
> there just on input I get from all the others ;)

You are welcome.  Having just spent some time on getting one bit of
fortran compliant with the needs of restricted expressions, I did not
want us to drift off somewhere else!

Cheers

Paul

Re: [Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Daniel Kraft]

Tobias Burnus wrote:
> Daniel Kraft wrote:
>> Regression test running on GNU/Linux-x86-32.  Ok for trunk if no
>> regressions?
> OK. Thanks for the patch.

No regressions, committed to trunk as rev. 141074.

Paul, I took your last comment as that's ok from your point of view, 
too.  But I'm of course open for further discussions and it would not be 
hard to remove this exceptions entirely should we decide to do so in the 
future!

Thanks,
Daniel

-- 
Done:  Arc-Bar-Cav-Rog-Sam-Val-Wiz
To go: Hea-Kni-Mon-Pri-Ran-Tou

Re: [Patch, Fortran] PR fortran/37688: Relax "used before typed" check

[Tobias Burnus]

Paul Richard Thomas wrote:
> Which other compilers accept this feature
g95 (-std=f95), ifort (-stand f95), openf95, sunf95, gfortran (-std=f95
-pedantic) 4.1, 4.2 and 4.3 [but not 4.4].

> and why is it so unreasonable to place declarations in standard
> conforming order?
I can tell you why it happens: Assuming a

   subroutine mysubroutine (x, y , n)

then it is natural to declare the variables in the order (x, y, n)
instead of (n, x, y) or (n, y, x). And it is quite natural to have the
order <array>, <size> in the argument list - after all one writes A(n)
in programs or  A \in Mat(n x n) in mathematics. If I look at LAPACK it
also has:

   subroutine dgeqrf(m,n,A,lda,Tau,Work,lwork,info)

where lda is used in A(lda,n) and lwork by work(lwork). (Well, the
original LAPACK declaration uses "*" everywhere).

> I am completely against bending the standard on the grounds that
> something is "used in real world code". As very least, do a standard
> check on it and preferably don't do it at all.
The standard check was added by Daniel's initial patch for 4.4 did. With
-std=f* even

  implicit none
  real :: A(n)
  integer :: n

gets rejected. (That is what the "strict" checking is for; gfortran <
4.4 had no such check at all.)


In general I would like that all programs are compilable which can be
compiled by several other compilers (either by default or as e.g. for
-f[no-]range-check with an error message which points to that option)
and I would like to have good diagnostics either as warning or (esp.
with -std=f*) as error if something is invalid according to the Fortran
standard. The former is useful for users of code, which just want to get
it running; the latter is useful for people developing Fortran programs.
-- Actually, best would be a stricter checks for developers and laxer
for mere users, but detecting this automatically is not so easy. ;-)
For Fortran 2003/2008 features one can be stricter as there is not so
much legacy code around and gfortran has enough market share to push
developers to standard conforming code (e.g. gfortran does not allow len
> 1 for character dummy arguments of bind(C) procedures, which is
required by the standard but not in line with other compilers).

I think the patch is thus reasonable, but I don't feel strong about it.

Tobias