On Sat, 6 Nov 2021 13:04:07 +0100 Mikael Morin wrote: > Le 05/11/2021 à 23:08, Bernhard Reutner-Fischer a écrit : > > On Fri, 5 Nov 2021 19:46:16 +0100 > > Mikael Morin wrote: > > > >> Le 29/10/2021 à 01:58, Bernhard Reutner-Fischer via Fortran a écrit : > >>> On Wed, 27 Oct 2021 23:39:43 +0200 > >>> Bernhard Reutner-Fischer wrote: > >>> > >>>> On Mon, 15 Oct 2018 10:23:06 +0200 > >>>> Bernhard Reutner-Fischer wrote: > >>>> > >>>>> If a finalization is not required we created a namespace containing > >>>>> formal arguments for an internal interface definition but never used > >>>>> any of these. So the whole sub_ns namespace was not wired up to the > >>>>> program and consequently was never freed. The fix is to simply not > >>>>> generate any finalization wrappers if we know that it will be unused. > >>>>> Note that this reverts back to the original r190869 > >>>>> (8a96d64282ac534cb597f446f02ac5d0b13249cc) handling for this case > >>>>> by reverting this specific part of r194075 > >>>>> (f1ee56b4be7cc3892e6ccc75d73033c129098e87) for PR fortran/37336. > >>>>> > >> I’m a bit concerned by the loss of the null_expr’s type interface. > >> I can’t convince myself that it’s either absolutely necessary or > >> completely useless. > > > > It's a delicate spot, yes, but i do think they are completely useless. > > If we do NOT need a finalization, the initializer can (and has to be > > AFAIU) be a null_expr and AFAICS then does not need an interface. > > > Well, the null pointer itself doesn’t need a type, but I think it’s > better if the pointer it’s assigned to has a type different from void*. > It will (hopefully) help the middle-end optimizers downstream. I would not expect this to help all that much or at all TBH. So i compiled for i in $(grep -li final $(grep -L dg-error /scratch/src/gcc-12.mine/gcc/testsuite/gfortran.dg/*.f*)); do gfortran -O2 -fcoarray=single -c $i -g -g3 -ggdb3 -fdump-tree-original -fdump-tree-optimized;done and diffed all .original and .optimized dumps against pristine trunk and they are identical. I inspected and ran the binary from finalize_14 and there is no change in the leaks compared to pristine trunk. The 3 shape_w in p leak as they used to. I do remember that finalize_14 was a good testcase, in sum i glared at it for quite some time ;) > > I will see if I can manage to create a testcase where it makes a > difference (don’t hold your breath, I don’t even have a bootstrapped > compiler ready yet). > That'd be great, TIA! [] btw.. Just because it's vagely related. I think f8add009ce300f24b75e9c2e2cc5dd944a020c28 for PR fortran/88009 (ICE in find_intrinsic_vtab, at fortran/class.c:2761) is incomplete in that i think all the internal class helpers should be flagged artificial. All these symbols built in gfc_build_class_symbol, generate_finalization_wrapper, gfc_find_derived_vtab etc. Looking at the history it seems the artificial bit often was forgotten. And most importantly i think it is not correct to ignore artificial in gfc_check_conflict! I'm attaching my notes on this to illustrate what i mean. Not a patch, even if it regtests cleanly.. The hunk in gfc_match_derived_decl() plugs another leak by first checking if the max extension level is reached before adding the component. Maybe i should split that hunk out. Similar to the removal of *head in gfc_match_derived_decl, there's another spot in gfc_match_decl_type_spec which should get rid of the *head and just wire the interface up as usual. Just cosmetics. Several tests do exercise this code: alloc_comp_class_1.f90, class_19.f03 and 62, unlimited_polymorphic_8.f90 and others. > >> The rest of the changes (appart from class.c) are mostly OK with the nit > >> below and should be put in their own commit. > >> > >> >>> @@ -3826,10 +3828,8 @@ free_tb_tree (gfc_symtree *t) > >> >>> > >> >>> free_tb_tree (t->left); > >> >>> free_tb_tree (t->right); > >> >>> - > >> >>> - /* TODO: Free type-bound procedure structs themselves; probably > >> needs some > >> >>> - sort of ref-counting mechanism. */ > >> >>> free (t->n.tb); > >> > >> Please keep a comment; it remains somehow valid but could be updated > >> maybe: gfc_typebound_proc’s u.generic field for example is nowhere freed > >> as far as I know. > > > > Well that's a valid point, not sure where they are freed indeed. > > Do you have a specific testcase in mind that leaks tbp's u.generic (or > > specific for that matter) for me to look at? > > > Any testcase with generic typebound procedures, I guess. > typebound_generic_3.f03 for example seems like a good candidate. I'll have a look at these later, thanks for the pointer. > > > I'm happy to change the comment to > > TODO: Free type-bound procedure u.generic and u.specific fields > > to reflect the current state. Ok? > > > I don’t think specific leaks because it’s one of gfc_namespace’s > sym_root sub-nodes, and it’s freed with gfc_namespace. > OK without "and u.specific". Ah right. Done. Thanks so far!