Le 17/03/2024 à 21:57, Harald Anlauf a écrit :
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.
What worries me a bit is the changes of gfc_current_ns.
It's the kind of change that has broad impact and can uncover weird
behaviors.
(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.)
To be honest, I posted it now in the hope that someone would be willing
to push it before stage 1 so that I can get rid of it sooner.
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?
Oh, do you really read all of my prose?
I wrote it from vague memories of debugging the problem weeks ago,
without thinking very much about it.
Actually, there are two dummy arguments here.
There is the one we are checking, and its bounds should be a
specification expression, so could possibly be another dummy variable
(the "sym" variable in this context).
The condition was allowing variables declared in the same scope to
appear in specification expression of dummy arguments.
I will try to rephrase the sentence, as it sounds more clear (and
sensible) as expressed here.
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-sandb...@uit.no>
Is the reporter's first name Markus or Rasmus?
Rasmus.
Reviews are sometimes helpful, more often than not.
I don't know where the Markus name comes from.
Thanks for the review.
