Message ID | 50C30DC3.2060208@netcologne.de |
---|---|
State | New |
Headers | show |
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
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
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
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
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)