Bug 36325 - specific or generic INTERFACE implies the EXTERNAL attribute
specific or generic INTERFACE implies the EXTERNAL attribute
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: fortran
4.4.0
: P3 normal
: ---
Assigned To: janus
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-25 13:35 UTC by Janus Weil
Modified: 2008-05-28 21:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-05-25 15:36:59


Attachments
patch (2.85 KB, patch)
2008-05-25 19:08 UTC, janus
Details | Diff
patch version 2 (4.60 KB, patch)
2008-05-26 18:44 UTC, janus
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Janus Weil 2008-05-25 13:35:31 UTC
I think the following code is invalid:

interface
  subroutine foo
  end subroutine
end interface
external foo

Because the INTERFACE statement already specifies the EXTERNAL attribute, which is thus specified twice.

This code *is* actually rejected (as of rev. 135859), but the error message is completely wrong:

external :: foo
              1
Error: EXTERNAL attribute conflicts with SUBROUTINE attribute at (1)

(which it does *not* for this case)


Quoting the Fortran 2003 standard (section 5.1.2.6):

"The EXTERNAL attribute specifies that an entity is an external procedure, dummy procedure, procedure pointer, or block data subprogram. This attribute may also be specified by an EXTERNAL statement (12.3.2.2), a procedure-declaration-stmt (12.3.2.3) or an interface body that is not in an abstract interface block (12.3.2.1)."

And further on in section 12.3.2.1:

"An interface body in a generic or specific interface block specifies the EXTERNAL attribute and an explicit specific interface for an external procedure or a dummy procedure. If the name of the declared procedure is that of a dummy argument in the subprogram containing the interface body, the procedure is a dummy procedure; otherwise, it is an external procedure."
Comment 1 Janus Weil 2008-05-25 14:02:23 UTC
Here is a first patch:

Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 135859)
+++ gcc/fortran/symbol.c	(working copy)
@@ -439,7 +439,7 @@ check_conflict (symbol_attribute *attr, 
   conf (external, intrinsic);
   conf (entry, intrinsic);
 
-  if ((attr->if_source && !attr->procedure) || attr->contained)
+  if ((attr->if_source == IFSRC_DECL && !attr->procedure) || attr->contained)
     {
       conf (external, subroutine);
       conf (external, function);
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 135859)
+++ gcc/fortran/parse.c	(working copy)
@@ -1917,12 +1917,26 @@ loop:
       new_state = COMP_SUBROUTINE;
       gfc_add_explicit_interface (gfc_new_block, IFSRC_IFBODY,
 				  gfc_new_block->formal, NULL);
+      if (current_interface.type != INTERFACE_ABSTRACT &&
+	 gfc_add_external (&gfc_new_block->attr, &gfc_current_locus) == FAILURE)
+	{
+	  reject_statement ();
+	  gfc_free_namespace (gfc_current_ns);
+	  goto loop;
+	}
       break;
 
     case ST_FUNCTION:
       new_state = COMP_FUNCTION;
       gfc_add_explicit_interface (gfc_new_block, IFSRC_IFBODY,
 				  gfc_new_block->formal, NULL);
+      if (current_interface.type != INTERFACE_ABSTRACT &&
+	 gfc_add_external (&gfc_new_block->attr, &gfc_current_locus) == FAILURE)
+	{
+	  reject_statement ();
+	  gfc_free_namespace (gfc_current_ns);
+	  goto loop;
+	}
       break;
 
     case ST_PROCEDURE:


This removes the conflict between EXTERNAL and SUBROUTINE for this case, and adds the EXTERNAL attribute for a non-abstract INTERFACE statement.

Checking for regressions ...
Comment 2 Janus Weil 2008-05-25 14:45:42 UTC
Ok, this produces an impressive list of regressions.

Many of those (e.g. actual_procedure_1.f90) seem to be related to

  conf (external, dimension);   /* See Fortran 95's R504.  */

I'm not sure if the constraint from R504 is implemented correctly here, or if it constrains too much.


Others testcases (like argument_checking_3.f90) fail because they define lots of specific interfaces, but no external implementation for those.
So I guess they are actually invalid?
Comment 3 Tobias Burnus 2008-05-25 15:36:58 UTC
> Ok, this produces an impressive list of regressions.
> Many of those (e.g. actual_procedure_1.f90) seem to be related to
>   conf (external, dimension);   /* See Fortran 95's R504.  */
> I'm not sure if the constraint from R504 is implemented correctly here, or if
> it constrains too much.

As written, I believe that

R504 entity-decl is object-name [( array-spec )] [ * char-length ]
                                [initialization ]
                 or function-name [ * char-length ]

only restricts the use of:
  REAL, EXTERNAL :: func(10)
                        °°°°<- array-spec, invalid for function names
but not (a)

  REAL, EXTERNAL, DIMENSION(10) :: func

nor (b) the dimension of function results.

Despite my failure to find anything in the standard which rejects (a) all my compilers reject it whereas all my compilers allow (b).

See also:
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/bb371413b5cbe3d7


> Others testcases (like argument_checking_3.f90) fail because they define lots
> of specific interfaces, but no external implementation for those.
> So I guess they are actually invalid?

As I did not apply your patch, I fail to understand this problem. Ignoring the problems with the rank mismatches and too few arguments, the program is valid from the Fortran side. External means that the user somehow needs to provide the foo,bar,foobar procedures, but that is outside of the scope of the Fortran standard (except that also Fortran procedures can be external procedures). In case of gfortran the linker would complain - but it should never reach that point because of the compile-time errors.

By the way: The the rank mismatch dg-error and not dg-warning should be used in argument_checking_3.f90. (However, dejagnu does not distingish.)
Comment 4 janus 2008-05-25 16:37:06 UTC
(In reply to comment #2)
> Others testcases (like argument_checking_3.f90) fail because they define lots
> of specific interfaces, but no external implementation for those.

Obviously I got this wrong. The actual reason for these tests failing is that with my patch all the interfaces acquire the EXTERNAL attribute (which they didn't have before), and apparently there is no argument checking done for external procedures. Which leads me to think we should probably implement this (for the case that the interface is explicit). Or is there any good reason that this is not done?
Comment 5 Tobias Burnus 2008-05-25 17:11:47 UTC
> Which leads me to think we should probably implement this (for the case that
> the interface is explicit). Or is there any good reason that this is not done?

Well, regarding the reason: Before interfaces had no EXTERNAL attribute and those procedures with EXTERNAL attribute had no explicit interface.

That interfaces have the EXTERNAL attribute only needed for procedure pointers. One could think of not giving the EXTERNAL attribute to procedures declared in interface bodies and modifying POINTER / the pointer resolution only. I'm not sure what is cleaner and simpler.

Regarding our questions, Richard Maine (mostly) answered them. See URL in comment #3.

  conf2(external,dimension)
is in any case wrong. It should be:
  conflict(external with implicit interface, dimension)
Comment 6 janus 2008-05-25 19:08:35 UTC
Created attachment 15684 [details]
patch

Ok, I extended the patch, and got the regression count down from a few million to exactly two:

FAIL: gfortran.dg/proc_decl_9.f90
FAIL: gfortran.dg/gomp/reduction3.f90

What I did was e.g. following Tobi's suggestion about the dimension problem:

>   conf2(external,dimension)
> is in any case wrong. It should be:
>   conflict(external with implicit interface, dimension)

Also I made sure not to interfere with dummy procedures, turned on argument checking for (explicit-interfaced) external procedures and fixed an error message for a test case.

I haven't looked at the two remaining regressions in detail, but both seem to be related to intrinsics. And I'm sure I can convince those two little fuckers to disappear soon.

Patch is attached.
Comment 7 Tobias Burnus 2008-05-25 19:58:37 UTC
> Patch is attached.

You need also to reject the following, which violates R504.

interface
  real function bar()
  end function bar
end interface
dimension :: bar(4)
end
Comment 8 janus 2008-05-25 22:33:16 UTC
The failure of proc_decl_9.f90 was actually due to a bug that slipped in with my procedure declaration update patch from May 1st, which I have fixed now.

So we're left with gomp/reduction3.f90, which contains this piece of code:

  interface
    function ior (a, b)
      integer :: ior, a, b
    end function
  end interface
  intrinsic ior

This produces:

  intrinsic ior
              1
Error: EXTERNAL attribute conflicts with INTRINSIC attribute at (1)

I haven't checked the standard on this, but I bet the code is illegal.
And after all: Why should one declare an explicit interface for an intrinsic (whose interface is known anyway) ...?
Comment 9 Tobias Burnus 2008-05-26 16:49:25 UTC
(In reply to comment #8)
> So we're left with gomp/reduction3.f90, which contains this piece of code:
>   interface
>     function ior (a, b)

>   intrinsic ior
> 
> This produces:
> Error: EXTERNAL attribute conflicts with INTRINSIC attribute at (1)


"C518 An entity shall not have both the EXTERNAL attribute and the INTRINSIC attribute."

For INTRINSIC one finds also the following, which lifts a related restriction.

"A name that identifies a specific intrinsic function in a scoping unit has a type as specified in 13.6. An explicit type declaration statement is not required; however, it is permitted. Specifying a type for a generic intrinsic function name in a type declaration statement is not sufficient, by itself, to remove the generic properties from that function."

However, I believe that only means that besides "intrinsic sin" also "real, intrinsic :: sin" is allows.

I tested the failing interface+intrinsic and all my compilers but gfortran reject it: ifort, NAG f95, g95, openf95, sunf95.

> I haven't checked the standard on this, but I bet the code is illegal.

I agree.

> And after all: Why should one declare an explicit interface for an intrinsic
> (whose interface is known anyway) ...?

Good question for which I do not know the answer.
But also for "real, intrinsic :: " I don't know the answer, but it is allowed.
(One difference is that one has not the intrinsic vs. external conflict and that old Fortran programs used it (probably incl. implicit typing). INTERFACE is F90 and there one wanted to be stricter.)
Comment 10 janus 2008-05-26 18:44:06 UTC
Created attachment 15687 [details]
patch version 2

The attached new version of the patch fixes the reduction3.f90 testcase by deleting the interface statement (the "intrinsic" has to stay I think). Moreover it includes another related bugfix (see PR35830 comment #2).

Regarding Tobi's comment #7:

interface
  real function bar()
  end function bar
end interface
dimension :: bar(4)

Why would I need to reject this? At least it's compatible with

  conflict(external with implicit interface, dimension)

because it has an explicit interface. I think the problem is rather that the
dimension statement contradicts the interface statement, which says that 'bar' returns a scalar real number (and not an array).

g95's error message is:

dimension :: bar(4)
             1
Error: Attribute declaration of 'bar' at (1) is outside of the INTERFACE body

So I guess it is indeed invalid (and accepted by gfortran), but this seems to be unrelated to this PR (probably it deserves a separate PR).

Apart from this I see no remaining problems. The patch regtests without any failures.
Comment 11 Tobias Burnus 2008-05-26 20:34:09 UTC
> Moreover it includes another related bugfix (see PR35830 comment #2).
Nice. If everything works, one should submit it, including new test cases.

> Regarding Tobi's comment #7:
> 
> interface
>   real function bar()
>   end function bar
> end interface
> dimension :: bar(4)
> 
> Why would I need to reject this? At least it's compatible with
> 
>   conflict(external with implicit interface, dimension)
> 
> because it has an explicit interface.

> I think the problem is rather that the dimension statement contradicts
> the interface statement, which says that 'bar'
> returns a scalar real number (and not an array).

I agree. Fortran 2003 states:

"An interface body specifies all of the characteristics of the explicit specific interface or abstract interface."

> g95's error message is:
> Error: Attribute declaration of 'bar' at (1) is outside of the INTERFACE body

Same with NAG f95:
Error: Dimensions specified for BAR outside its INTERFACE body

ifort has:
"Error: This name has already been used as an external procedure name. [BAR]"

> So I guess it is indeed invalid (and accepted by gfortran), but this seems to
> be unrelated to this PR (probably it deserves a separate PR).

I agree that it can be an extra PR. Analogous problem for ALLOCATABLE.
Comment 12 janus 2008-05-28 21:28:42 UTC
Subject: Bug 36325

Author: janus
Date: Wed May 28 21:27:56 2008
New Revision: 136130

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=136130
Log:
2008-05-28  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/36325
	PR fortran/35830
	* interface.c (gfc_procedure_use): Enable argument checking for
	external procedures with explicit interface.
	* symbol.c (check_conflict): Fix conflict checking for externals.
	(copy_formal_args): Fix handling of arrays.
	* resolve.c (resolve_specific_f0, resolve_specific_s0): Fix handling
	of intrinsics.
	* parse.c (parse_interface): Non-abstract INTERFACE statement implies
	EXTERNAL attribute.


2008-05-28  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/36325
	PR fortran/35830
	* gfortran.dg/interface_23.f90: New.
	* gfortran.dg/gomp/reduction3.f90: Fixed invalid code.
	* gfortran.dg/proc_decl_12.f90: New:
	* gfortran.dg/external_procedures_1.f90: Fixed error message.

Added:
    trunk/gcc/testsuite/gfortran.dg/interface_23.f90
    trunk/gcc/testsuite/gfortran.dg/proc_decl_12.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/interface.c
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/symbol.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/external_procedures_1.f90
    trunk/gcc/testsuite/gfortran.dg/gomp/reduction3.f90

Comment 13 janus 2008-05-28 21:37:22 UTC
Fixed with r136130. Closing.