Re: [Patch/gfortran] PR18283

[Paul Thomas <paulthomas2 at wanadoo dot fr>]

Tobi,

> How shall we proceed? The standard procedure would be to back out the 
> patch
> until it's final, peer-reviewed incarnation is ready, but I don't 
> doubt that
> it's correct, and if we do this quickly, the need will probably not arise.

This is fine by me, especially since I am turning into a pumpkin on 
Wednesday night and will not be back in town for three weeks.

> So, can you cook up a patch which addresses the issues that I had and 
> post a
> diff against trans-expr.c rev. 1.44 and trans-array.c rev. 1.45 (i.e. 
> before
> your commit), so that I can review the whole thing thoroughly?
This will have to be done tomorrow night.  I am trying to meet a 
deadline on the daytime work.

Before I do this patch, I just want to pass a couple of remarks on your 
comments:

+> + {
> + /* Dereference non-charcter scalar dummy arguments. */
> + if ((sym->attr.dummy) && (!sym->attr.dimension))
> + se->expr = gfc_build_indirect_ref (se->expr);
> +
> + /* Dereference scalar hidden result. */
> + if ((gfc_option.flag_f2c && sym->ts.type == BT_COMPLEX)
> + && (sym->attr.function || sym->attr.result)
> + && (!sym->attr.dimension))
> + se->expr = gfc_build_indirect_ref (se->expr);
> +
> + /* Dereference non-character pointer variables.
> + These must be dummys or results or scalars. */
> + if ((sym->attr.pointer || sym->attr.allocatable)
> + && ((sym->attr.dummy)
> + || (sym->attr.function || sym->attr.result)
> + || (!sym->attr.dimension)))
> + se->expr = gfc_build_indirect_ref (se->expr);
> + }
> +
> ref = expr->ref;
> }
>

I think you're using parentheses much too liberally, making the code 
harder to
read.

I partially disagree. Not only do I find the code easier to understand 
with the parenthesis but it is completely unambiguous. In fact, the 
compiler suggested many of them for exactly this reason.  We obviously 
see code in a completely different way.  I remember being struck by 
this, when you commented on the first version of this section of code. 
However, I can agree that the singleton variables can be left 
unbracketed - it doesn't make it any easier to read though.....

> ! /* Build char[4] * pstr. */

Shouldn't that be char[0:len-1] in both comments instead of 4?

I agree. I stared too long at the code from the test case!

Best regards

Paul T