On 29/08/2012 21:53, Tobias Burnus wrote: > Dear all, > > that's the revised version of patch at > http://gcc.gnu.org/ml/fortran/2012-08/msg00095.html, taking the review > comments into account. > > Reminder: This patch only generates the finalization wrapper, which is > in the virtual table. It does not add the required calls; hence, it > still doesn't allow to use finalization. > > > The patch consists of three parts: > > a) The main patch, which implements the wrapper. > I am asking for approval for that patch. A few more nitpicks below.
> > b) A patch which removes the gfc_error "not yet implemented" > I suggest to only remove the error after finalization calls have been > added Sensible. By the way some (testsuite) parts of b) should be part of a). > > c) A patch which bumps the .mod version > - or alternatively - > a patch which disables the _final generation in the vtable. > > I have build and regtested (on x86-64-linux) the patch with (a) and > (a)+(b) applied. > > > I would like to include the patch (c) as modifying the vtable changes > the ABI. Bumping the .mod version is a reliable way to force > recompilation. The alternative is to wait until the final FINAL patch > before bumping the .mod version (and disable the "_final" generation). I don't like bumping the module version, for something not module-related (vtypes are output as normal types in the module files), but I guess it is the most user-friendly thing to do. > > One possibility, if deemed useful, is to combine the .mod version bump > with backward compatible reading of .mod files, i.e., only error out > when BT_CLASS is encountered in an old .mod file. Let's keep things simple, let's not do that. > > > Is the patch (a) OK for the trunk? With which version of (c)? > > (I am slightly inclined to do the .mod bump now. As a follow up, one can > also commit Janus' proc-pointer patch, > http://gcc.gnu.org/ml/fortran/2012-04/msg00033.html, though I think > someone has still to review it.) I am inclined to disable finalization completely (thus defer .mod bump), because the new code is hardly tested and doesn't provide (yet) new benefits such as reduced memory leaks as it is disabled. We could do the bump now, but I'm afraid that we discover a bug when finalization gets completely implemented, and we have to bump again to fix it (though I don't see what kind of bug it could be). I think Janus' patch is a much less serious problem in the sense that people trying to link codes compiled with patched and non-patched compiler will get a link error. I don't think it's worth a .mod bump. Unless I miss something. For (a), I noticed a few indenting issues (8+ spaces) and (mostly wording) nits below to be fixed. Then OK. > diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c > index 21a91ba..9d58aab 100644 > --- a/gcc/fortran/class.c > +++ b/gcc/fortran/class.c > @@ -689,6 +694,702 @@ copy_vtab_proc_comps (gfc_symbol *declared, gfc_symbol > *vtype) > } > > > +/* Returns true if any of its nonpointer nonallocatable components or > + their nonpointer nonallocatable subcomponents has a finalization > + subroutine. */ > + > +static bool > +has_finalizer_component (gfc_symbol *derived) Rename to has_finalizable_component ? > +{ > + gfc_component *c; > + > + for (c = derived->components; c; c = c->next) > + { > + if (c->ts.type == BT_DERIVED && c->ts.u.derived->f2k_derived > + && c->ts.u.derived->f2k_derived->finalizers) > + return true; > + > + if (c->ts.type == BT_DERIVED > + && !c->attr.pointer && !c->attr.allocatable > + && has_finalizer_component (c->ts.u.derived)) > + return true; > + } > + return false; > +} > + > + > +/* Call DEALLOCATE for the passed component if it is allocatable, if it is > + neither allocatable nor a pointer but has a finalizer, call it. If it > + is a nonpointer component with allocatable or finalizes components, walk s/finalizes/finalizable/ ? > + them. Either of the is required; other nonallocatables and pointers aren't s/the/them/ ? > + handled gracefully. > + Note: The DEALLOCATE handling takes care of finalizers, coarray > + deregistering and allocatable components of the allocatable. */ "coarray deregistering and allocatable components" is confusing. Note: If the component is allocatable, the DEALLOCATE handling takes care of calling the appropriate finalizer(s), of coarray deregistering, and of deallocating allocatable (sub)components. > + > +static void > +finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, > + gfc_expr *stat, gfc_code **code) [...] > + > + > +/* Generate the wrapper finalization/polymorphic freeing subroutine for the Difficult to read. "Generate the finalization/polymorphic freeing wrapper subroutine..." or something ? > + derived type "derived". The function first calls the approriate FINAL > + subroutine, then it DEALLOCATEs (finalizes/frees) the allocatable > + components (but not the inherited ones). Last, it calls the wrapper > + subroutine of the parent. The generated wrapper procedure takes as > argument > + an assumed-rank array. > + If neither allocatable components nor FINAL subroutines exists, the vtab > + will contain a NULL pointer. */ > + > +static void > +generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, > + const char *tname, gfc_component *vtab_final) > +{ > + gfc_symbol *final, *array, *nelem; > + gfc_symbol *ptr = NULL, *idx = NULL; > + gfc_component *comp; > + gfc_namespace *sub_ns; > + gfc_code *last_code; > + char name[GFC_MAX_SYMBOL_LEN+1]; > + bool finalizable_comp = false; > + gfc_expr *ancestor_wrapper = NULL; > + > + /* Search for the ancestor's finalizers. */ > + if (derived->attr.extension && derived->components > + && (!derived->components->ts.u.derived->attr.abstract > + || has_finalizer_component (derived))) > + { > + gfc_symbol *vtab; > + gfc_component *comp; > + > + vtab = gfc_find_derived_vtab (derived->components->ts.u.derived); > + for (comp = vtab->ts.u.derived->components; comp; comp = comp->next) > + if (comp->name[0] == '_' && comp->name[1] == 'f') > + { > + ancestor_wrapper = comp->initializer; > + break; > + } > + } > + > + /* No wrapper of the ancestor and no own FINAL subroutines and > + allocatable components: Return a NULL() expression. */ > + if ((!ancestor_wrapper || ancestor_wrapper->expr_type == EXPR_NULL) > + && !derived->attr.alloc_comp > + && (!derived->f2k_derived || !derived->f2k_derived->finalizers) > + && !has_finalizer_component (derived)) > + { > + vtab_final->initializer = gfc_get_null_expr (NULL); > + return; > + } > + > + /* Check whether there are new allocatable components. */ > + for (comp = derived->components; comp; comp = comp->next) > + { > + if (comp == derived->components && derived->attr.extension > + && ancestor_wrapper && ancestor_wrapper->expr_type != EXPR_NULL) > + continue; > + > + if (comp->ts.type != BT_CLASS && !comp->attr.pointer > + && (comp->attr.alloc_comp || comp->attr.allocatable > + || (comp->ts.type == BT_DERIVED > + && has_finalizer_component (comp->ts.u.derived)))) > + finalizable_comp = true; > + else if (comp->ts.type == BT_CLASS && CLASS_DATA (comp) > + && CLASS_DATA (comp)->attr.allocatable) > + finalizable_comp = true; > + } > + > + /* If there is no new finalizer and no new allocatable, return with > + an expr to the ancestor's one. */ > + if ((!derived->f2k_derived || !derived->f2k_derived->finalizers) > + && !finalizable_comp) > + { > + vtab_final->initializer = gfc_copy_expr (ancestor_wrapper); > + return; > + } > + > + /* We now create a wrapper, which does the following: > + 1. It calls the suitable finalization subroutine for this type s/It calls/Call/ ? (to be in line with the other verbs). > + 2. In a loop over all noninherited allocatable components and > noninherited s/In a loop/Loop/ > + components with allocatable components and DEALLOCATE those; this will > + take care of finalizers, coarray deregistering and allocatable > + nested components. > + 3. Call the ancestor's finalizer. */ > + > + /* Declare the wrapper function; it takes an assumed-rank array > + as argument. */ > + Thanks. Mikael
