[Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported

[Paul Richard Thomas]

Dear Tobias, dear all,

Please accept my apologies for the long delay in responding to the
review.  A combination of overwhelming daytime works and a complete
failure of my workstation at home have knocked me out for the last six
weeks.

Find attached a revised patch to fix PR 41600.

On Sun, Mar 18, 2012 at 11:16 PM, Tobias Burnus <burnus@net-b.de> wrote:
> 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.)

Done - expr1 and expr2 are re
>
>> +   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.

This has been improved to ensure that the references are correctly treated.

Note that array_ref%class is now excluded, except for scalars; see
select_type_28.f03
Select_type_27.f03 deals with class%array_ref

>

>> +   /* 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.

Done

>
>>     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.

FIxed

>
>
>> (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))

I still do not see this but undertake to fix/understand.

>
>
> Overall, the patch looks okay - I am just unsure about the expr2->ref usage
> in copy_ts_from_selector_to_associate.

Thanks for the review - I hope that the new version is satisfactory.

Cheers

Paul

See above.
2012-05-01  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/41600
	* trans-array.c (build_array_ref): New static function.
	(gfc_conv_array_ref, gfc_get_dataptr_offset): Call it.
	* trans-expr.c (gfc_get_vptr_from_expr): New function.
	(gfc_conv_derived_to_class): Add a new argument for a caller
	supplied vptr and use it if it is not NULL.
	(gfc_conv_procedure_call): Add NULL to call to above.
	symbol.c (gfc_is_associate_pointer): Return true if symbol is
	a class object.
	* trans-stmt.c (trans_associate_var): Handle class associate-
	names.
	* expr.c (gfc_get_variable_expr): Supply the array-spec if
	possible.
	* trans-types.c (gfc_typenode_for_spec): Set GFC_CLASS_TYPE_P
	for class types.
	* trans.h : Add prototypes for gfc_get_vptr_from_expr and
	gfc_conv_derived_to_class. Define GFC_CLASS_TYPE_P.
	* resolve.c (resolve_variable): For class arrays, ensure that
	the target expression has all the necessary _data references.
	(resolve_assoc_var): Throw a "not yet implemented" error for
	class array selectors that need a temporary.
	* match.c (copy_ts_from_selector_to_associate,
	select_derived_set_tmp, select_class_set_tmp): New functions.
	(select_type_set_tmp): Call one of last two new functions.
	(gfc_match_select_type): Copy_ts_from_selector_to_associate is
	called if associate-name is typed.

2012-05-01  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/41600
	* gfortran.dg/select_type_26.f03 : New test.
	* gfortran.dg/select_type_27.f03 : New test.
	* gfortran.dg/select_type_28.f03 : New test.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: resubmit.diff
Type: text/x-diff
Size: 31799 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20120501/fcd1f2a2/attachment.bin>