diff mbox series

[Fortran] PR93309 – permit repeated 'implicit none(external)' in sub-scoping unit

Message ID 08a3b3f3-1803-001a-4c72-94e3baf15181@codesourcery.com
State New
Headers show
Series [Fortran] PR93309 – permit repeated 'implicit none(external)' in sub-scoping unit | expand

Commit Message

Tobias Burnus Jan. 20, 2020, 2:55 p.m. UTC
Using "implicit none" multiple times in a scoping unit is not permitted 
– and checked for.

However, using one in the parent name space and re-confirming it in the 
current name space is permitted – but was before rejected.

OK for the trunk?

Tobias

Comments

Bernhard Reutner-Fischer Jan. 21, 2020, 11:43 a.m. UTC | #1
On 20 January 2020 15:55:23 CET, Tobias Burnus <tobias@codesourcery.com> wrote:
>Using "implicit none" multiple times in a scoping unit is not permitted
>
>– and checked for.
>
>However, using one in the parent name space and re-confirming it in the
>
>current name space is permitted – but was before rejected.


+      if (sym->attr.proc == PROC_UNKNOWN)
+	for (gfc_namespace *ns = sym->ns; ns; ns = ns->parent)
+	   if (ns->has_implicit_none_export)
+	     has_implicit_none_export = true;

s/;/, break;/

If we don't do this for >= -O1 already.

+      if (has_implicit_none_export)
>
>OK for the trunk?

LGTM otherwise,
thanks
Tobias Burnus Jan. 21, 2020, 12:47 p.m. UTC | #2
Hi Bernhard,

On 1/21/20 12:43 PM, Bernhard Reutner-Fischer wrote:
> +      if (sym->attr.proc == PROC_UNKNOWN)
> +	for (gfc_namespace *ns = sym->ns; ns; ns = ns->parent)
> +	   if (ns->has_implicit_none_export)
> +	     has_implicit_none_export = true;
>
> s/;/, break;/
>
> If we don't do this for >= -O1 already.

We don't. All three variants generate for -O0 to -O3 a different 
optimized tree. (I didn't look at RTL or assembler.) But it is also not 
completely clear to me which approach is the best. I think it also 
depends how it is converted into assembler and what the hardware 
actually does.

I was thinking about all three variants and had settled for the shortest 
when sending the patch. The third variant would be:

	for (gfc_namespace *ns = sym->ns;
	     ns && !has_implicit_none_export;
	     ns = ns->parent)
	   if (ns->has_implicit_none_export)
	     has_implicit_none_export = true;

which may or may not be faster in execution – the conditional assignment 
might be faster, but one has to do the "&&" operation in each step (can 
be AND_EXPR; it does not need to be THEN_IF_EXPR). — However, I find it 
less readable.

At the end, it does not really matter as the loop body and expressions 
are small and the typical nest-depth is probably two (module + module 
procedures) or three (module procedures + BLOCK or host-associated 
procedure) such that an early abort is not really worthwhile. Even with 
more nesting levels, the chance that one has many sym->attr.proc == 
PROC_UNKNOWN in a deeply nested scoping unit should be low. (And if it 
is not, it will be still not the hot loop for that code.)

However, I have now used the version with "break", which is also fine.
r10-6108-gb31f80231df9ce6d9b50c62d28b8d2a4654ef564

Cheers,

Tobias
diff mbox series

Patch

	PR fortran/93309
	* interface.c (gfc_procedure_use): Also check parent namespace for
	'implict none (external)'.
	* symbol.c (gfc_get_namespace): Don't set has_implicit_none_export
	to parent namespace's setting. 

	PR fortran/93309
	* gfortran.dg/external_implicit_none_2.f90: New.

diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index c4a6882533b..5193ac4a314 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -3798,8 +3798,13 @@  gfc_procedure_use (gfc_symbol *sym, gfc_actual_arglist **ap, locus *where)
      explicitly declared at all if requested.  */
   if (sym->attr.if_source == IFSRC_UNKNOWN && !sym->attr.is_iso_c)
     {
+      bool has_implicit_none_export = false;
       implicit = true;
-      if (sym->ns->has_implicit_none_export && sym->attr.proc == PROC_UNKNOWN)
+      if (sym->attr.proc == PROC_UNKNOWN)
+	for (gfc_namespace *ns = sym->ns; ns; ns = ns->parent)
+	   if (ns->has_implicit_none_export)
+	     has_implicit_none_export = true;
+      if (has_implicit_none_export)
 	{
 	  const char *guessed
 	    = gfc_lookup_function_fuzzy (sym->name, sym->ns->sym_root);
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 526f6c466b3..47b716454f2 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -2898,9 +2898,6 @@  gfc_get_namespace (gfc_namespace *parent, int parent_types)
 	}
     }
 
-  if (parent_types && ns->parent != NULL)
-    ns->has_implicit_none_export = ns->parent->has_implicit_none_export;
-
   ns->refs = 1;
 
   return ns;
diff --git a/gcc/testsuite/gfortran.dg/external_implicit_none_2.f90 b/gcc/testsuite/gfortran.dg/external_implicit_none_2.f90
new file mode 100644
index 00000000000..b2b1dd1e6d7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/external_implicit_none_2.f90
@@ -0,0 +1,28 @@ 
+! { dg-do compile }
+!
+! PR fortran/93309
+!
+module m
+  implicit none(external)
+contains
+  subroutine s
+    implicit none(external) ! OK
+  end subroutine
+end module
+
+module m2
+  implicit none(external)
+contains
+  subroutine s
+    call foo(1)  ! { dg-error "not explicitly declared" }
+  end subroutine
+end module
+
+module m3
+  implicit none(external)
+contains
+  subroutine s
+    implicit none(external) ! OK
+    implicit none(external) ! { dg-error "Duplicate IMPLICIT NONE statement" }
+  end subroutine
+end module