This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, fortran] PR25058 & PR25090 - Ping


Could somebody review this, please? It's been in my working tree for a week and has not caused any problems.

Paul

:ADDPATCH fortran:

This patch is the third attempt to fix PR25090 and PR25058.

The problem with the first version of the patch to fix PR25090 was that it emitted an error if a specification expression was not preceeded by all the variables, as dummies, in the first entry. The standard does not demand this. Rather, 12.5.2.5 requires: "If a dummy argument is used in a specification expression to specify an array bound or character length of an object, the appearance of the object in a statement that is executed during a procedure reference is permitted only if the dummy argument appears in the dummy argument list of the procedure name referenced and it is present(12.4.1.5). "

This cannot easily be implemented. However, this paragraph does imply that a reference to the variable, in whose declaration the specification expressions appear, should be preceeded by the appearance of the requisite variables as dummy argments.

In the second version of the patch, I took it that it follows that if any one of the variables appear, they all must. This was never applied
after a correct intervention from Brooks. This version of the patch
attempts no such thing.


In addition, the standard requires that "In a subprogram, a name that appears as a dummy argument in an ENTRY statement shall not appear in an executable statement preceding that ENTRY statement, unless it also appears in a FUNCTION, SUBROUTINE, or ENTRY statement that precedes the executable statement." This has been implemented at the same time as the repair, thereby dispatching PR25058.

The body of the patch is to be found, in resolve_variable.

cs_base != NULL is used to detect code resolution. A new int field for gfc_symbol, entry_id, is checked against the last entry ID seen. This was done to prevent this check from being done for every reference to the symbol in every line of code. If the extra field is thought to be too high a price to be paid, I can add an attribute bit that is reset for each entry.

It is my belief that PR25090 is not strictly fixable as it stands, for the reasons that Brooks outlined. Therefore, I have modified it to
require that a reference that implicitly refers to a dummy argument, through a specification expression, is what concerns us. To see what
I mean, compare the PR with entry_dummy_ref_1.f90.


Regtested on FC5/Athlon. OK for trunk and for 4.1, when reopened?

Paul

2006-05-26 Paul Thomas <pault@gcc.gnu.org>

    PR fortran/25090
    PR fortran/25058
    * gfortran.h : Add int entry_id to gfc_symbol.
    * resolve.c : Add static variables current_entry_id and
    specification_expr.
    (resolve_variable): During code resolution, check if a
    reference to a dummy variable in an executable expression
    is preceded by its appearance as a parameter in an entry.
    Likewise check its specification expressions.
    (resolve_code): Update current_entry_id on EXEC_ENTRY.
    (resolve_charlen, resolve_fl_variable): Set and reset
    specifiaction_expr.
    (is_non_constant_shape_array): Do not return on detection
    of a variable but continue to resolve all the expressions.
    (resolve_codes): set current_entry_id to an out of range
    value.

2006-05-26 Paul Thomas <pault@gcc.gnu.org>

    PR fortran/25090
    * gfortran.dg/entry_dummy_ref_1.f90: New test.

PR fortran/25058
* gfortran.dg/entry_dummy_ref_2.f90: New test.










------------------------------------------------------------------------

Index: gcc/fortran/gfortran.h
===================================================================
*** gcc/fortran/gfortran.h (revision 114126)
--- gcc/fortran/gfortran.h (working copy)
*************** typedef struct gfc_symbol
*** 838,843 ****
--- 838,845 ----
order. */
int dummy_order;
+ int entry_id;
+ gfc_namelist *namelist, *namelist_tail;
/* Change management fields. Symbols that might be modified by the
Index: gcc/fortran/resolve.c
===================================================================
*** gcc/fortran/resolve.c (revision 114127)
--- gcc/fortran/resolve.c (working copy)
*************** static int omp_workshare_flag;
*** 60,65 ****
--- 60,71 ----
resets the flag each time that it is read. */
static int formal_arg_flag = 0;
+ /* True if we are resolving a specification expression. */
+ static int specification_expr = 0;
+ + /* The id of the last entry seen. */
+ static int current_entry_id;
+ int
gfc_is_formal_arg (void)
{
*************** static try
*** 2661,2666 ****
--- 2667,2675 ----
resolve_variable (gfc_expr * e)
{
gfc_symbol *sym;
+ try t;
+ + t = SUCCESS;
if (e->ref && resolve_ref (e) == FAILURE)
return FAILURE;
*************** resolve_variable (gfc_expr * e)
*** 2688,2694 ****
if (check_assumed_size_reference (sym, e))
return FAILURE;
! return SUCCESS;
}
--- 2697,2769 ----
if (check_assumed_size_reference (sym, e))
return FAILURE;
! /* Deal with forward references to entries during resolve_code, to
! satisfy, at least partially, 12.5.2.5. */
! if (gfc_current_ns->entries
! && current_entry_id == sym->entry_id
! && cs_base
! && cs_base->current
! && cs_base->current->op != EXEC_ENTRY)
! {
! gfc_entry_list *entry;
! gfc_formal_arglist *formal;
! int n;
! bool seen;
! ! /* If the symbol is a dummy... */
! if (sym->attr.dummy)
! {
! entry = gfc_current_ns->entries;
! seen = false;
! ! /* ...test if the symbol is a parameter of previous entries. */
! for (; entry && entry->id <= current_entry_id; entry = entry->next)
! for (formal = entry->sym->formal; formal; formal = formal->next)
! {
! if (formal->sym && sym->name == formal->sym->name)
! seen = true;
! }
! ! /* If it has not been seen as a dummy, this is an error. */
! if (!seen)
! {
! if (specification_expr)
! gfc_error ("Variable '%s',used in a specification expression, "
! "is referenced at %L before the ENTRY statement "
! "in which it is a parameter",
! sym->name, &cs_base->current->loc);
! else
! gfc_error ("Variable '%s' is used at %L before the ENTRY "
! "statement in which it is a parameter",
! sym->name, &cs_base->current->loc);
! t = FAILURE;
! }
! }
! ! /* Now do the same check on the specification expressions. */
! specification_expr = 1;
! if (sym->ts.type == BT_CHARACTER
! && gfc_resolve_expr (sym->ts.cl->length) == FAILURE)
! t = FAILURE;
! ! if (sym->as)
! for (n = 0; n < sym->as->rank; n++)
! {
! specification_expr = 1;
! if (gfc_resolve_expr (sym->as->lower[n]) == FAILURE)
! t = FAILURE;
! specification_expr = 1;
! if (gfc_resolve_expr (sym->as->upper[n]) == FAILURE)
! t = FAILURE;
! }
! specification_expr = 0;
! ! if (t == SUCCESS)
! /* Update the symbol's entry level. */
! sym->entry_id = current_entry_id + 1;
! }
! ! return t;
}
*************** resolve_code (gfc_code * code, gfc_names
*** 4388,4394 ****
--- 4463,4473 ----
case EXEC_EXIT:
case EXEC_CONTINUE:
case EXEC_DT_END:
+ break;
+ case EXEC_ENTRY:
+ /* Keep track of which entry we are up to. */
+ current_entry_id = code->ext.entry->id;
break;
case EXEC_WHERE:
*************** resolve_values (gfc_symbol * sym)
*** 4667,4673 ****
static try
resolve_index_expr (gfc_expr * e)
{
- if (gfc_resolve_expr (e) == FAILURE)
return FAILURE;
--- 4746,4751 ----
*************** resolve_charlen (gfc_charlen *cl)
*** 4690,4697 ****
cl->resolved = 1;
if (resolve_index_expr (cl->length) == FAILURE)
! return FAILURE;
return SUCCESS;
}
--- 4768,4780 ----
cl->resolved = 1;
+ specification_expr = 1;
+ if (resolve_index_expr (cl->length) == FAILURE)
! {
! specification_expr = 0;
! return FAILURE;
! }
return SUCCESS;
}
*************** is_non_constant_shape_array (gfc_symbol *** 4704,4710 ****
--- 4787,4795 ----
{
gfc_expr *e;
int i;
+ bool not_constant;
+ not_constant = false;
if (sym->as != NULL)
{
/* Unfortunately, !gfc_is_compile_time_shape hits a legal case that
*************** is_non_constant_shape_array (gfc_symbol *** 4715,4729 ****
e = sym->as->lower[i];
if (e && (resolve_index_expr (e) == FAILURE
|| !gfc_is_constant_expr (e)))
! return true;
e = sym->as->upper[i];
if (e && (resolve_index_expr (e) == FAILURE
|| !gfc_is_constant_expr (e)))
! return true;
}
}
! return false;
}
/* Resolution of common features of flavors variable and procedure. */
--- 4800,4814 ----
e = sym->as->lower[i];
if (e && (resolve_index_expr (e) == FAILURE
|| !gfc_is_constant_expr (e)))
! not_constant = true;
e = sym->as->upper[i];
if (e && (resolve_index_expr (e) == FAILURE
|| !gfc_is_constant_expr (e)))
! not_constant = true;
}
}
! return not_constant;
}
/* Resolution of common features of flavors variable and procedure. */
*************** resolve_fl_variable (gfc_symbol *sym, in
*** 4779,4796 ****
if (resolve_fl_var_and_proc (sym, mp_flag) == FAILURE)
return FAILURE;
! /* The shape of a main program or module array needs to be constant. */
! if (sym->ns->proc_name
! && (sym->ns->proc_name->attr.flavor == FL_MODULE
! || sym->ns->proc_name->attr.is_main_program)
! && !sym->attr.use_assoc
&& !sym->attr.allocatable
&& !sym->attr.pointer
&& is_non_constant_shape_array (sym))
{
! gfc_error ("The module or main program array '%s' at %L must "
! "have constant shape", sym->name, &sym->declared_at);
! return FAILURE;
}
if (sym->ts.type == BT_CHARACTER)
--- 4864,4889 ----
if (resolve_fl_var_and_proc (sym, mp_flag) == FAILURE)
return FAILURE;
! /* Set this flag to check that variables are parameters of all entries.
! This check is effected by the call to gfc_resolve_expr through
! is_non_constant_shape_array. */
! specification_expr = 1;
! ! if (!sym->attr.use_assoc
&& !sym->attr.allocatable
&& !sym->attr.pointer
&& is_non_constant_shape_array (sym))
{
! /* The shape of a main program or module array needs to be constant. */
! if (sym->ns->proc_name
! && (sym->ns->proc_name->attr.flavor == FL_MODULE
! || sym->ns->proc_name->attr.is_main_program))
! {
! gfc_error ("The module or main program array '%s' at %L must "
! "have constant shape", sym->name, &sym->declared_at);
! specification_expr = 0;
! return FAILURE;
! }
}
if (sym->ts.type == BT_CHARACTER)
*************** resolve_codes (gfc_namespace * ns)
*** 6314,6319 ****
--- 6407,6414 ----
gfc_current_ns = ns;
cs_base = NULL;
+ /* Set to an out of range value. */
+ current_entry_id = -1;
resolve_code (ns->code, ns);
}
Index: gcc/testsuite/gfortran.dg/entry_dummy_ref_1.f90
===================================================================
*** gcc/testsuite/gfortran.dg/entry_dummy_ref_1.f90 (revision 0)
--- gcc/testsuite/gfortran.dg/entry_dummy_ref_1.f90 (revision 0)
***************
*** 0 ****
--- 1,15 ----
+ ! { dg-do compile }
+ ! Tests fix for PR25090 in which references in specification
+ ! expressions to variables that were not entry formal arguments
+ ! would be missed.
+ !
+ ! Contributed by Joost VandeVondele <jv244@cam.ac.uk>
+ !
+ SUBROUTINE S1(I)
+ CHARACTER(LEN=I+J) :: a
+ real :: x(i:j), z
+ a = "" ! { dg-error "before the ENTRY statement in which it is a parameter" }
+ x = 0.0 ! { dg-error "before the ENTRY statement in which it is a parameter" }
+ ENTRY E1(J)
+ END SUBROUTINE S1
+ END
Index: gcc/testsuite/gfortran.dg/entry_dummy_ref_2.f90
===================================================================
*** gcc/testsuite/gfortran.dg/entry_dummy_ref_2.f90 (revision 0)
--- gcc/testsuite/gfortran.dg/entry_dummy_ref_2.f90 (revision 0)
***************
*** 0 ****
--- 1,20 ----
+ ! { dg-do compile }
+ ! Tests fix for PR25058 in which references to dummy
+ ! parameters before the entry would be missed.
+ !
+ ! Contributed by Joost VandeVondele <jv244@cam.ac.uk>
+ !
+ MODULE M1
+ CONTAINS
+ FUNCTION F1(I) RESULT(RF1)
+ INTEGER :: I,K,RE1,RF1
+ RE1=K ! { dg-error "before the ENTRY statement" }
+ RETURN
+ ENTRY E1(K) RESULT(RE1)
+ RE1=-I
+ RETURN
+ END FUNCTION F1
+ END MODULE M1
+ END
+ + ! { dg-final { cleanup-modules "M1" } }


------------------------------------------------------------------------

2006-05-26 Paul Thomas <pault@gcc.gnu.org>

	PR fortran/25090
	PR fortran/25058
	* gfortran.h : Add int entry_id to gfc_symbol.
	* resolve.c : Add static variables current_entry_id and
	specification_expr.
	(resolve_variable): During code resolution, check if a
	reference to a dummy variable in an executable expression
	is preceded by its appearance as a parameter in an entry.
	Likewise check its specification expressions.
	(resolve_code): Update current_entry_id on EXEC_ENTRY.
	(resolve_charlen, resolve_fl_variable): Set and reset
	specifiaction_expr.
	(is_non_constant_shape_array): Do not return on detection
	of a variable but continue to resolve all the expressions.
	(resolve_codes): set current_entry_id to an out of range
	value.

2006-05-26 Paul Thomas <pault@gcc.gnu.org>

	PR fortran/25090
	* gfortran.dg/entry_dummy_ref_1.f90: New test.

PR fortran/25058
* gfortran.dg/entry_dummy_ref_2.f90: New test.







Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]