This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch,Fortran] Add polymorphic type support (OOP, RTTI) - merge from Fortran-dev branch


Hi Tobias,

>>> attached patch adds more OOP; it is just the difference between the 4.5
>>> trunk and the Fortran-dev branch (with one additional line).
>
> I have now committed the unmodified patch to the trunk as Rev. 152345.

thanks a lot for taking care of the merge. I will have a look at your
comments later (I will be back home tomorrow).

Also: Now that all the OOP stuff has landed on trunk, I would
encourage all interested users (Salvatore, Damian, anyone else?) to
test the current implementation, so that we can quickly fix the
remaining bugs.

Cheers,
Janus



> ?* * *
>
> Some comments - (a)-(d) go mostly to Janus, the comment afterwards
> mostly to Paul.
>
> a) SAME_TYPE_AS and EXTENDS_TYPE_OF: I think one could add a
> compile-time simplifier.
>
> b) As shown my posted test program, the following often does not work
> (nested select_type blocks, ...). I think one could have a static global
> linked list, which gets pushed the current variable when entering the
> block and popped when leaving. In the block one then walks through the
> list of variables. And instead of checking the current state, one simply
> checks whether the linked list is NULL.
>
> + ?/* Special case: If we're in a SELECT TYPE block,
> + ? ? replace the selector variable by a temporary. ?*/
> + ?if (gfc_current_state () == COMP_SELECT_TYPE
> + ? ? ?&& st && st->n.sym == type_selector)
> + ? ?st = select_type_tmp;
> +
>
> c) Allocation of abstract types requires a type spec or a SOURCE
> type, abstract :: t
> end type t
> class(t), allocatable :: a
> allocate(a) ! missing type-spec/source=
> allocate(a, source=b) ! Wrong: b is abstract
> ? ? ? ? ? ? ? ? ? ? ?! though only checkable at runtime
> end
>
> ?* * *
>
> Similarly to Daniel, I do not think that the current vtable
> implementation is sufficient. For instance the following program fails
> to compile although it is valid - here all items are "TYPE IS" and all
> types are known via use association, but still it fails.
>
> ========= File: bb.f90 =========
> module m
> type t
> end type t
> end module m
> ========= File: cc.f90 =========
> module m2
> ?use m
> ?type, extends(t) :: t2
> ?end type t2
> end module m2
> ========= File: dd.f90 =========
> use m
> use m2
> type,extends(t) :: t3
> end type t3
>
> integer :: i
> class(t), allocatable :: a
> allocate(t3 :: a)
> select type(a)
> ?type is(t)
> ? ?i = 1
> ?type is(t2)
> ? ?i = 2
> ?type is(t3)
> ? ?i = 3
> end select
> print *, i
> end
> ========================
>
> The error message is:
>
> $ gfortran bb.f90 cc.f90 dd.f90
> dd.f90:10.10:
>
> ?type is(t)
> ? ? ? ? ?1
> dd.f90:12.10:
>
> ?type is(t2)
> ? ? ? ? ?2
> Error: CASE label at (1) overlaps with CASE label at (2)
>
> ?* * *
>
> Tobias
>
> PS: Daniel's comments should also be answered:
>> here are some comments while reading through the patch:
>>
>> + ? ? ?if (argss == gfc_ss_terminator)
>> + ? ? ? ? ? ?{
>> + ? ? ? ? ?gfc_conv_expr_reference (&parmse, e);
>> + ? ? ? ? ?tmp = fold_convert (TREE_TYPE (data),
>> + ? ? ? ? ? ? ? ? ?parmse.expr);
>> + ? ? ? ? ?gfc_add_modify (&parmse.pre, data, tmp);
>> + ? ? ? ?}
>>
>> It seems the white-space of the opening curly brace is messed up.
>>
>>
>> + ?if (derived->attr.extension)
>> + ? ?return gfc_get_ultimate_derived_super_type (derived);
>> + ?else
>> + ? ?return derived;
>>
>> I like the tail-recursive style here instead of a loop :)
>>
>> + ?(*as) = NULL; ?/* XXX */
>>
>> What's about this one?
>>
>> +/* Counter for assigning a unique vindex number to each derived
>> type. ?*/
>> +static int vindex_counter = 0;
>> +
>> +
>> ?/* Match the beginning of a derived type declaration. ?If a type name
>> ? ? was the result of a function, then it is possible to have a symbol
>> ? ? already to be known as a derived type yet have no components. ?*/
>> @@ -6823,6 +6896,10 @@ gfc_match_derived_decl (void)
>> ? ? ? ?st->n.sym = sym;
>> ? ? ?}
>>
>> + ?if (!sym->vindex)
>> + ? ?/* Set the vindex for this type and increment the counter. ?*/
>> + ? ?sym->vindex = ++vindex_counter;
>> +
>>
>> I'm not really sure about the way vindices are calculated and this
>> looks quite fine... ?But is this safe even with types in multiple
>> modules combined and stuff like that? ?Or different compilation units?
>>
>> + ?if (!pointer && !proc_pointer
>> + ? ?&& !(lvalue->ts.type == BT_CLASS
>> + ? ? ? ?&& lvalue->ts.u.derived->components->attr.pointer))
>>
>> I'm not sure how you handle this, but maybe you could set the
>> attributes on the container to the same as the declared type component
>> and get rid of checks like that? ?Would that work?
>>
>> + ?if (sym->ts.type == BT_CLASS)
>> + ? ?{
>> + ? ? ?allocatable = sym->ts.u.derived->components->attr.allocatable;
>> + ? ? ?pointer = sym->ts.u.derived->components->attr.pointer;
>> + ? ?}
>> + ?else
>> + ? ?{
>> + ? ? ?allocatable = sym->attr.allocatable;
>> + ? ? ?pointer = sym->attr.pointer;
>> + ? ?}
>>
>> Ditto (and some others like that).
>>
>> + ? ?case EXEC_SELECT_TYPE:
>> + ? ? ?/* Do nothing. SELECT TYPE statements should be transformed into
>> + ? ? ?an ordinary SELECT CASE at resolution stage.
>> + ? ? ?TODO: Add an error message here once this is done. ?*/
>> + ? ? ?res = NULL_TREE;
>> + ? ? ?break;
>>
>> What's with this, shouldn't there be an unreachable or assertion, now
>> that I assume the transformation is really already done?
>>
>> + ? ? ?/* Kill the dead block, but not the blocks below it. ?*/
>>
>> ...which raises the important question, if something dead can be
>> killed? ?Just ignore me here, though :)
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]