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-> == AR_FULL)
+       expr2->rank = CLASS_DATA (expr2)->as->rank;
+       else if (expr2->ref-> == AR_SECTION)
+       expr2->rank = expr2->ref->;
+     }

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);
!   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.


Reply via email to