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] |
Hello, here is a fix for PR47586: missing deep copy for the case: dt_w_alloc = ptr_func(arg) where dt_w_alloc is of derived type with allocatable components, and ptr_func returns a data pointer. The fix tweaks expr_is_variable so that gfc_trans_scalar_assign is called with the flag enabling deep copy set. I added a few fixes loosely related before, so that the patches are as follows: 1/4: gfc_is_proc_ptr_comp interface change, 2/4: gfc_is_scalar_ptr deep_copy flag lengthy explanation, 3/4: regression fix, 4/4: patch fixing the PR. Regression-tested on x86_64-unknown-linux-gnu. OK for trunk? Mikael
gfc_is_proc_ptr_comp has a side effect: if the expression references a procedure pointer component, it returns true and assigns to its second argument the component. As I don't like side effects, this patch removes the second argument and replaces the cases where it is useful by a call to (the new function) gfc_get_proc_ptr_comp. This is optional: I can adjust the patch depending on it (patch 4) to do it the old way if it's preferred. OK? 2012-08-13 Mikael Morin <mikael@gcc.gnu.org> * gfortran.h (gfc_get_proc_ptr_comp): New prototype. (gfc_is_proc_ptr_comp): Update prototype. * expr.c (gfc_get_proc_ptr_comp): New function based on the old gfc_is_proc_ptr_comp. (gfc_is_proc_ptr_comp): Call gfc_get_proc_ptr_comp. (gfc_specification_expr, gfc_check_pointer_assign): Use gfc_get_proc_ptr_comp. * trans-array.c (gfc_walk_function_expr): Likewise. * resolve.c (resolve_structure_cons, update_ppc_arglist, resolve_ppc_call, resolve_expr_ppc): Likewise. (resolve_function): Update call to gfc_is_proc_ptr_comp. * dump-parse-tree.c (show_expr): Likewise. * interface.c (compare_actual_formal): Likewise. * match.c (gfc_match_pointer_assignment): Likewise. * primary.c (gfc_match_varspec): Likewise. * trans-io.c (gfc_trans_transfer): Likewise. * trans-expr.c (gfc_conv_variable, conv_function_val, conv_isocbinding_procedure, gfc_conv_procedure_call, gfc_trans_pointer_assignment): Likewise. (gfc_conv_procedure_call, gfc_trans_array_func_assign): Use gfc_get_proc_ptr_comp. diff --git a/dump-parse-tree.c b/dump-parse-tree.c index 681dc8d..cb8fab4 100644 --- a/dump-parse-tree.c +++ b/dump-parse-tree.c @@ -569,7 +569,7 @@ show_expr (gfc_expr *p) if (p->value.function.name == NULL) { fprintf (dumpfile, "%s", p->symtree->n.sym->name); - if (gfc_is_proc_ptr_comp (p, NULL)) + if (gfc_is_proc_ptr_comp (p)) show_ref (p->ref); fputc ('[', dumpfile); show_actual_arglist (p->value.function.actual); @@ -578,7 +578,7 @@ show_expr (gfc_expr *p) else { fprintf (dumpfile, "%s", p->value.function.name); - if (gfc_is_proc_ptr_comp (p, NULL)) + if (gfc_is_proc_ptr_comp (p)) show_ref (p->ref); fputc ('[', dumpfile); fputc ('[', dumpfile); diff --git a/expr.c b/expr.c index cb5e1c6..18e8b5b 100644 --- a/expr.c +++ b/expr.c @@ -2965,12 +2965,12 @@ gfc_specification_expr (gfc_expr *e) return FAILURE; } + comp = gfc_get_proc_ptr_comp (e); if (e->expr_type == EXPR_FUNCTION - && !e->value.function.isym - && !e->value.function.esym - && !gfc_pure (e->symtree->n.sym) - && (!gfc_is_proc_ptr_comp (e, &comp) - || !comp->attr.pure)) + && !e->value.function.isym + && !e->value.function.esym + && !gfc_pure (e->symtree->n.sym) + && (!comp || !comp->attr.pure)) { gfc_error ("Function '%s' at %L must be PURE", e->symtree->n.sym->name, &e->where); @@ -3478,12 +3478,14 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_expr *rvalue) } } - if (gfc_is_proc_ptr_comp (lvalue, &comp)) + comp = gfc_get_proc_ptr_comp (lvalue); + if (comp) s1 = comp->ts.interface; else s1 = lvalue->symtree->n.sym; - if (gfc_is_proc_ptr_comp (rvalue, &comp)) + comp = gfc_get_proc_ptr_comp (rvalue); + if (comp) { s2 = comp->ts.interface; name = comp->name; @@ -4058,31 +4060,35 @@ gfc_expr_set_symbols_referenced (gfc_expr *expr) } -/* Determine if an expression is a procedure pointer component. If yes, the - argument 'comp' will point to the component (provided that 'comp' was - provided). */ +/* Determine if an expression is a procedure pointer component and return + the component in that case. Otherwise return NULL. */ -bool -gfc_is_proc_ptr_comp (gfc_expr *expr, gfc_component **comp) +gfc_component * +gfc_get_proc_ptr_comp (gfc_expr *expr) { gfc_ref *ref; - bool ppc = false; if (!expr || !expr->ref) - return false; + return NULL; ref = expr->ref; while (ref->next) ref = ref->next; - if (ref->type == REF_COMPONENT) - { - ppc = ref->u.c.component->attr.proc_pointer; - if (ppc && comp) - *comp = ref->u.c.component; - } + if (ref->type == REF_COMPONENT + && ref->u.c.component->attr.proc_pointer) + return ref->u.c.component; + + return NULL; +} + - return ppc; +/* Determine if an expression is a procedure pointer component. */ + +bool +gfc_is_proc_ptr_comp (gfc_expr *expr) +{ + return (gfc_get_proc_ptr_comp (expr) != NULL); } diff --git a/gfortran.h b/gfortran.h index e1f2e3c..0697771 100644 --- a/gfortran.h +++ b/gfortran.h @@ -2766,7 +2766,8 @@ gfc_try gfc_expr_check_typed (gfc_expr*, gfc_namespace*, bool); void gfc_expr_replace_symbols (gfc_expr *, gfc_symbol *); void gfc_expr_replace_comp (gfc_expr *, gfc_component *); -bool gfc_is_proc_ptr_comp (gfc_expr *, gfc_component **); +gfc_component * gfc_get_proc_ptr_comp (gfc_expr *); +bool gfc_is_proc_ptr_comp (gfc_expr *); bool gfc_ref_this_image (gfc_ref *ref); bool gfc_is_coindexed (gfc_expr *); diff --git a/interface.c b/interface.c index 098ec3d..fe9962f 100644 --- a/interface.c +++ b/interface.c @@ -2421,7 +2421,7 @@ compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, && a->expr->symtree->n.sym->attr.proc_pointer) || (a->expr->expr_type == EXPR_FUNCTION && a->expr->symtree->n.sym->result->attr.proc_pointer) - || gfc_is_proc_ptr_comp (a->expr, NULL))) + || gfc_is_proc_ptr_comp (a->expr))) { if (where) gfc_error ("Expected a procedure pointer for argument '%s' at %L", @@ -2431,7 +2431,7 @@ compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, /* Satisfy 12.4.1.2 by ensuring that a procedure actual argument is provided for a procedure formal argument. */ - if (a->expr->ts.type != BT_PROCEDURE && !gfc_is_proc_ptr_comp (a->expr, NULL) + if (a->expr->ts.type != BT_PROCEDURE && !gfc_is_proc_ptr_comp (a->expr) && a->expr->expr_type == EXPR_VARIABLE && f->sym->attr.flavor == FL_PROCEDURE) { diff --git a/match.c b/match.c index 737d6a3..c1d98a5 100644 --- a/match.c +++ b/match.c @@ -1344,7 +1344,7 @@ gfc_match_pointer_assignment (void) } if (lvalue->symtree->n.sym->attr.proc_pointer - || gfc_is_proc_ptr_comp (lvalue, NULL)) + || gfc_is_proc_ptr_comp (lvalue)) gfc_matching_procptr_assignment = 1; else gfc_matching_ptr_assignment = 1; diff --git a/primary.c b/primary.c index e2c3f99..3ee97d6 100644 --- a/primary.c +++ b/primary.c @@ -1862,7 +1862,7 @@ gfc_match_varspec (gfc_expr *primary, int equiv_flag, bool sub_flag, if ((equiv_flag && gfc_peek_ascii_char () == '(') || gfc_peek_ascii_char () == '[' || sym->attr.codimension || (sym->attr.dimension && sym->ts.type != BT_CLASS - && !sym->attr.proc_pointer && !gfc_is_proc_ptr_comp (primary, NULL) + && !sym->attr.proc_pointer && !gfc_is_proc_ptr_comp (primary) && !(gfc_matching_procptr_assignment && sym->attr.flavor == FL_PROCEDURE)) || (sym->ts.type == BT_CLASS && sym->attr.class_ok diff --git a/resolve.c b/resolve.c index 370e5cd..5668b66 100644 --- a/resolve.c +++ b/resolve.c @@ -1156,7 +1156,8 @@ resolve_structure_cons (gfc_expr *expr, int init) const char *name; char err[200]; - if (gfc_is_proc_ptr_comp (cons->expr, &c2)) + c2 = gfc_get_proc_ptr_comp (cons->expr); + if (c2) { s2 = c2->ts.interface; name = c2->name; @@ -3060,9 +3061,9 @@ resolve_function (gfc_expr *expr) sym = expr->symtree->n.sym; /* If this is a procedure pointer component, it has already been resolved. */ - if (gfc_is_proc_ptr_comp (expr, NULL)) + if (gfc_is_proc_ptr_comp (expr)) return SUCCESS; - + if (sym && sym->attr.intrinsic && resolve_intrinsic (sym, &expr->where) == FAILURE) return FAILURE; @@ -5675,7 +5676,8 @@ update_ppc_arglist (gfc_expr* e) gfc_component *ppc; gfc_typebound_proc* tb; - if (!gfc_is_proc_ptr_comp (e, &ppc)) + ppc = gfc_get_proc_ptr_comp (e); + if (!ppc) return FAILURE; tb = ppc->tb; @@ -6298,10 +6300,9 @@ static gfc_try resolve_ppc_call (gfc_code* c) { gfc_component *comp; - bool b; - b = gfc_is_proc_ptr_comp (c->expr1, &comp); - gcc_assert (b); + comp = gfc_get_proc_ptr_comp (c->expr1); + gcc_assert (comp != NULL); c->resolved_sym = c->expr1->symtree->n.sym; c->expr1->expr_type = EXPR_VARIABLE; @@ -6333,10 +6334,9 @@ static gfc_try resolve_expr_ppc (gfc_expr* e) { gfc_component *comp; - bool b; - b = gfc_is_proc_ptr_comp (e, &comp); - gcc_assert (b); + comp = gfc_get_proc_ptr_comp (e); + gcc_assert (comp != NULL); /* Convert to EXPR_FUNCTION. */ e->expr_type = EXPR_FUNCTION; diff --git a/trans-array.c b/trans-array.c index 555d696..4891d1e 100644 --- a/trans-array.c +++ b/trans-array.c @@ -8612,7 +8612,7 @@ gfc_walk_function_expr (gfc_ss * ss, gfc_expr * expr) sym = expr->symtree->n.sym; /* A function that returns arrays. */ - gfc_is_proc_ptr_comp (expr, &comp); + comp = gfc_get_proc_ptr_comp (expr); if ((!comp && gfc_return_by_reference (sym) && sym->result->attr.dimension) || (comp && comp->attr.dimension)) return gfc_get_array_ss (ss, expr, expr->rank, GFC_SS_FUNCTION); diff --git a/trans-expr.c b/trans-expr.c index 263605a..c51eb7b 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -1512,9 +1512,9 @@ gfc_conv_variable (gfc_se * se, gfc_expr * expr) separately. */ if (se->want_pointer) { - if (expr->ts.type == BT_CHARACTER && !gfc_is_proc_ptr_comp (expr, NULL)) + if (expr->ts.type == BT_CHARACTER && !gfc_is_proc_ptr_comp (expr)) gfc_conv_string_parameter (se); - else + else se->expr = gfc_build_addr_expr (NULL_TREE, se->expr); } } @@ -2438,7 +2438,7 @@ conv_function_val (gfc_se * se, gfc_symbol * sym, gfc_expr * expr) { tree tmp; - if (gfc_is_proc_ptr_comp (expr, NULL)) + if (gfc_is_proc_ptr_comp (expr)) tmp = get_proc_ptr_comp (expr); else if (sym->attr.dummy) { @@ -3448,7 +3448,7 @@ conv_isocbinding_procedure (gfc_se * se, gfc_symbol * sym, if (arg->next->expr->rank == 0) { if (sym->intmod_sym_id == ISOCBINDING_F_POINTER - || gfc_is_proc_ptr_comp (arg->next->expr, NULL)) + || gfc_is_proc_ptr_comp (arg->next->expr)) fptrse.want_pointer = 1; gfc_conv_expr (&fptrse, arg->next->expr); @@ -3650,7 +3650,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && conv_isocbinding_procedure (se, sym, args)) return 0; - gfc_is_proc_ptr_comp (expr, &comp); + comp = gfc_get_proc_ptr_comp (expr); if (se->ss != NULL) { @@ -3959,7 +3959,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && e->symtree->n.sym->attr.dummy)) || (fsym->attr.proc_pointer && e->expr_type == EXPR_VARIABLE - && gfc_is_proc_ptr_comp (e, NULL)) + && gfc_is_proc_ptr_comp (e)) || (fsym->attr.allocatable && fsym->attr.flavor != FL_PROCEDURE))) { @@ -6008,7 +6008,7 @@ gfc_trans_pointer_assignment (gfc_expr * expr1, gfc_expr * expr2) if (expr1->ts.type == BT_CHARACTER && expr2->expr_type != EXPR_NULL && !expr1->ts.deferred && !expr1->symtree->n.sym->attr.proc_pointer - && !gfc_is_proc_ptr_comp (expr1, NULL)) + && !gfc_is_proc_ptr_comp (expr1)) { gcc_assert (expr2->ts.type == BT_CHARACTER); gcc_assert (lse.string_length && rse.string_length); @@ -6701,9 +6701,9 @@ gfc_trans_arrayfunc_assign (gfc_expr * expr1, gfc_expr * expr2) /* The frontend doesn't seem to bother filling in expr->symtree for intrinsic functions. */ + comp = gfc_get_proc_ptr_comp (expr2); gcc_assert (expr2->value.function.isym - || (gfc_is_proc_ptr_comp (expr2, &comp) - && comp && comp->attr.dimension) + || (comp && comp->attr.dimension) || (!comp && gfc_return_by_reference (expr2->value.function.esym) && expr2->value.function.esym->result->attr.dimension)); diff --git a/trans-io.c b/trans-io.c index 8218f85..9d7d5b6 100644 --- a/trans-io.c +++ b/trans-io.c @@ -2252,7 +2252,7 @@ gfc_trans_transfer (gfc_code * code) /* Transfer an array. If it is an array of an intrinsic type, pass the descriptor to the library. Otherwise scalarize the transfer. */ - if (expr->ref && !gfc_is_proc_ptr_comp (expr, NULL)) + if (expr->ref && !gfc_is_proc_ptr_comp (expr)) { for (ref = expr->ref; ref && ref->type != REF_ARRAY; ref = ref->next);
I have been confused more than once by the intent of gfc_trans_scalar_assign's r_is_var argument. This patch renames it to deep_copy, which, I think, describes it better. It also adds as comment the result of my investigations while working on this PR. OK? 2012-08-13 Mikael Morin <mikael@gcc.gnu.org> * trans-expr.c (gfc_trans_scalar_assign): Rename argument, extend comment. diff --git a/trans-expr.c b/trans-expr.c index c51eb7b..85289d2 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -6308,11 +6308,34 @@ gfc_conv_string_parameter (gfc_se * se) /* Generate code for assignment of scalar variables. Includes character strings and derived types with allocatable components. - If you know that the LHS has no allocations, set dealloc to false. */ + If you know that the LHS has no allocations, set dealloc to false. + + DEEP_COPY has no effect if the typespec TS is not a derived type with + allocatable components. Otherwise, if it is set, an explicit copy of each + allocatable component is made. This is necessary as a simple copy of the + whole object would copy array descriptors as is, so that the lhs's + allocatable components would point to the rhs's after the assignment. + Typically, setting DEEP_COPY is necessary if the rhs is a variable, and not + necessary if the rhs is a non-pointer function, as the allocatable components + are not accessible by other means than the function's result after the + function has returned. It is even more subtle when temporaries are involved, + as the two following examples show: + 1. When we evaluate an array constructor, a temporary is created. Thus + there is theoretically no alias possible. However, no deep copy is + made for this temporary, so that if the constructor is made of one or + more variable with allocatable components, those components still point + to the variable's: DEEP_COPY should be set for the assignment from the + temporary to the lhs in that case. + 2. When assigning a scalar to an array, we evaluate the scalar value out + of the loop, store it into a temporary variable, and assign from that. + In that case, deep copying when assigning to the temporary would be a + waste of resources; however deep copies should happen when assigning from + the temporary to each array element: again DEEP_COPY should be set for + the assignment from the temporary to the lhs. */ tree gfc_trans_scalar_assign (gfc_se * lse, gfc_se * rse, gfc_typespec ts, - bool l_is_temp, bool r_is_var, bool dealloc) + bool l_is_temp, bool deep_copy, bool dealloc) { stmtblock_t block; tree tmp; @@ -6346,9 +6369,9 @@ gfc_trans_scalar_assign (gfc_se * lse, gfc_se * rse, gfc_typespec ts, else if (ts.type == BT_DERIVED && ts.u.derived->attr.alloc_comp) { cond = NULL_TREE; - + /* Are the rhs and the lhs the same? */ - if (r_is_var) + if (deep_copy) { cond = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node, gfc_build_addr_expr (NULL_TREE, lse->expr), @@ -6364,7 +6387,7 @@ gfc_trans_scalar_assign (gfc_se * lse, gfc_se * rse, gfc_typespec ts, { tmp = gfc_evaluate_now (lse->expr, &lse->pre); tmp = gfc_deallocate_alloc_comp (ts.u.derived, tmp, 0); - if (r_is_var) + if (deep_copy) tmp = build3_v (COND_EXPR, cond, build_empty_stmt (input_location), tmp); gfc_add_expr_to_block (&lse->post, tmp); @@ -6378,7 +6401,7 @@ gfc_trans_scalar_assign (gfc_se * lse, gfc_se * rse, gfc_typespec ts, /* Do a deep copy if the rhs is a variable, if it is not the same as the lhs. */ - if (r_is_var) + if (deep_copy) { tmp = gfc_copy_alloc_comp (ts.u.derived, rse->expr, lse->expr, 0); tmp = build3_v (COND_EXPR, cond, build_empty_stmt (input_location),
This fixes a regression on proc_ptr_comp_15.f90 introduced by the next patch. The next patch sets gfc_trans_scalar_assign's deep_copy flag for pointer returning functions. It also handles typebound procedures and procedure pointer components, and for the latter looks at the pointer attribute of the procedure interface's return symbol. The problem in proc_ptr_comp_15.f90 is there is no result symbol for the following case: procedure(character(len=5)), pointer, nopass :: ptr I could have handled it in the next patch by looking at the interface's symbol if it hadn't any result, but I chose instead to copy the interface as result instead, which is what we do for regular functions. OK? 2012-08-13 Mikael Morin <mikael@gcc.gnu.org> * decl.c (match_ppc_decl): Copy the procedure interface's symbol as procedure interface's result. diff --git a/decl.c b/decl.c index 39c0493..dd684c5 100644 --- a/decl.c +++ b/decl.c @@ -5071,6 +5071,7 @@ match_ppc_decl (void) { c->ts = ts; c->ts.interface = gfc_new_symbol ("", gfc_current_ns); + c->ts.interface->result = c->ts.interface; c->ts.interface->ts = ts; c->ts.interface->attr.flavor = FL_PROCEDURE; c->ts.interface->attr.function = 1;
As explained before, this fixes the missing deep copy when assigning from a pointer function result by adding extra code in expr_is_variable to handle regular functions, procedure pointer, and typebound functions. I'm not very confident with the two latter ones (I hope I access the expression struct's fields correctly), but the testcase passes :-). OK? 2012-08-13 Mikael Morin <mikael@gcc.gnu.org> PR fortran/47586 * trans-expr.c (expr_is_variable): Handle regular, procedure pointer, and typebound functions returning a data pointer. diff --git a/trans-expr.c b/trans-expr.c index 85289d2..22317dc 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -6962,6 +6962,8 @@ static bool expr_is_variable (gfc_expr *expr) { gfc_expr *arg; + gfc_component *comp; + gfc_symbol *func_ifc; if (expr->expr_type == EXPR_VARIABLE) return true; @@ -6973,7 +6975,50 @@ expr_is_variable (gfc_expr *expr) return expr_is_variable (arg); } + /* A data-pointer-returning function should be considered as a variable + too. */ + if (expr->expr_type == EXPR_FUNCTION + && expr->ref == NULL) + { + if (expr->value.function.isym != NULL) + return false; + + if (expr->value.function.esym != NULL) + { + func_ifc = expr->value.function.esym; + goto found_ifc; + } + else + { + gcc_assert (expr->symtree); + func_ifc = expr->symtree->n.sym; + goto found_ifc; + } + + gcc_unreachable (); + } + + comp = gfc_get_proc_ptr_comp (expr); + if ((expr->expr_type == EXPR_PPC || expr->expr_type == EXPR_FUNCTION) + && comp) + { + func_ifc = comp->ts.interface; + goto found_ifc; + } + + if (expr->expr_type == EXPR_COMPCALL) + { + gcc_assert (!expr->value.compcall.tbp->is_generic); + func_ifc = expr->value.compcall.tbp->u.specific->n.sym; + goto found_ifc; + } + return false; + +found_ifc: + gcc_assert (func_ifc->attr.function + && func_ifc->result != NULL); + return func_ifc->result->attr.pointer; }
2012-08-13 Mikael Morin <mikael@gcc.gnu.org> PR fortran/47586 * gfortran.dg/typebound_proc_20.f90: Enable runtime test. * gfortran.dg/typebound_proc_26.f03: New test. diff --git a/gcc/testsuite/gfortran.dg/typebound_proc_20.f90 b/gcc/testsuite/gfortran.dg/typebound_proc_20.f90 index b63daf9..47c131c 100644 --- a/gcc/testsuite/gfortran.dg/typebound_proc_20.f90 +++ b/gcc/testsuite/gfortran.dg/typebound_proc_20.f90 @@ -1,5 +1,4 @@ -! { dg-do compile } -! TODO: make runtime testcase once bug is fixed +! { dg-do run } ! ! PR fortran/47455 !
Attachment:
typebound_proc_26.f03
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |