Re: [patch, fortran] Really fix PR 56782

[Mikael Morin <mikael dot morin at sfr dot fr>]

Hello,

Le 14/04/2013 11:57, Thomas Koenig a écrit :
> Hello world,
> 
> the attached patch completely fixes the regression,
> PR 56782.
>
typo: it's PR 56872 everywhere.

> Regression-tested.  OK for trunk and 4.8?
> 
>     Thomas
> 
> 2013-04-14  Thomas Koenig  <tkoenig@gcc.gnu.org>
> 
>         PR fortran/56782
>         * frontend-passes.c (copy_walk_reduction_arg):  Do not
>         call the expression walker with callback_reduction.
>         (insert_iterator_function):  New function.
>         (callback_reduction):  If an iterator is present, call
>         insert_iterator_function and reset the iterator on the
>         original array iterator.
> 
> 2013-04-08  Thomas Koenig  <tkoenig@gcc.gnu.org>
> 
>         PR fortran/56782
>         * gfortran.dg/array_constructor_45.f90:  New test.
>         * gfortran.dg/array_constructor_46.f90:  New test.
>         * gfortran.dg/array_constructor_40.f90:  Adjust number of
>         while loops.

> Index: fortran/frontend-passes.c
> ===================================================================
> --- fortran/frontend-passes.c	(Revision 197610)
> +++ fortran/frontend-passes.c	(Arbeitskopie)
> @@ -221,8 +221,47 @@ copy_walk_reduction_arg (gfc_expr *e, gfc_expr *fn
>        fcn->symtree->n.sym->attr.access = ACCESS_PRIVATE;
>      }
>  
> -  (void) gfc_expr_walker (&fcn, callback_reduction, NULL);

why remove this?

> +  return fcn;
> +}
>  
> +/* Auxiliary function to create function with an an array expression with
> +   iterator argument.  */
> +
> +static gfc_expr *
> +insert_iterator_function (gfc_expr *e, gfc_expr *fn, gfc_iterator *iterator)
> +{

[...]

> +
> +  if (id == GFC_ISYM_SUM || id == GFC_ISYM_PRODUCT)
> +    fcn = gfc_build_intrinsic_call (current_ns,
> +				    fn->value.function.isym->id,
> +				    fn->value.function.isym->name,
> +				    fn->where, 3, new_expr,
> +				    NULL, NULL);
> +  else if (id == GFC_ISYM_ANY || id == GFC_ISYM_ALL)
> +    fcn = gfc_build_intrinsic_call (current_ns,
> +				    fn->value.function.isym->id,
> +				    fn->value.function.isym->name,
> +				    fn->where, 2, new_expr,
> +				    NULL);
> +  else
> +    gfc_internal_error ("Illegal id in insert_iterator_function");

This duplicated code could probably be merged with
copy_walk_reduction_arg.  I think callback_reduction's iterator handling
should happen there as well.
Like this:

copy_walk_reduction_arg (...)
{
  fcn = gfc_copy_expr (c->expr);
  gfc_expr_walker (&fcn, callback_reduction, NULL);
  if (c->iterator)
    {
      /* wrap in array constructor  */
    }

  if (!scalar)
    {
      /* wrap in intrinsic call */
    }

  return ...;
}


Mikael