Re: [patch, fortran] PR96686 Namelist group objects shall be defined before appearing in namelist

[Tobias Burnus <Tobias_Burnus@mentor.com>]

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