Hi,

>> one thing that I do not like about your patch is the modification of
>> "gfc_find_derived_vtab": You create two versions of it, one of which creates
>> the vtab if it does not exist, while the other version does not do this.
>> [...] can you explain to me why this would be necessary?
>
>
> Well, strictly speaking it is not necessary. However, I use it in the
> to-be-submitted calling part of the patch:
>
>           else if (al->expr->ts.type == BT_DERIVED)
>             {
>               gfc_symbol *vtab = gfc_find_derived_vtab
> (al->expr->ts.u.derived);
>               if (vtab)
>
> Here, I do not want to force the generation of a vtab which wouldn't
> otherwise exist. Otherwise, one had to at least guard it by checks for
> nonextensible derived types (sequence, bind(C)).

I don't think it is a good idea to base the decision whether to call a
finalizer on the presence of a vtab. In my version of the patch I
introduced a routine 'gfc_is_finalizable' to perform this decision.


>> [Moreover, the problem is that your new "gfc_find_derived_vtab" behaves
>> different from the old one but has the same name, while your new
>> "gfc_get_derived_vtab" behaves like the old "gfc_find_derived_vtab".
>
>
> That's because of the bad choice of the current name. The other "find"
> functions do not generate the symbol if it does not exist, the "get"
> functions do. But otherwise I concur that changing the name is confusing.
>
>
>> Therefore, the places where you change the behavior by keeping the call to
>> "gfc_find_derived_vtab" are not visible in the patch.
>
>
> That should not happen. When I created the patch, I first renamed all
> existing versions, though it seems as if I there are currently three new
> ones which the current patch misses.
>
> However, if you insist on the current meaning, can you provide a good name?
> Otherwise, I could use gfc_really_find_derived_vtab ;-)

I do not oppose to renaming gfc_find_derived_vtab to
gfc_get_derived_vtab. My main point is that we do not need a variant
which only searches for the vtab but does not generate it.

Cheers,
Janus

Reply via email to