Patchwork [Fortran] Fix PR 55593

login
register
mail settings
Submitter Thomas Koenig
Date Dec. 8, 2012, 9:52 a.m.
Message ID <50C30DC3.2060208@netcologne.de>
Download mbox | patch
Permalink /patch/204643/
State New
Headers show

Comments

Thomas Koenig - Dec. 8, 2012, 9:52 a.m.
Hello world,

the attached patch fixes a regression introduced with the recent
checking for DO loop variables when they are used with a generic
subroutine where the generic name matches one of the actual names.

Regression-tested.  OK for trunk?

	Thomas

2012-12-08  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/55593
         * frontend-passes.c (doloop_code):  Use resolved_sym
         instead of n.sym->formal for formal argument list
         to get the correct version for all generic subroutines.

2012-12-08  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/55593
         * gfortran.dg/do_check_8.f90:  New test.
Janus Weil - Dec. 8, 2012, 4:39 p.m.
Hi Thomas,

> the attached patch fixes a regression introduced with the recent
> checking for DO loop variables when they are used with a generic
> subroutine where the generic name matches one of the actual names.
>
> Regression-tested.  OK for trunk?

A few questions about that patch:

-      f = co->symtree->n.sym->formal;

+      if (co->resolved_sym == NULL)
+    break;
+
+      f = co->resolved_sym->formal;


In which cases does it happen that "resolved_sym==NULL" ? Would it
make sense to do the following instead?

if (co->resolved_sym)
  f = co->resolved_sym->formal;
else
  f = co->symtree->n.sym->formal;

Or could we even use an "gfc_assert (co->resolved_sym)"?

Cheers,
Janus
Thomas Koenig - Dec. 8, 2012, 7:50 p.m.
Am 08.12.2012 17:39, schrieb Janus Weil:
> Hi Thomas,
>
>> the attached patch fixes a regression introduced with the recent
>> checking for DO loop variables when they are used with a generic
>> subroutine where the generic name matches one of the actual names.
>>
>> Regression-tested.  OK for trunk?
>
> A few questions about that patch:
>
> -      f = co->symtree->n.sym->formal;
>
> +      if (co->resolved_sym == NULL)
> +    break;
> +
> +      f = co->resolved_sym->formal;
>
>
> In which cases does it happen that "resolved_sym==NULL" ?

This happens for (now part of the test case, because I got an ICE out of
it during testing)

   do undeclared=1,10        ! { dg-error "has no IMPLICIT type" }
      call sub(undeclared)
   end do

Because undeclared is not declared, the resolution doesn't happen,
and resolved_sym is NULL.  This is OK because an error has been
raised about this anyway; one more error would not do much good.


Would it
> make sense to do the following instead?
>
> if (co->resolved_sym)
>    f = co->resolved_sym->formal;
> else
>    f = co->symtree->n.sym->formal;

For some reason, co->symtree->n.sym->formal contains bogus
information under the conditions of the test case.  The
intent for the dummy argument in co->symtree->n.sym->formal
is set to INOUT.  So, I'd rather not fall back to this.

	Thomas
Janus Weil - Dec. 8, 2012, 10:30 p.m.
2012/12/8 Thomas Koenig <tkoenig@netcologne.de>:
> Am 08.12.2012 17:39, schrieb Janus Weil:
>
>> Hi Thomas,
>>
>>> the attached patch fixes a regression introduced with the recent
>>> checking for DO loop variables when they are used with a generic
>>> subroutine where the generic name matches one of the actual names.
>>>
>>> Regression-tested.  OK for trunk?
>>
>>
>> A few questions about that patch:
>>
>> -      f = co->symtree->n.sym->formal;
>>
>> +      if (co->resolved_sym == NULL)
>> +    break;
>> +
>> +      f = co->resolved_sym->formal;
>>
>>
>> In which cases does it happen that "resolved_sym==NULL" ?
>
>
> This happens for (now part of the test case, because I got an ICE out of
> it during testing)
>
>   do undeclared=1,10        ! { dg-error "has no IMPLICIT type" }
>      call sub(undeclared)
>   end do
>
> Because undeclared is not declared, the resolution doesn't happen,
> and resolved_sym is NULL.  This is OK because an error has been
> raised about this anyway;

good, if it only happens when an error has been raised, I think your
patch is ok.

Thanks,
Janus
Thomas Koenig - Dec. 9, 2012, 9:16 a.m.
Hi Janus,

> 2012/12/8 Thomas Koenig <tkoenig@netcologne.de>:

>> Because undeclared is not declared, the resolution doesn't happen,
>> and resolved_sym is NULL.  This is OK because an error has been
>> raised about this anyway;
>
> good, if it only happens when an error has been raised, I think your
> patch is ok.

Committed as revision 194329.

Thanks for the review!

	Thomas

Patch

Index: frontend-passes.c
===================================================================
--- frontend-passes.c	(Revision 193793)
+++ frontend-passes.c	(Arbeitskopie)
@@ -1277,8 +1277,12 @@  doloop_code (gfc_code **c, int *walk_subtrees ATTR
       break;
 
     case EXEC_CALL:
-      f = co->symtree->n.sym->formal;
 
+      if (co->resolved_sym == NULL)
+	break;
+
+      f = co->resolved_sym->formal;
+
       /* Withot a formal arglist, there is only unknown INTENT,
 	 which we don't check for.  */
       if (f == NULL)