Message ID | 55556DA7.5060005@gmail.com |
---|---|
State | New |
Headers | show |
On Thu, May 14, 2015 at 08:53:11PM -0700, Russell Whitesides wrote: > Patch below prevents printing of duplicate statements in module files. > Without it module files grow exponentially in size with nested use of > modules that contain equivalence statements. Tested on x86-64-linux. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60780 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40958 > Sorry, but I don't recognize your name. Do you have a copyright assignment on file with FSF?
On 05/14/2015 09:36 PM, Steve Kargl wrote: > On Thu, May 14, 2015 at 08:53:11PM -0700, Russell Whitesides wrote: >> Patch below prevents printing of duplicate statements in module files. >> Without it module files grow exponentially in size with nested use of >> modules that contain equivalence statements. Tested on x86-64-linux. >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60780 >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40958 >> > > Sorry, but I don't recognize your name. Do you > have a copyright assignment on file with FSF? > I do not.
On Thu, May 14, 2015 at 09:39:03PM -0700, Russell Whitesides wrote: > > On 05/14/2015 09:36 PM, Steve Kargl wrote: > > On Thu, May 14, 2015 at 08:53:11PM -0700, Russell Whitesides wrote: > >> Patch below prevents printing of duplicate statements in module files. > >> Without it module files grow exponentially in size with nested use of > >> modules that contain equivalence statements. Tested on x86-64-linux. > >> > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60780 > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40958 > >> > > > > Sorry, but I don't recognize your name. Do you > > have a copyright assignment on file with FSF? > > > > I do not. The change may be small enough that an assignment isn't needed. We (ie, the gfortran developers) will need to check. Having an assignment will help when you submit additional patches. :-)
> The change may be small enough that an assignment isn't needed. > We (ie, the gfortran developers) will need to check. I think that’s small enough, compared to what we’ve accepted as such in the past. If not, a disclaimer by Russell putting his change in the public domain would also be a quick way: https://gcc.gnu.org/contribute.html#legal > Having an assignment will help when you submit additional patches. :-) Indeed! Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is necessary, but it appears module strings at this point are not GCC identifiers, but normal strings.) Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that you have bootstrapped it and regression-tested the full gfortran testsuite ? Cheers, FX
>> The change may be small enough that an assignment isn't needed. >> We (ie, the gfortran developers) will need to check. > > I think that’s small enough, compared to what we’ve accepted as such in the past. > If not, a disclaimer by Russell putting his change in the public domain would also be a quick way: https://gcc.gnu.org/contribute.html#legal Let me know either way. > Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is necessary, but it appears module strings at this point are not GCC identifiers, but normal strings.) That was my understanding. At any rate, I based the logic on the existing check for unused equivalences. > Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that you have bootstrapped it and regression-tested the full gfortran testsuite ? Yes, as long as `make check-fortran` is the full gfortran testsuite. On Fri, May 15, 2015 at 4:08 AM, FX <fxcoudert@gmail.com> wrote: >> The change may be small enough that an assignment isn't needed. >> We (ie, the gfortran developers) will need to check. > > I think that’s small enough, compared to what we’ve accepted as such in the past. > If not, a disclaimer by Russell putting his change in the public domain would also be a quick way: https://gcc.gnu.org/contribute.html#legal > >> Having an assignment will help when you submit additional patches. :-) > > Indeed! > > > Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is necessary, but it appears module strings at this point are not GCC identifiers, but normal strings.) > > Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that you have bootstrapped it and regression-tested the full gfortran testsuite ? > > Cheers, > FX
Index: module.c =================================================================== --- module.c (revision 223202) +++ module.c (working copy) @@ -4479,8 +4479,8 @@ static void load_equiv (void) { - gfc_equiv *head, *tail, *end, *eq; - bool unused; + gfc_equiv *head, *tail, *end, *eq, *equiv; + bool unused, duplicate; mio_lparen (); in_load_equiv = true; @@ -4523,8 +4523,20 @@ } } - if (unused) + /* Check for duplicate equivalences being loaded from different modules */ + duplicate = false; + for (equiv = gfc_current_ns->equiv; equiv; equiv = equiv->next) { + if (equiv->module && head->module + && strcmp (equiv->module, head->module) == 0) + { + duplicate = true; + break; + } + } + + if (unused || duplicate) + { for (eq = head; eq; eq = head) {