Re: [PATCH v3] Re: avoid useless if-before-free tests
Janne Blomqvist wrote: On Thu, Mar 24, 2011 at 18:51, Jim Meyering j...@meyering.net wrote: Janne Blomqvist wrote: On Tue, Mar 8, 2011 at 19:53, Jim Meyering j...@meyering.net wrote: Relative to v2, I've added libgo/ to the list of exempt directories and added this recently discussed gfc_free patch, at the request of Tobias Burnus. Also, I corrected an error in fortran's ChangeLog and removed all whitespace changes from all ChangeLog files. The libgfortran changes are Ok for 4.7. For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd - Replace all calls to gfc_free (x) with free (x). - Remove the gfc_free() function and prototype. - Remove the free() macro which currently prevents calling free() directly. Following up, I've refreshed the series but hit a minor snag while converting new uses of gfc_free, removing new tests-before-free and merging/reordering changes. Applying this fix first makes my problem go away: [snip] So, what's the plan here? Do you plan to get a GCC account, do you already have one, or what? Now that 4.7 is open for development, it's perhaps the right time to poke the maintainers to get this patch in. If you don't have a GCC account, as one of the Fortran maintainers I can commit the Fortran and libgfortran parts, but someone else will have to do the rest (were they ever approved, BTW?) as I have only write after approval privileges for the rest of GCC. Janne, I've rebased and divided/reordered these changes as you suggested. Here are the fortran parts. I'll post the other parts separately. Parts 1 and 3 are manual. Part 2 is the big mechanical change with two manual adjustments: [PATCH 1/3] gfortran: remove cpp definition of free, ... [PATCH 2/3] convert each use of gfc_free (p) to free (p) [PATCH 3/3] gfortran: remove now-unused definition of gfc_free From ca8e1ad7d098a1e91113fc7052607b37c1c2a636 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 15 Apr 2011 18:22:34 +0200 Subject: [PATCH 1/3] gfortran: remove cpp definition of free, ... in preparation for the s/gfc_free/free/ transformation. * gfortran.h (free): Remove macro definition that would otherwise prevent direct use of the function. --- gcc/fortran/ChangeLog |7 +++ gcc/fortran/gfortran.h |1 - 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog index 7154e62..094541c 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,10 @@ +2011-04-15 Jim Meyering meyer...@redhat.com + + gfortran: remove cpp definition of free, ... + in preparation for the s/gfc_free/free/ transformation. + * gfortran.h (free): Remove macro definition that would otherwise + prevent direct use of the function. + 2011-04-18 Tobias Burnus bur...@net-b.de PR fortran/18918 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index c2c9d05..49fbd1f 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -49,7 +49,6 @@ along with GCC; see the file COPYING3. If not see #define MAX_SUBRECORD_LENGTH 2147483639 /* 2**31-9 */ -#define free(x) Use_gfc_free_instead_of_free() #define gfc_is_whitespace(c) ((c==' ') || (c=='\t')) /* Stringization. */ -- 1.7.5.rc2.295.g19c42 From 672a558437e4cd52ca95b415c6036a6740a03f4c Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 15 Apr 2011 18:28:22 +0200 Subject: [PATCH 2/3] convert each use of gfc_free (p) to free (p) Do that by running this command: perl -pi -e's/\bgfc_free ?\(/free (/' \ $(git grep -El '\bgfc_free ?\(') which also corrects the few uses that lacked a space between the function name and the open parenthesis. Manually undo the change to the function definition itself and its prototype. They'll be removed next. --- gcc/fortran/ChangeLog | 11 + gcc/fortran/array.c |4 +- gcc/fortran/constructor.c |2 +- gcc/fortran/cpp.c |8 +++--- gcc/fortran/data.c|2 +- gcc/fortran/decl.c| 22 +- gcc/fortran/dependency.c |2 +- gcc/fortran/error.c |4 +- gcc/fortran/expr.c| 24 +- gcc/fortran/frontend-passes.c | 18 +++--- gcc/fortran/interface.c | 16 ++-- gcc/fortran/intrinsic.c | 10 gcc/fortran/io.c | 16 ++-- gcc/fortran/match.c | 14 +- gcc/fortran/misc.c|2 +- gcc/fortran/module.c | 50 gcc/fortran/openmp.c |2 +- gcc/fortran/options.c |2 +- gcc/fortran/parse.c |4 +- gcc/fortran/primary.c |4 +- gcc/fortran/resolve.c |8 +++--- gcc/fortran/scanner.c | 46 ++-- gcc/fortran/simplify.c| 16 ++-- gcc/fortran/st.c
Re: [PATCH v3] Re: avoid useless if-before-free tests
On Mon, Apr 18, 2011 at 18:08, Jim Meyering j...@meyering.net wrote: I've rebased and divided/reordered these changes as you suggested. Here are the fortran parts. I'll post the other parts separately. Parts 1 and 3 are manual. Part 2 is the big mechanical change with two manual adjustments: [PATCH 1/3] gfortran: remove cpp definition of free, ... [PATCH 2/3] convert each use of gfc_free (p) to free (p) [PATCH 3/3] gfortran: remove now-unused definition of gfc_free Thanks. The patches themselves are ok, however the changelog parts should conform to the layout used otherwise for changelog entries (GNU ChangeLog format, IIRC). The commit logs themselves that you have now used are, IMHO, more informative and contain less clutter, but alas.. Note that it's recommended and usually easiest to provide changelog entries separately, not as diffs from the ChangeLog file. Then when you commit you insert the entry. This avoids the problem of frequent conflicts as the top of the changelog file changes for every commit. (Why we, in the age of non-sucky version control, persist in keeping manual changelog files is beyond me..) With these changes, Ok for trunk. Thanks a lot for the contribution! -- Janne Blomqvist
Re: [PATCH v3] Re: avoid useless if-before-free tests
Janne Blomqvist wrote: On Mon, Apr 18, 2011 at 18:08, Jim Meyering j...@meyering.net wrote: I've rebased and divided/reordered these changes as you suggested. Here are the fortran parts. I'll post the other parts separately. Parts 1 and 3 are manual. Part 2 is the big mechanical change with two manual adjustments: [PATCH 1/3] gfortran: remove cpp definition of free, ... [PATCH 2/3] convert each use of gfc_free (p) to free (p) [PATCH 3/3] gfortran: remove now-unused definition of gfc_free Thanks. The patches themselves are ok, however the changelog parts should conform to the layout used otherwise for changelog entries (GNU ChangeLog format, IIRC). The commit logs themselves that you have now used are, IMHO, more informative and contain less clutter, but alas.. Note that it's recommended and usually easiest to provide changelog entries separately, not as diffs from the ChangeLog file. Ok. I interpret that as meaning I should add a line for each file/function affected by the large mechanical change in part 2. I will append that to the existing ChangeLog entry. Thanks to Ralf's vc-chlog, that will be automatic: http://git.sv.gnu.org/cgit/vc-dwim.git/tree/README Then when you commit you insert the entry. This avoids the problem of frequent conflicts as the top of the changelog file changes for every commit. If you work with the gcc repository using git, it's easy to rebase even pesky ChangeLog diffs once you hook up Bruno Haible's git-merge-changelog: http://git.sv.gnu.org/cgit/gnulib.git/tree/lib/git-merge-changelog.c If you'd like me to change the other two ChangeLog entries or any commit log message, please let me know. (Why we, in the age of non-sucky version control, persist in keeping manual changelog files is beyond me..) I couldn't agree more ;-) Even in projects without a VC'd ChangeLog file, I still keep one (and build up each ChangeLog entry as usual) and use vc-dwim to automatically cross-check any change I make. It regularly warns me about a changed file that's listed in ChangeLog but for which my editor has unsaved changes. With these changes, Ok for trunk. Thanks a lot for the contribution! Thanks for the review.
Re: [PATCH v3] Re: avoid useless if-before-free tests
Janne Blomqvist wrote: On Thu, Mar 24, 2011 at 18:51, Jim Meyering j...@meyering.net wrote: Janne Blomqvist wrote: On Tue, Mar 8, 2011 at 19:53, Jim Meyering j...@meyering.net wrote: Relative to v2, I've added libgo/ to the list of exempt directories and added this recently discussed gfc_free patch, at the request of Tobias Burnus. Also, I corrected an error in fortran's ChangeLog and removed all whitespace changes from all ChangeLog files. The libgfortran changes are Ok for 4.7. For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd - Replace all calls to gfc_free (x) with free (x). - Remove the gfc_free() function and prototype. - Remove the free() macro which currently prevents calling free() directly. Following up, I've refreshed the series but hit a minor snag while converting new uses of gfc_free, removing new tests-before-free and merging/reordering changes. Applying this fix first makes my problem go away: [snip] So, what's the plan here? Do you plan to get a GCC account, do you already have one, or what? Now that 4.7 is open for development, it's perhaps the right time to poke the maintainers to get this patch in. If you don't have a GCC account, as one of the Fortran maintainers I can commit the Fortran and libgfortran parts, but someone else will have to do the rest (were they ever approved, BTW?) as I have only write after approval privileges for the rest of GCC. Can someone add me to the gcc group? That would help. I already have ssh access to sourceware.org. I've rebased the series a few times, but it's been a week or so. More convertible uses are being added regularly. Plus I have to reorder/split things a little to avoid a new conflict.
Re: [PATCH v3] Re: avoid useless if-before-free tests
On Fri, Apr 15, 2011 at 10:54, Jim Meyering j...@meyering.net wrote: Janne Blomqvist wrote: On Thu, Mar 24, 2011 at 18:51, Jim Meyering j...@meyering.net wrote: Janne Blomqvist wrote: On Tue, Mar 8, 2011 at 19:53, Jim Meyering j...@meyering.net wrote: Relative to v2, I've added libgo/ to the list of exempt directories and added this recently discussed gfc_free patch, at the request of Tobias Burnus. Also, I corrected an error in fortran's ChangeLog and removed all whitespace changes from all ChangeLog files. The libgfortran changes are Ok for 4.7. For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd - Replace all calls to gfc_free (x) with free (x). - Remove the gfc_free() function and prototype. - Remove the free() macro which currently prevents calling free() directly. Following up, I've refreshed the series but hit a minor snag while converting new uses of gfc_free, removing new tests-before-free and merging/reordering changes. Applying this fix first makes my problem go away: [snip] So, what's the plan here? Do you plan to get a GCC account, do you already have one, or what? Now that 4.7 is open for development, it's perhaps the right time to poke the maintainers to get this patch in. If you don't have a GCC account, as one of the Fortran maintainers I can commit the Fortran and libgfortran parts, but someone else will have to do the rest (were they ever approved, BTW?) as I have only write after approval privileges for the rest of GCC. Can someone add me to the gcc group? That would help. I already have ssh access to sourceware.org. According to http://gcc.gnu.org/svnwrite.html , you should send an email to overseers(at)gcc.gnu.org . It says that you need a sponsor and The steering committee or a well-established GCC maintainer (including reviewers) can approve for write access any person with GNU copyright assignment papers in place and known to submit good patches.. I'm not sure if I'm considered to be well-established enough, so could someone help Jim out here, please? Or, if nobody pipes up within a few days, just mention my name as your sponsor and we'll see if the overseers consider me well-established or not.. ;) -- Janne Blomqvist
Re: [PATCH v3] Re: avoid useless if-before-free tests
Janne Blomqvist wrote: On Fri, Apr 15, 2011 at 10:54, Jim Meyering j...@meyering.net wrote: Janne Blomqvist wrote: On Thu, Mar 24, 2011 at 18:51, Jim Meyering j...@meyering.net wrote: Janne Blomqvist wrote: On Tue, Mar 8, 2011 at 19:53, Jim Meyering j...@meyering.net wrote: Relative to v2, I've added libgo/ to the list of exempt directories and added this recently discussed gfc_free patch, at the request of Tobias Burnus. Also, I corrected an error in fortran's ChangeLog and removed all whitespace changes from all ChangeLog files. The libgfortran changes are Ok for 4.7. For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd - Replace all calls to gfc_free (x) with free (x). - Remove the gfc_free() function and prototype. - Remove the free() macro which currently prevents calling free() directly. Following up, I've refreshed the series but hit a minor snag while converting new uses of gfc_free, removing new tests-before-free and merging/reordering changes. Applying this fix first makes my problem go away: [snip] So, what's the plan here? Do you plan to get a GCC account, do you already have one, or what? Now that 4.7 is open for development, it's perhaps the right time to poke the maintainers to get this patch in. If you don't have a GCC account, as one of the Fortran maintainers I can commit the Fortran and libgfortran parts, but someone else will have to do the rest (were they ever approved, BTW?) as I have only write after approval privileges for the rest of GCC. Can someone add me to the gcc group? That would help. I already have ssh access to sourceware.org. According to http://gcc.gnu.org/svnwrite.html , you should send an email to overseers(at)gcc.gnu.org . It says that you need a sponsor and The steering committee or a well-established GCC maintainer (including reviewers) can approve for write access any person with GNU copyright assignment papers in place and known to submit good patches.. I'm not sure if I'm considered to be well-established enough, so could someone help Jim out here, please? Or, if nobody pipes up within a few days, just mention my name as your sponsor and we'll see if the overseers consider me well-established or not.. ;) Thanks. FYI, I have an ANY assignment on file, so no need to wait for paperwork.
Re: [PATCH v3] Re: avoid useless if-before-free tests
Janne == Janne Blomqvist blomqvist.ja...@gmail.com writes: Jim Can someone add me to the gcc group? That would help. Jim I already have ssh access to sourceware.org. Janne I'm not sure if I'm considered to be well-established Janne enough, so could someone help Jim out here, please? I added Jim to the gcc group. Tom