[patch, fortran] fix for PR 60780, PR 40958

[patch, fortran] fix for PR 60780, PR 40958

[Russell Whitesides]

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

2015-05-14

     PR fortran 60780
     * module.c (load_equiv) : add check for duplicate modules


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)
  	  {
  	    head = eq->eq;

Re: [patch, fortran] fix for PR 60780, PR 40958

[Steve Kargl]

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?

-- 
Steve

Re: [patch, fortran] fix for PR 60780, PR 40958

[Russell Whitesides]

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.

Re: [patch, fortran] fix for PR 60780, PR 40958

[Steve Kargl]

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. :-)

-- 
Steve

Re: [patch, fortran] fix for PR 60780, PR 40958

[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

> 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

Re: [patch, fortran] fix for PR 60780, PR 40958

[russelldub .]

>> 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