Re: [PATCH v2 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]

[Harald Anlauf <anlauf@gmx.de>]

Hi Mikael,

thanks for the patch!

Regarding the first part of the patch, I think that fixing bad testcases
can be done at any time.  Retaining identified, broken testcases means
that one may hit bogus regressions, hindering progress.

The second part of the patch looks at first glance fine to me.  And as
the patch changes less than its size suggests, in particular due to
code refactoring, I don't see a reason to postpone it to stage 1.

(On the contrary, deferring it to stage 1 might make future backports
from 15 for later patches on top of that code harder.
This is my opinion, and others might see this differently.)

On 3/17/24 17:57, Mikael Morin wrote:
> 	* expr.cc (check_restricted): Remove the case where symbol is dummy
> 	and declared in the current ns.  Use gfc_get_spec_ns to get the
> 	right namespace.

Looking at the original and new code, I don't fully understand
that part of the commit message: the changed check comes into play
when the symbol is *not* in_common, ..., a dummy, ...
So technically, we didn't access the (now removed) formal_arg_flag
here in those cases.
Or am I missing something?

> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
> index 82a642b01f7..0852bc5f493 100644
> --- a/gcc/fortran/expr.cc
> +++ b/gcc/fortran/expr.cc
> @@ -3509,19 +3509,13 @@ check_restricted (gfc_expr *e)
>         if (!check_references (e->ref, &check_restricted))
>   	break;
>
> -      /* gfc_is_formal_arg broadcasts that a formal argument list is being
> -	 processed in resolve.cc(resolve_formal_arglist).  This is done so
> -	 that host associated dummy array indices are accepted (PR23446).
> -	 This mechanism also does the same for the specification expressions
> -	 of array-valued functions.  */
>         if (e->error
>   	    || sym->attr.in_common
>   	    || sym->attr.use_assoc
>   	    || sym->attr.dummy
>   	    || sym->attr.implied_index
>   	    || sym->attr.flavor == FL_PARAMETER
> -	    || is_parent_of_current_ns (sym->ns)
> -	    || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns)))
> +	    || is_parent_of_current_ns (gfc_get_spec_ns (sym)))
>   	{
>   	  t = true;
>   	  break;


> diff --git a/gcc/testsuite/gfortran.dg/spec_expr_8.f90 b/gcc/testsuite/gfortran.dg/spec_expr_8.f90
> new file mode 100644
> index 00000000000..5885810d421
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/spec_expr_8.f90
> @@ -0,0 +1,24 @@
> +! { dg-do compile }
> +!
> +! PR fortran/111781
> +! We used to reject the example below because the dummy procedure g was
> +! setting the current namespace without properly restoring it, which broke
> +! the specification expression check for the dimension of A later on.
> +!
> +! Contributed by Markus Vikhamar-Sandberg  <rasmus.vikhamar-sandberg@uit.no>

Is the reporter's first name Markus or Rasmus?


Thanks,
Harald