Re: [committed] Fortran: Add more documentation for mixed-language programming
Hi Sandra, Looks good to me. Thanks Paul On Fri, 5 Nov 2021 at 21:13, Sandra Loosemore wrote: > I was recently pinged about PR35276. It's an old issue, but a couple of > the concerns raised there haven't been fixed yet, so I've checked in > this patch to fill in the gaps. > > -Sandra > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/102715 - [12 Regression] ICE in gfc_simplify_transpose, at fortran/simplify.c:8184
Le 31/10/2021 à 22:35, Harald Anlauf via Fortran a écrit : Dear Fortranners, the fix for initialization of DT arrays caused an apparent regression for cases where inconsistent ranks were used in such an initialization. This caused either an ICE in subsequent uses of these arrays, or showed up in valgrind as invalid reads, all of which seemed to be related to this rank mismatch. The cleanest solution seems to be to strictly reject rank mismatch earlier than we used to, which helps error recovery. I had to adjust one testcase accordingly. The place I inserted the check does not distinguish between explicit shape and implied shape. The Intel compiler does give a slightly different error message for the implied shape case. If anyone feels strongly about this, I'm open to suggestions for better choices of handling this. Regtested on x86_64-pc-linux-gnu. OK for mainline / affected branches? OK, thanks. Mikael
Re: [PATCH] Fortran: Diagnose all operands/arguments with constraint violations
Le 05/11/2021 à 03:58, Sandra Loosemore a écrit : This is an expanded version of the patch for PR 101337 that Bernhard sent out a few days ago with a request for me to finish it. Bernhard did the part for operands and I added the pieces for procedure arguments and intrinsics, along with fixing up the test cases that were previously full of xfails and a few others that were now showing multiple diagnostics as a result of this change. I suspect there might be other places where we are failing to check all subexpressions for errors, but this catches all the ones I wrote TS29113-related testcases for, at least. OK to commit? Ok. Thanks to both of you. Mikael
Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers
Le 05/11/2021 à 19:46, Mikael Morin a écrit : Don’t you get the same effect on the memory leaks if you keep just the following hunk? >>> @@ -1605,8 +1608,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, >>> /* Set up the namespace. */ >>> sub_ns = gfc_get_namespace (ns, 0); >>> sub_ns->sibling = ns->contained; >>> - if (!expr_null_wrapper) >>> - ns->contained = sub_ns; >>> + ns->contained = sub_ns; >>> sub_ns->resolved = 1; >>> >>> /* Set up the procedure symbol. */ That’s probably not a good idea on second thought; it’s preferable to leak memory and not generate an empty finalization procedure.
Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers
Sorry, I hadn’t seen your message. 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 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). Don’t you get the same effect on the memory leaks if you keep just the following hunk? No, i don't think emitting the finalization-wrappers unconditionally is correct. > (... lengthy explaination ...) > Agreed, it was a poor suggestion. 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'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".
Re: gfortran.org
greetings to all, my ownership of gfortran.org has lapsed and it is now owned by a "speculator". it appears to be available for $99, if someone is interested. gfortran.com is paid until June 25th, 2024. this one is set to auto-renew, whereas gfortran.org was not. it redirects to http://gfortran.meteodat.ch , as it has for several years. if someone wants the domain, I will transfer it. Regards, Bud Davis On Fri, Nov 5, 2021 at 4:15 AM Manfred Schwarb wrote: > Hi, > > for quite some time, there was a forwarding from http://gfortran.org to my > dumping ground http://gfortran.meteodat.ch. > > At the moment, at least for me, this domain points to some gaming blog. > Does anybody know what is going on? > There were some ideas what do with this domain at some point (Jerry, > Paul?). > > Bud, are you still owner of this domain? Do you have some plans what to do > with it? > > Cheers, > Manfred > > > PS: the domain gfortran.com still points to my site, but the above > questions > apply to this domain as well, of course. >
Re: [PATCH] PR fortran/91497 -- Silence conversion warnings for MIN1 and MAX1
Am 02.11.21 um 19:39 schrieb Thomas Koenig: > On 02.11.21 15:22, Manfred Schwarb wrote: >> Am 02.11.21 um 14:26 schrieb Thomas Koenig: >>> Hi Manfred, >>> In addition to the patches of Steve Kargl for PR 91497: The MIN1 and MAX1 intrinsics do explicit type conversions and should be silenced too for -Wconversion and -Wconversion-extra. Adjust testcase to only use *4 and *8 real types, provide a second testcase for *10 and *16 precisions. >>> Two points: >>> >>> We should modify existing test cases only when necessary, because >>> modification can impede a regression test. It is better to create >>> a new one. >>> I only changed the test case to use dg-require-effective-target and real(n) notation now. Added a second case without real(10) and real(16), but for all targets, which covers MIN1 and MAX1 too. >> >> Yes, but this was a quick-and-dirty test of mine, and I realized only >> afterwards >> that Steve had used it as-is. The new testcase is more consistent and more >> complete. >> Sandra got errors on targets without REAL(16) support and requested changes, >> so I decided to split it. >> >> So you want me to "split" it in 3 parts? >> - existing test as is, only for targets with REAL(16) support >> - additional tests incl. complex intrinsics for targets with REAL(16) support >> - additional tests incl. complex intrinsics for all targets, only single and >> double precision >> >> OTOH, it is perhaps not worth the trouble to do REAL(10) and REAL(16) tests, >> either >> it warns or it does not. > > Anything that tests both complex and REAL(16) is fine by me. I don't > think you need to test the combination of COMPLEX(16), both > codepaths have been seen by that time :-) > > Or you can split it three ways, like you wrote above. > >>> While we do recognize real*4 and real*8 and so on, these are >>> non-standard extensions, and I would like to avoid to have these >>> with new test cases. >>> >>> Instead of real*8, you can use real(8) or double precision. >>> >> >> Well, double precision is deprecated AFAIK. > > Not in Fortran 2018. > > Best regards > > Thomas > 2021-11-06 Manfred Schwarb gcc/fortran/ChangeLog: PR fortran/91497 * simplify.c (simplify_min_max): Disable conversion warnings for MIN1 and MAX1. --- a/gcc/fortran/simplify.c +++ b/gcc/fortran/simplify.c @@ -5087,6 +5087,7 @@ min_max_choose (gfc_expr *arg, gfc_expr static gfc_expr * simplify_min_max (gfc_expr *expr, int sign) { + int tmp1, tmp2; gfc_actual_arglist *arg, *last, *extremum; gfc_expr *tmp, *ret; const char *fname; @@ -5131,7 +5132,16 @@ simplify_min_max (gfc_expr *expr, int si if ((tmp->ts.type != BT_INTEGER || tmp->ts.kind != gfc_integer_4_kind) && (strcmp (fname, "min1") == 0 || strcmp (fname, "max1") == 0)) { + /* Explicit conversion, turn off -Wconversion and -Wconversion-extra + warnings. */ + tmp1 = warn_conversion; + tmp2 = warn_conversion_extra; + warn_conversion = warn_conversion_extra = 0; + ret = gfc_convert_constant (tmp, BT_INTEGER, gfc_integer_4_kind); + + warn_conversion = tmp1; + warn_conversion_extra = tmp2; } else if ((tmp->ts.type != BT_REAL || tmp->ts.kind != gfc_real_4_kind) && (strcmp (fname, "amin0") == 0 || strcmp (fname, "amax0") == 0)) 2021-11-06 Manfred Schwarb gcc/testsuite/ChangeLog: PR fortran/91497 * gfortran.dg/pr91497.f90: Adjust test to use dg-require-effective-target directive. * gfortran.dg/pr91497_2.f90: New test to cover all targets. Cover MAX1 and MIN1 intrinsics. --- a/gcc/testsuite/gfortran.dg/pr91497.f90 +++ b/gcc/testsuite/gfortran.dg/pr91497.f90 @@ -1,4 +1,6 @@ -! { dg-do compile { target { i?86-*-* x86_64-*-* } } } +! { dg-do compile } +! { dg-require-effective-target fortran_real_10 } +! { dg-require-effective-target fortran_real_16 } ! { dg-options "-Wall" } ! Code contributed by Manfred Schwarb ! PR fortran/91497 @@ -8,13 +10,13 @@ ! program foo - real*4 a,aa - real*8 b,bb - real*10 c,cc - real*16 d - integer*2 e,ee - integer*4 f,ff - integer*8 g,gg + real(4) a,aa + real(8) b,bb + real(10) c,cc + real(16) d + integer(2) e,ee + integer(4) f,ff + integer(8) g,gg PARAMETER(a=3.1415927_4) PARAMETER(b=3.1415927_8) PARAMETER(c=3.1415927_10) @@ -36,11 +38,10 @@ program foo aa=CEILING(b) aa=CEILING(c) aa=CEILING(d) - !---unknown but documented type conversions: + !---DEC specific type conversions (-fdec): !!aa=FLOATI(e) !!aa=FLOATJ(f) !!aa=FLOATK(g) - !---documentation is wrong for sngl: aa=SNGL(c) aa=SNGL(d) bb=REAL(c, kind=8) @@ -98,7 +99,7 @@ program foo ff=IFIX(a) ff=IDINT(b) ff=IDNINT(b) - !---LONG not allowed anymore in gfortran 10 (?): + !---LONG support got removed: !!ff=LONG(a) !!ff=LONG(b) !!ff=LONG(c) --- /dev/null
Re: [PATCH] PR fortran/91497 -- Silence conversion warnings for MIN1 and MAX1
Hi Manfred, looks good to me. Thanks for the patch! Best regards Thomas
Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]
Hi Bernhard, I cannot comment on the gcc/ parts, but Am 05.11.21 um 22:17 schrieb Bernhard Reutner-Fischer via Gcc-patches: From: Bernhard Reutner-Fischer gcc/fortran/ChangeLog: PR fortran/68800 * cpp.h (gfc_cpp_free_cpp_dirs): New declaration. * cpp.c (gfc_cpp_free_cpp_dirs): New definition. (gfc_cpp_add_include_path, gfc_cpp_add_include_path_after): Add comment. * f95-lang.c (gfc_finish): Call gfc_cpp_free_cpp_dirs. * scanner.c (add_path_to_list): Allocate unzeroed memory. (gfc_add_include_path): Add comment and fix formatting. --- Bootstrapped and regtested without regressions on x86_64-unknown-linux. Ok for trunk? Note: in add_path_to_list i changed dir->path = XCNEWVEC to XNEWVEC since strcpy and strcat add a terminating null byte for us anyway. Fixes: -== 68 bytes in 1 blocks are still reachable in loss record 228 of 371 -==at : malloc (vg_replace_malloc.c:380) -==by : xmalloc (xmalloc.c:149) -==by : xstrdup (xstrdup.c:34) -==by : gfc_add_include_path(char const*, bool, bool, bool, bool) (scanner.c :428) -==by : gfc_handle_option(unsigned long, char const*, long, int, unsigned in t, cl_option_handlers const*) (options.c:702) -==by : handle_option(gcc_options*, gcc_options*, cl_decoded_option const*, unsigned int, int, unsigned int, cl_option_handlers const*, bool, diagnostic_con text*) (opts-common.c:1181) -==by : read_cmdline_option(gcc_options*, gcc_options*, cl_decoded_option*, unsigned int, unsigned int, cl_option_handlers const*, diagnostic_context*) (opt s-common.c:1431) -==by : read_cmdline_options (opts-global.c:237) -==by : decode_options(gcc_options*, gcc_options*, cl_decoded_option*, unsig ned int, unsigned int, diagnostic_context*, void (*)()) (opts-global.c:319) -==by : toplev::main(int, char**) (toplev.c:2273) -==by : main (main.c:39) --- gcc/fortran/cpp.c | 13 +++-- gcc/fortran/cpp.h | 1 + gcc/fortran/f95-lang.c | 2 +- gcc/fortran/scanner.c | 7 --- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c index e86386c8b17..04fe8fe460b 100644 --- a/gcc/fortran/cpp.c +++ b/gcc/fortran/cpp.c @@ -728,12 +728,20 @@ gfc_cpp_done (void) cpp_clear_file_cache (cpp_in); } why do you introduce a wrapper for something outside of fortran that is used only once, -/* PATH must be malloc-ed and NULL-terminated. */ +/* Free all cpp include dirs. */ +void +gfc_cpp_free_cpp_dirs (void) +{ + free_cpp_dirs (); +} + +/* PATH must be NULL-terminated. */ void gfc_cpp_add_include_path (char *path, bool user_supplied) { /* CHAIN sets cpp_dir->sysp which differs from 0 if PATH is a system - include path. Fortran does not define any system include paths. */ + include path. Fortran does not define any system include paths. + incpath.c manages the incoming, xstrdup()ed path. */ int cxx_aware = 0; add_path (path, INC_BRACKET, cxx_aware, user_supplied); @@ -742,6 +750,7 @@ gfc_cpp_add_include_path (char *path, bool user_supplied) void gfc_cpp_add_include_path_after (char *path, bool user_supplied) { + /* incpath.c manages the incoming, xstrdup()ed path. */ int cxx_aware = 0; add_path (path, INC_AFTER, cxx_aware, user_supplied); } diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h index 44644a2a333..963b9a9c89e 100644 --- a/gcc/fortran/cpp.h +++ b/gcc/fortran/cpp.h @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool); bool gfc_cpp_preprocess (const char *source_file); void gfc_cpp_done (void); +void gfc_cpp_free_cpp_dirs (void); void gfc_cpp_add_include_path (char *path, bool user_supplied); void gfc_cpp_add_include_path_after (char *path, bool user_supplied); diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c index 58dcaf01d75..ec4c2cf01d9 100644 --- a/gcc/fortran/f95-lang.c +++ b/gcc/fortran/f95-lang.c @@ -275,7 +275,7 @@ gfc_finish (void) gfc_cpp_done (); gfc_done_1 (); gfc_release_include_path (); - return; namely here? + gfc_cpp_free_cpp_dirs (); } Why not call free_cpp_dirs () here directly, omit all unnecessary stuff, and maybe only add a brief comment here? /* These functions and variables deal with binding contours. We only diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c index 69b81ab97f8..268d1e3e7fb 100644 --- a/gcc/fortran/scanner.c +++ b/gcc/fortran/scanner.c @@ -370,7 +370,7 @@ add_path_to_list (gfc_directorylist **list, const char *path, char *q; size_t len; int i; - + p = path; while (*p == ' ' || *p == '\t') /* someone might do "-I include" */ if (*p++ == '\0') @@ -409,7 +409,7 @@ add_path_to_list (gfc_directorylist **list, const char *path, *list = dir; dir->use_for_modules = use_for_modules; dir->warn = warn; > - dir->path = XCNEWVEC (char, strlen (p) + 2); + dir->path = XNEWVEC (char, strlen (p) + 2); strcpy (dir->path, p); strcat (
Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers
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
Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]
On Sat, 6 Nov 2021 20:22:53 +0100 Harald Anlauf wrote: > Hi Bernhard, > > I cannot comment on the gcc/ parts, but > > > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c > > index e86386c8b17..04fe8fe460b 100644 > > --- a/gcc/fortran/cpp.c > > +++ b/gcc/fortran/cpp.c > > @@ -728,12 +728,20 @@ gfc_cpp_done (void) > > cpp_clear_file_cache (cpp_in); > > } > > why do you introduce a wrapper for something outside of fortran > that is used only once, > > > -/* PATH must be malloc-ed and NULL-terminated. */ > > +/* Free all cpp include dirs. */ > > +void > > +gfc_cpp_free_cpp_dirs (void) > > +{ > > + free_cpp_dirs (); > > +} > > diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h > > index 44644a2a333..963b9a9c89e 100644 > > --- a/gcc/fortran/cpp.h > > +++ b/gcc/fortran/cpp.h > > @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool); > > bool gfc_cpp_preprocess (const char *source_file); > > > > void gfc_cpp_done (void); > > +void gfc_cpp_free_cpp_dirs (void); > > > > void gfc_cpp_add_include_path (char *path, bool user_supplied); > > void gfc_cpp_add_include_path_after (char *path, bool user_supplied); > > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c > > index 58dcaf01d75..ec4c2cf01d9 100644 > > --- a/gcc/fortran/f95-lang.c > > +++ b/gcc/fortran/f95-lang.c > > @@ -275,7 +275,7 @@ gfc_finish (void) > > gfc_cpp_done (); > > gfc_done_1 (); > > gfc_release_include_path (); > > - return; > > namely here? > > > + gfc_cpp_free_cpp_dirs (); > > } > > Why not call free_cpp_dirs () here directly, omit all unnecessary > stuff, and maybe only add a brief comment here? cpp.c includes incpath.h, f95-lang.c does not and should not. So the cleanest thing is to keep the cpp handling in cpp.[ch] and have the language frontend call into it's cpp bits. It would be rather rogue to extern void free_cpp_dirs (void); in f95-lang.c and directly call it in gfc_finish, i'd say? thanks,