diff mbox series

[fortran] PR96686 Namelist group objects shall be defined before appearing in namelist

Message ID 61fc2d13-04fc-0262-578a-b3f48d130589@charter.net
State New
Headers show
Series [fortran] PR96686 Namelist group objects shall be defined before appearing in namelist | expand

Commit Message

Jerry DeLisle Feb. 17, 2021, 3:02 a.m. UTC
Hi all,

Attached patch adds to checks.  In the case of IMPLICIT typing it checks 
to see if the objects listed in the NAMELIST have defined types andf if 
not, sets them to the default implicit types.

In the case of IMPLICIT NONE, the types are required be declared before 
the NAMELIST.   If an object type is found to not be declared already, 
an error is issued.

One new test case added and one modified to pass.

Regression tested.

OK for trunk?

Regards,

Jerry

fortran: Object types should be declared before use in NAMELIST.

gcc/fortran/ChangeLog:

     PR fortran/98686
     * match.c (gfc_match_namelist): Add checks for IMPLICIT NONE and
     whether the type for each namelist object has been defined before
     the namelist declaration.  For IMPLICIT, set the types so that
     any subsequent use of objects will have their types confirmed.

gcc/testsuite/ChangeLog:

     PR fortran/98686
     * gfortran.dg/namelist_4.f90: Modify.
     * gfortran.dg/namelist_98.f90: New test.

Comments

Tobias Burnus Feb. 17, 2021, 9:19 a.m. UTC | #1
Hi Jerry,

I note that you have not written that testcase and I am still half
aspleep, but I fail to see what's wrong with the following program
(before and after your change):

f2 looks like a local and implicitly typed real variable. At least ifort
compiles this program successfully.

F2018 has: "A namelist group object shall either be accessed by use or
host association or shall have its declared type, kind type parameters
of the declared type, and rank specified by previous specification
statements or the procedure heading in the same scoping unit or by the
implicit typing rules in effect for the scoping unit. If a namelist
group object is typed by the implicit typing rules, its appearance in
any subsequent type declaration statement shall confirm the implied type
and type parameters."

Tobias

On 17.02.21 04:02, Jerry DeLisle wrote:
> index 538bceaa4b6..4e021253f01 100644
> --- a/gcc/testsuite/gfortran.dg/namelist_4.f90
> +++ b/gcc/testsuite/gfortran.dg/namelist_4.f90
> @@ -27,14 +27,14 @@ END module M1
>   program P1
>   CONTAINS
>   ! This has the additional wrinkle of a reference to the object.
> +  INTEGER FUNCTION F2()
> +    F2=1
> +  END FUNCTION
>     INTEGER FUNCTION F1()
>       NAMELIST /NML3/ F2 ! { dg-error "PROCEDURE attribute conflicts" }
>   ! Used to ICE here
> -    f2 = 1             ! { dg-error "is not a VALUE" }
> +    f2 = 1             ! { dg-error "is not a variable" }
>       F1=1
>     END FUNCTION
> -  INTEGER FUNCTION F2()
> -    F2=1
> -  END FUNCTION
>   END
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Tobias Burnus Feb. 19, 2021, 4:42 p.m. UTC | #2
Hi Jerry,

On 17.02.21 04:02, Jerry DeLisle wrote:
> Attached patch adds to checks.  In the case of IMPLICIT typing it
> checks to see if the objects listed in the NAMELIST have defined types
> andf if not, sets them to the default implicit types.
>
> In the case of IMPLICIT NONE, the types are required be declared
> before the NAMELIST.   If an object type is found to not be declared
> already, an error is issued.
> Regression tested.
>
> OK for trunk?

After taking a look while being less groggy,
it looks good to me, but I have a few remarks:

I think it looks cleaner to swap inner/outer the conditions, i.e.

if (sym->ts.type == BT_UNKNOWN)
   {
     if (!gfc_current_ns->seen_implicit_none)
       ...
     else
       ...
   }

> +++ b/gcc/testsuite/gfortran.dg/namelist_4.f90
> @@ -27,14 +27,14 @@ END module M1
>   program P1
>   CONTAINS
>   ! This has the additional wrinkle of a reference to the object.
> +  INTEGER FUNCTION F2()
> +    F2=1
> +  END FUNCTION
>     INTEGER FUNCTION F1()
>       NAMELIST /NML3/ F2 ! { dg-error "PROCEDURE attribute conflicts" }
>   ! Used to ICE here
> -    f2 = 1             ! { dg-error "is not a VALUE" }
> +    f2 = 1             ! { dg-error "is not a variable" }
>       F1=1
>     END FUNCTION
> -  INTEGER FUNCTION F2()
> -    F2=1
> -  END FUNCTION
>   END

Unless I made a mistake, there is no need to modify this testcase – even
with the patch, the error remains the same.

However, if the "f2 = 1" line is removed, the previously hidden error in
the NAMELIST line is shown: 'PROCEDURE attribute conflicts with NAMELIST
attribute in ‘f2’ at (1)' is shown.

I think you should retain this testcase – and either add between the two
functions a new one, copying 'f1' but with 'f2 = 1' removed. You then
get the expected NAMELIST error. — Or to add a new testcase for this check.

PLUS: I think it would be useful to add a test of the form:

   namelist /nml/ f
   integer :: f

which then shows the new error (declared before the namelist) which
is currently not checked for.

  * * *

On 19.02.21 16:58, Jerry DeLisle wrote:
> On 2/17/21 1:19 AM, Tobias Burnus wrote:
>> f2 looks like a local and implicitly typed real variable. At least ifort
>> compiles this program successfully.

I have to admit that I am not sure about this – implicit typing is odd,
if combined with host association. But I think you are right. I also got
confused due to the reordering, which is not needed.

Regarding:

> contains
>       integer function f1()
>         !f2 = 1   !This gives an error trying to assign a value to a
>    function.
Okay. Also matches ifort: "This name has already been used as an
external function name."
>     j = f2    ! This is OK

This one is odd – you assign a function address to an integer (the
compiler does the casting). ifort rejects it with: "This name has
already been used as an external function name."

That looks like a variant of https://gcc.gnu.org/PR98890 – I added it as
additional example.

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
diff mbox series

Patch

diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index 2df6191d7e6..3a06f308812 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -5536,6 +5536,27 @@  gfc_match_namelist (void)
 	  if (m == MATCH_ERROR)
 	    goto error;
 
+	  if (!gfc_current_ns->seen_implicit_none)
+	    {
+	      /* If the type is not set already, we set it here to the
+		 implicit default type.  It is not allowed to set it
+		 later to any other type.  */
+	      if (sym->ts.type == BT_UNKNOWN)
+		gfc_set_default_type (sym, 0, gfc_current_ns);
+	    }
+	  else
+	    {
+	      /* It is required that members of a namelist be declared
+		 before the namelist.  We check this by checking if the
+		 symbol has a defined type for IMPLICIT NONE.  */
+	      if (sym->ts.type == BT_UNKNOWN)
+		{
+		  gfc_error ("Symbol %qs in namelist %qs at %C must be "
+			     "declared before the namelist is declared.",
+			     sym->name, group_name->name);
+		  gfc_error_check ();
+		}
+	    }
 	  if (sym->attr.in_namelist == 0
 	      && !gfc_add_in_namelist (&sym->attr, sym->name, NULL))
 	    goto error;
diff --git a/gcc/testsuite/gfortran.dg/namelist_4.f90 b/gcc/testsuite/gfortran.dg/namelist_4.f90
index 538bceaa4b6..4e021253f01 100644
--- a/gcc/testsuite/gfortran.dg/namelist_4.f90
+++ b/gcc/testsuite/gfortran.dg/namelist_4.f90
@@ -27,14 +27,14 @@  END module M1
 program P1
 CONTAINS
 ! This has the additional wrinkle of a reference to the object.
+  INTEGER FUNCTION F2()
+    F2=1
+  END FUNCTION
   INTEGER FUNCTION F1()
     NAMELIST /NML3/ F2 ! { dg-error "PROCEDURE attribute conflicts" }
 ! Used to ICE here
-    f2 = 1             ! { dg-error "is not a VALUE" }
+    f2 = 1             ! { dg-error "is not a variable" }
     F1=1
   END FUNCTION
-  INTEGER FUNCTION F2()
-    F2=1
-  END FUNCTION
 END
 
diff --git a/gcc/testsuite/gfortran.dg/namelist_98.f90 b/gcc/testsuite/gfortran.dg/namelist_98.f90
new file mode 100644
index 00000000000..19a7e869f92
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/namelist_98.f90
@@ -0,0 +1,11 @@ 
+! { dg-do compile }
+! pr98686
+  implicit none
+  real    :: x, m
+  namelist /NML/ x, m, q ! { dg-error "must be declared before the namelist*" }
+  integer :: q
+  x = 1.0
+  m = 2.0
+  q = 3
+  write(*, nml=NML)
+end