| Message ID | CAKwh3qgdG=c0kzoZvW1Rq+uiZm801oPSMgdkNFUc8bE5YNDT_w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 11, 2016 at 03:05:06PM +0100, Janus Weil wrote: > 2016-11-11 14:38 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: > > [Btw, speaking of gfc_get_tbp_symtree: Can anyone tell me by chance > > why it is necessary to nullify 'result->n.tb' on a newly-created > > symtree?] > > Removing the corresponding line does not do any harm to the testsuite, > as I just verified: > > Index: gcc/fortran/class.c > =================================================================== > --- gcc/fortran/class.c (Revision 242066) > +++ gcc/fortran/class.c (Arbeitskopie) > @@ -2970,7 +2970,6 @@ gfc_get_tbp_symtree (gfc_symtree **root, const cha > { > result = gfc_new_symtree (root, name); > gcc_assert (result); > - result->n.tb = NULL; > } > > return result; > I think the assert can be removed as well. gfc_new_symtree is defined by XCNEW, which is defined in terms of xcalloc, which is defined in libiberty/xmalloc.c in terms of calloc. calloc zeros allocated memory. xcalloc also checks for a valid allocation, so gcc_assert is redundant.
Le 11/11/2016 à 19:30, Steve Kargl a écrit : > On Fri, Nov 11, 2016 at 03:05:06PM +0100, Janus Weil wrote: >> 2016-11-11 14:38 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >>> [Btw, speaking of gfc_get_tbp_symtree: Can anyone tell me by chance >>> why it is necessary to nullify 'result->n.tb' on a newly-created >>> symtree?] >> >> Removing the corresponding line does not do any harm to the testsuite, >> as I just verified: >> >> Index: gcc/fortran/class.c >> =================================================================== >> --- gcc/fortran/class.c (Revision 242066) >> +++ gcc/fortran/class.c (Arbeitskopie) >> @@ -2970,7 +2970,6 @@ gfc_get_tbp_symtree (gfc_symtree **root, const cha >> { >> result = gfc_new_symtree (root, name); >> gcc_assert (result); >> - result->n.tb = NULL; >> } >> >> return result; >> > > I think the assert can be removed as well. gfc_new_symtree > is defined by XCNEW, which is defined in terms of xcalloc, > which is defined in libiberty/xmalloc.c in terms of calloc. > calloc zeros allocated memory. xcalloc also checks for a > valid allocation, so gcc_assert is redundant. > > And you can remove «tbp» from the function name, as there is nothing related to typebound procedures any more.
2016-11-12 20:15 GMT+01:00 Mikael Morin <morin-mikael@orange.fr>: >>>> [Btw, speaking of gfc_get_tbp_symtree: Can anyone tell me by chance >>>> why it is necessary to nullify 'result->n.tb' on a newly-created >>>> symtree?] >>> >>> >>> Removing the corresponding line does not do any harm to the testsuite, >>> as I just verified: >>> >>> Index: gcc/fortran/class.c >>> =================================================================== >>> --- gcc/fortran/class.c (Revision 242066) >>> +++ gcc/fortran/class.c (Arbeitskopie) >>> @@ -2970,7 +2970,6 @@ gfc_get_tbp_symtree (gfc_symtree **root, const cha >>> { >>> result = gfc_new_symtree (root, name); >>> gcc_assert (result); >>> - result->n.tb = NULL; >>> } >>> >>> return result; >>> >> >> I think the assert can be removed as well. gfc_new_symtree >> is defined by XCNEW, which is defined in terms of xcalloc, >> which is defined in libiberty/xmalloc.c in terms of calloc. >> calloc zeros allocated memory. xcalloc also checks for a >> valid allocation, so gcc_assert is redundant. >> >> > And you can remove «tbp» from the function name, as there is nothing related > to typebound procedures any more. True. Probably one should also move it to symbol.c. However, one can wonder whether renaming to gfc_get_symtree would not make the name too similar to gfc_get_sym_tree, which also lives in symbol.c ...? Cheers, Janus
2016-11-12 21:21 GMT+01:00 Janus Weil <janus@gcc.gnu.org>: >>>> Index: gcc/fortran/class.c >>>> =================================================================== >>>> --- gcc/fortran/class.c (Revision 242066) >>>> +++ gcc/fortran/class.c (Arbeitskopie) >>>> @@ -2970,7 +2970,6 @@ gfc_get_tbp_symtree (gfc_symtree **root, const cha >>>> { >>>> result = gfc_new_symtree (root, name); >>>> gcc_assert (result); >>>> - result->n.tb = NULL; >>>> } >>>> >>>> return result; >>>> >>> >>> I think the assert can be removed as well. gfc_new_symtree >>> is defined by XCNEW, which is defined in terms of xcalloc, >>> which is defined in libiberty/xmalloc.c in terms of calloc. >>> calloc zeros allocated memory. xcalloc also checks for a >>> valid allocation, so gcc_assert is redundant. >>> >>> >> And you can remove «tbp» from the function name, as there is nothing related >> to typebound procedures any more. > > True. Probably one should also move it to symbol.c. > > However, one can wonder whether renaming to gfc_get_symtree would not > make the name too similar to gfc_get_sym_tree, which also lives in > symbol.c ...? I have just opened PR 78377 to track this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78377 Cheers, Janus
Index: gcc/fortran/class.c =================================================================== --- gcc/fortran/class.c (Revision 242066) +++ gcc/fortran/class.c (Arbeitskopie) @@ -2970,7 +2970,6 @@ gfc_get_tbp_symtree (gfc_symtree **root, const cha { result = gfc_new_symtree (root, name); gcc_assert (result); - result->n.tb = NULL; } return result;