Re: [PATCH,gfortran] Fix PR 16222

[Paul Brook <paul at codesourcery dot com>]

On Sunday 28 November 2004 00:51, Steve Kargl wrote:
> The attached patch fixes PR 16222.  Bootstrapped
> and regression tested on i386-unknown-freebsd6.0.
>
> 2004-11-24  Steven G. Kargl  <kargls@comcast.net>
>
>         PR 16222
>         * gfortran.h (gfc_resolve_do_iterator): Add prototype
>         * resolve.c (gfc_resolve_do_iterator): New function; use it
>         * trans-stmt.c (gfc_trans_do): Allow REAL iterator

Duplicating gfc_resolve_iterator seems a bad idea.

I think it would be better to add an extra boolean argument to 
gfc_resolve_iterator which says whether read arguments are allowed.
You may also want to factor out the individual variable checks into a common 
routine.

Are you deliberately allowing a mixture of integer and real types? This seems 
like it deserves at least a -Wsuprising/-Wconversion warning.
This conversion should be performed during resolution, not in trans-stmt.c 
(this is a pre-existing bug).

You should also update the comment(s) in gfc_trans_do. Remove the TODO, and 
mention how we handle this case - make the conversion explicit in the 
pseudocode.

>    /* Decrement the loop count.  */
> +  if (TREE_CODE (type) == INTEGER_TYPE)
>    tmp = build2 (MINUS_EXPR, type, count, gfc_index_one_node);
> +  else
> +    tmp = build2 (MINUS_EXPR, gfc_array_index_type, count, 
gfc_index_one_node);
>    gfc_add_modify_expr (&body, count, tmp);

The existing code is wrong, and your code is overly complicated. Try:

tree count_one = build_int_cst (TREE_TYPE (count), 1);
...
tmp = build2 (MINUS_EXPR, TREE_TYPE (count), count, count_one);

Paul