Hi Mikael, hi all,

thanks for the review. I have made some changes. Answers to your questions are
inline below.

On Sun, 19 Apr 2015 12:01:23 +0200
Mikael Morin <mikael.morin@sfr.fr> wrote:

<snip>
> > I was pointed to the patch in comment #44 of pr61831 which seemingly fixes
> > the #3 comment of pr58586, too, but causes a memory leak. I therefore like
> > to point out, that adding the a->expr.expr_type != EXPR_STRUCTURE of
> > Mikael's patch in pr61831 should not be added to
> > trans-expr.c::gfc_conv_procedure_call (), when this patch for 58586 is
> > applied.
> Note that I plan to submit the pr61831 patch soon, and that the comment
> #44 patch doesn't have the a->expr.expr_type != EXPR_STRUCTURE (in
> opposition to precedent patches).
> I hope that means the patches are compatible. ;-)

I have tested the code in the comments of pr61831 with v2 of this patch and got
no issues.

> > diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
> > index 9e6432f..80dfed1 100644
> > --- a/gcc/fortran/trans-expr.c
> > +++ b/gcc/fortran/trans-expr.c
> > @@ -5344,8 +5344,19 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol *
> > sym, && (e->expr_type != EXPR_VARIABLE && !e->rank))
> >          {
> >  	  int parm_rank;
> > -	  tmp = build_fold_indirect_ref_loc (input_location,
> > -					 parmse.expr);
> > +	  /* It is known the e returns a structure type with at least one
> > +	     allocatable component.  When e is a function, ensure that the
> > +	     function is called once only by using a temporary variable.
> > */
> > +	  if (e->expr_type == EXPR_FUNCTION)
> > +	    parmse.expr = gfc_evaluate_now_loc (input_location,
> > +						parmse.expr, &se->pre);
> You need not limit this to functions only.
> I think you can even do this without condition.

Yes, one could do that, but then an unnecessary temporary variable in the - for
my taste - already too clobbered pseudo code is introduced. Furthermore, is the
penalty on doing the check for a function negligible. I therefore have not
changed that.

> > +	  if (POINTER_TYPE_P (TREE_TYPE (parmse.expr)))
> This distinguishes arguments with/without value attribute, right?
> I think it's better to use the frontend information here (fsym->attr.value).

Changed to check for value.

> Ah, and don't forget to provide a ChangeLog entry with it.

The Changelog entry comes in an additional attachment. 

Version 2 of this patch adds a chunk to resolve.c, where results of
functions that are defined in a module, but are not referenced there, are
now marked referenced when they use allocatable or pointer components.
Furthermore, does the chunk prevent duplicate pseudo code generation. The
former code adds a EXPR_INIT_ASSIGN and then gfc_generate_function_code ()
does nearly the same again. I fixed this in both place. I also have added a
test to check this. 

The chunks in trans-decl.c take care to have variable/result declaration and
initialize it properly. For this I had to make gfc_trans_structure_assign ()
public to the trans-stage.

Bootstraps and regtests ok on x86_64-linux-gnu/F21.

Ok, for trunk?

Regards,
	Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de