| Message ID | 51058B05.8030904@sfr.fr |
|---|---|
| State | New |
| Headers | show |
Am 27.01.2013 21:16, schrieb Mikael Morin:
> Or maybe wait for the fix for comment #4?
I would rather commit the fixes now, just on general principles.
If any regression should occur, it would be easier to pinpoint
the reason.
Thomas
Hi, >> subroutine sub (arg) >> procedure(sub) :: arg >> end subroutine >> > You forgot to precise that this case (which is basically comment #4 in the > PR) is *not* fixed by the patch, as it fails later on at translation stage. yes, I just silently ignored that fact. > I have made up my mind that it's not possible for the middle-end to build > such a recursive type. So `arg' will have to have a variadic function type. > No patch yet, sorry; I have just figured it out. Don't worry. I agree with Thomas that this should preferably be done in a follow-up patch. (The hard part is probably to detect all the cases of 'recursive' definitions, also the indirect ones.) >> Anyway, should we bump the mod version with this patch, or should we >> rather avoid it? >> > I forgot the reason why we are so reluctant to do it. Module versions are > not a rare resource. I'm in favor of bumping (and any time we change module > format). I'm also ok with a bump, but of course it would be preferable to have a stable ABI and module format at some point. As a matter of fact, the module format has changed in all recent releases, though (that's why we introduced the module versioning, after all). > About the patch, one nit: > > Index: gcc/fortran/gfortran.h > =================================================================== > --- gcc/fortran/gfortran.h (revision 195493) > +++ gcc/fortran/gfortran.h (working copy) > @@ -974,8 +974,6 @@ typedef struct gfc_component > struct gfc_component *next; > > /* Needed for procedure pointer components. */ > - struct gfc_formal_arglist *formal; > - struct gfc_namespace *formal_ns; > struct gfc_typebound_proc *tb; > } > gfc_component; > > The comment should probably be removed as well. Well, it still applies to 'tb' ... >> The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk? >> > OK from my side; you may or may not need someone else's ack as I'm the > coauthor. Certainly a second review/opinion could not hurt. I'm attaching an updated patch, which includes the module-version bump now. > Or maybe wait for the fix for comment #4? Rather not (technically it's a separate issue, I guess). Cheers, Janus
Hi Janus, >> Or maybe wait for the fix for comment #4? > Rather not (technically it's a separate issue, I guess). While the patch is rather large, I think it is OK. One request: Could you add a comment to gfc_sym_get_dummy_args explaining what the function does and under which conditions sym->formal is NULL, while sym->ts.interface->formal isn't? Regards Thomas
Hi Thomas, >>> Or maybe wait for the fix for comment #4? >> >> Rather not (technically it's a separate issue, I guess). > > > While the patch is rather large, I think it is OK. > > One request: Could you add a comment to gfc_sym_get_dummy_args > explaining what the function does and under which conditions sym->formal > is NULL, while sym->ts.interface->formal isn't? thanks for the review. I have added the comment to gfc_sym_get_dummy_args that you requested, and committed the patch as r195562. Cheers, Janus
Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 195493) +++ gcc/fortran/gfortran.h (working copy) @@ -974,8 +974,6 @@ typedef struct gfc_component struct gfc_component *next; /* Needed for procedure pointer components. */ - struct gfc_formal_arglist *formal; - struct gfc_namespace *formal_ns; struct gfc_typebound_proc *tb; } gfc_component;