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