Re: [Patch, fortran] PR46897 - [OOP] type-bound defined ASSIGNMENT(=) not used for derived type component in intrinsic assign

[Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>]

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