thanks for the patch.
Paul Richard Thomas wrote:
I think it is not obvious which type spec is which. Maybe you could
add a "(expr1)" and "(expr2)" in the comment. (Alternatively, one could
rename expr1 and expr2.)
+ /* Transfer the selector typespec to the associate name. */
+ copy_ts_from_selector_to_associate (gfc_expr *expr1, gfc_expr *expr2)
+ if (expr2->ts.type == BT_CLASS
+ && CLASS_DATA (expr2)->as
+ && expr2->ref&& expr2->ref->type == REF_ARRAY)
+ if (expr2->ref->u.ar.type == AR_FULL)
+ expr2->rank = CLASS_DATA (expr2)->as->rank;
+ else if (expr2->ref->u.ar.type == AR_SECTION)
+ expr2->rank = expr2->ref->u.ar.dimen;
I have a bad feeling about that one for code like:
I fear that at least one of those will fail. In any case, assuming that
- if the last ref is BT_CLASS - the expr->ref is the right one, looks
wrong. But I might have missed some fine print and it is guaranteed to
be always the correct.
+ /* Logic is a LOT clearer with separate functions for class and derived
+ type temporaries! There are not many more lines of code either. */
if (ts->type == BT_CLASS)
! tmp = select_class_set_tmp (ts);
! tmp = select_derived_set_tmp (ts);
While I concur with the comment, I think one should remove it. As patch
explanation it makes sense, but as committed it is not helpful.
gfc_add_type (tmp->n.sym, ts, NULL);
! /* Copy across the array spec to the selector, taking care as to
! whether or not it is a class object or not. */
The indention looks wrong.
(iii) The error that is thrown in resolve_assoc_var is necessary
because wrong code is produced at the moment since the size of the
declared type, rather than the dynamic type, is used for allocation of
the temporary. The necessary machinery is in place to fix this and I
will do so soon
I assume that's:
! gfc_error ("CLASS selector at %L needs a temporary which is not "
! "yet implemented",&target->where);
But I think one should also look into:
! TODO Understand why class scalar expressions must be excluded. */
! if (sym->assoc&& !(sym->ts.type == BT_CLASS&& e->rank == 0))
Overall, the patch looks okay - I am just unsure about the expr2->ref
usage in copy_ts_from_selector_to_associate.