Dear Mikael,
Thanks for the usually thorough review.
....snip....
> And here comes the next round of comments.
....snip....
>> + e->rank = c && c->as ? c->as->rank : 0;
> This is bogus if e->rank was != 0 previously (think of the case
> array(:)%scalar_comp).
mistaken maybe but not bogus!
> The c == NULL case should be handled at the beginning (if at all).
>
>> + if (e->rank)
> the condition should be on c->as (for the case array(:)%scalar_comp again).
OK point taken.
....snip...
>> + this_code->op = op;
> all calls are with op == EXEC_ASSIGN, you may as well hardcode it.
I thought to leave it general so that the function could be reused for
other purposes.
....snip...
.
>> + gcc_assert (e->expr_type == EXPR_VARIABLE
>> + || e->expr_type == EXPR_FUNCTION);
> As far as I know anything can be used, not only variables and functions.
> The derived type cases are a bit specific but at least array/structure
> constructors are missing. There could be also typebound function calls
> (I never know whether they are EXPR_FUNCTION or something else).
The reason for this assert is the later use of e->symtree. I'll see
what I can do to generalise it.
....snip....
> I guess it should be `t1%cmp {defined=} expr2%cmp'?
mmmm..... it might just be
>> + || comp1->attr.proc_pointer_comp
> That one doesn't look right.
Why not?
> `this_code' should be cleared, otherwise it is used in the next iteration.
I'll check that this is not done in gfc_free_statements (no source to
hand at the moment) - I believe that it is.
....snip...
>> + = super_type->attr.defined_assign_comp;
> I guess Tobias' reported bug is here. The flag shouldn't be cleared
> here if it was set just before.
>
I am sure that it is in this vicinity.... :-)
>
> To finish, I would like to draw your attention on the scalarizer not
> supporting multiple arrays in the reference chain. The initial
> expressions are guaranteed to have at most one array in the chain, but
> as we add subfield references, that condition can not remain true. We
> could try adding multiple references support in the scalarizer, but I
> don't know how difficult it would be. Or maybe better fix it at the
> front-end AST level by using elemental functions to split the
> scalarization work. Or something else. What do you think?
resolve_expr punts on this, does it not? I'll check.
I cannot conceivably come back to this for a week or so because
daytime and private life are overwhelmingly hectic (wife and daughter
moving back to UK).
Thanks again
Paul
