[Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
Tobias Burnus
burnus@net-b.de
Sun Mar 18 22:17:00 GMT 2012
Dear Paul,
thanks for the patch.
Paul Richard Thomas wrote:
> + /* Transfer the selector typespec to the associate name. */
> +
> + copy_ts_from_selector_to_associate (gfc_expr *expr1, gfc_expr *expr2)
> + {
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.)
> + 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:
dt%class(1:2)
class%class(1:2)
dt(1:2)%class
class(1:2)%class
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);
> ! else
> ! 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.
Tobias
More information about the Fortran
mailing list