Re: [Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
Dear Tobias,
Thanks for completing the review. I should be able to commit tonight.
> Thanks for the patch. I think it is OK.
>
> Regarding:
>
>> ! if (ref&& ref->type != REF_ARRAY&& seen_array)
>> ! {
>> ! gfc_error ("CLASS selector at %L is an array with CLASS "
>> ! "components; this is not allowed since the "
>> ! "elements could have different dynamic types",
>> ! &target->where);
>
>
> Could you open a PR for it? If possible with a test case.
select_type_28.f03 is that testcase (see below). I am not sure what
the PR would be for - surely such selectors make no logical sense?
Oddly I can see no such restriction in the standard. Indeed, there
seems to me to be an identical diffculty with pointer assignment.
Maybe a message to clf would be in order?
Cheers
Paul
Cheers
Paul
implicit none
type t0
integer :: j = 42
end type t0
type, extends(t0) :: t1
integer :: k = 99
end type t1
type t
integer :: i
class(t0), allocatable :: foo
end type t
type(t) :: m(4)
integer :: n
do n = 1, 2
allocate(m(n)%foo, source = t0(n*99))
end do
do n = 3, 4
allocate(m(n)%foo, source = t1(n*99, n*999))
end do
! The class components 'foo' of m(1:2) now have a different dynamic
type to those of m(3:4)
! An array of objects with ultimate class components cannot be a selector
! since each element could have a different dynamic type.
select type(bar => m%foo) ! { dg-error "is an array with CLASS components" }
type is(t0)
if (any (bar%j .ne. [99, 198, 297, 396])) call abort
type is(t1)
call abort
end select
end
Re: [Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
Dear Paul,
Paul Richard Thomas wrote:
Find attached a revised patch to fix PR 41600.
Thanks for the patch. I think it is OK.
Regarding:
! if (ref&& ref->type != REF_ARRAY&& seen_array)
! {
! gfc_error ("CLASS selector at %L is an array with CLASS "
!"components; this is not allowed since the "
!"elements could have different dynamic types",
! &target->where);
Could you open a PR for it? If possible with a test case.
Tobias
Re: [Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
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 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
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
PR fortran/41600
* gfortran.dg/select_type_26.f03 : New test.
* gfortran.dg/select_type_27.f03 : New test.
* gfortran.dg/select_typ
Re: [Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
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
