Message ID | 1456217637.7770.108.camel@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Feb 23, 2016 at 09:53:57AM +0100, Mark Wielaard wrote: > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,10 @@ > +2016-02-23 Mark Wielaard <mjw@redhat.com> > + Jakub Jelinek <jakub@redhat.com> > + > + PR c/69911 > + * cgraphunit.c (check_global_declaration): Check main_input_filename > + and DECL_SOURCE_FILE are not NULL. > + > 2016-02-20 Mark Wielaard <mjw@redhat.com> This is ok for trunk if it passes testing. Thanks. > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > index 27a073a..8b3fddc 100644 > --- a/gcc/cgraphunit.c > +++ b/gcc/cgraphunit.c > @@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, > static void > check_global_declaration (symtab_node *snode) > { > + const char *decl_file; > tree decl = snode->decl; > > /* Warn about any function declared static but not defined. We don't > @@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode) > || (((warn_unused_variable && ! TREE_READONLY (decl)) > || (warn_unused_const_variable > 0 && TREE_READONLY (decl) > && (warn_unused_const_variable == 2 > - || filename_cmp (main_input_filename, > - DECL_SOURCE_FILE (decl)) == 0))) > + || (main_input_filename != NULL > + && (decl_file = DECL_SOURCE_FILE (decl)) != NULL > + && filename_cmp (main_input_filename, > + decl_file) == 0)))) > && TREE_CODE (decl) == VAR_DECL)) > && ! DECL_IN_SYSTEM_HEADER (decl) > && ! snode->referred_to_p (/*include_self=*/false) Jakub
On 23/02/16 08:56, Jakub Jelinek wrote: > On Tue, Feb 23, 2016@09:53:57AM +0100, Mark Wielaard wrote: >> --- a/gcc/ChangeLog >> +++ b/gcc/ChangeLog >> @@ -1,3 +1,10 @@ >> +2016-02-23 Mark Wielaard <mjw@redhat.com> >> + Jakub Jelinek <jakub@redhat.com> >> + >> + PR c/69911 >> + * cgraphunit.c (check_global_declaration): Check main_input_filename >> + and DECL_SOURCE_FILE are not NULL. >> + >> 2016-02-20 Mark Wielaard <mjw@redhat.com> > > This is ok for trunk if it passes testing. Thanks. > >> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c >> index 27a073a..8b3fddc 100644 >> --- a/gcc/cgraphunit.c >> +++ b/gcc/cgraphunit.c >> @@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, >> static void >> check_global_declaration (symtab_node *snode) >> { >> + const char *decl_file; >> tree decl = snode->decl; >> >> /* Warn about any function declared static but not defined. We don't >> @@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode) >> || (((warn_unused_variable && ! TREE_READONLY (decl)) >> || (warn_unused_const_variable > 0 && TREE_READONLY (decl) >> && (warn_unused_const_variable == 2 >> - || filename_cmp (main_input_filename, >> - DECL_SOURCE_FILE (decl)) == 0))) >> + || (main_input_filename != NULL >> + && (decl_file = DECL_SOURCE_FILE (decl)) != NULL >> + && filename_cmp (main_input_filename, >> + decl_file) == 0)))) Can we please please please hide this ugliness behind an (inline?) function such as bool in_main_file_at (location_t) in input.[ch]? The condition here is quickly becoming unreadable. Also because in the future somebody would want to re-implement this using MAIN_FILE_P() from line-map.h, which is faster. Cheers, Manuel.
On Tue, Feb 23, 2016 at 09:51:05AM +0000, Manuel López-Ibáñez wrote: > >>diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > >>index 27a073a..8b3fddc 100644 > >>--- a/gcc/cgraphunit.c > >>+++ b/gcc/cgraphunit.c > >>@@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, > >> static void > >> check_global_declaration (symtab_node *snode) > >> { > >>+ const char *decl_file; > >> tree decl = snode->decl; > >> > >> /* Warn about any function declared static but not defined. We don't > >>@@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode) > >> || (((warn_unused_variable && ! TREE_READONLY (decl)) > >> || (warn_unused_const_variable > 0 && TREE_READONLY (decl) > >> && (warn_unused_const_variable == 2 > >>- || filename_cmp (main_input_filename, > >>- DECL_SOURCE_FILE (decl)) == 0))) > >>+ || (main_input_filename != NULL > >>+ && (decl_file = DECL_SOURCE_FILE (decl)) != NULL > >>+ && filename_cmp (main_input_filename, > >>+ decl_file) == 0)))) > > > Can we please please please hide this ugliness behind an (inline?) function > such as bool in_main_file_at (location_t) in input.[ch]? The condition here > is quickly becoming unreadable. > > Also because in the future somebody would want to re-implement this using > MAIN_FILE_P() from line-map.h, which is faster. I'm not against that, but I'd prefer if it is done incrementally, we don't want to keep the trunk broken too much for too long. So if Mark is already testing this patch, IMHO it is better to commit it in this shape sooner. Jakub
On Tue, 2016-02-23 at 09:51 +0000, Manuel López-Ibáñez wrote: > On 23/02/16 08:56, Jakub Jelinek wrote: > > On Tue, Feb 23, 2016@09:53:57AM +0100, Mark Wielaard wrote: > >> --- a/gcc/ChangeLog > >> +++ b/gcc/ChangeLog > >> @@ -1,3 +1,10 @@ > >> +2016-02-23 Mark Wielaard <mjw@redhat.com> > >> + Jakub Jelinek <jakub@redhat.com> > >> + > >> + PR c/69911 > >> + * cgraphunit.c (check_global_declaration): Check main_input_filename > >> + and DECL_SOURCE_FILE are not NULL. > >> + > >> 2016-02-20 Mark Wielaard <mjw@redhat.com> > > > > This is ok for trunk if it passes testing. Thanks. > > > >> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > >> index 27a073a..8b3fddc 100644 > >> --- a/gcc/cgraphunit.c > >> +++ b/gcc/cgraphunit.c > >> @@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, > >> static void > >> check_global_declaration (symtab_node *snode) > >> { > >> + const char *decl_file; > >> tree decl = snode->decl; > >> > >> /* Warn about any function declared static but not defined. We don't > >> @@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode) > >> || (((warn_unused_variable && ! TREE_READONLY (decl)) > >> || (warn_unused_const_variable > 0 && TREE_READONLY (decl) > >> && (warn_unused_const_variable == 2 > >> - || filename_cmp (main_input_filename, > >> - DECL_SOURCE_FILE (decl)) == 0))) > >> + || (main_input_filename != NULL > >> + && (decl_file = DECL_SOURCE_FILE (decl)) != NULL > >> + && filename_cmp (main_input_filename, > >> + decl_file) == 0)))) > > > Can we please please please hide this ugliness behind an (inline?) function > such as bool in_main_file_at (location_t) in input.[ch]? The condition here is > quickly becoming unreadable. > > Also because in the future somebody would want to re-implement this using > MAIN_FILE_P() from line-map.h, which is faster. Sorry, I pushed the minimum fix. I am too embarrassed already for breaking stuff to risk refactoring anything and making another dumb mistake. (My mistake was only checking the top-level compiler .sum files, I had forgotten to check the library ones). It might be a good idea to file a bug for this and write a patch to submit during next stage one if you believe this is a useful cleanup. Cheers, Mark
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 49f6c25..e9e1aab 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2016-02-23 Mark Wielaard <mjw@redhat.com> + Jakub Jelinek <jakub@redhat.com> + + PR c/69911 + * cgraphunit.c (check_global_declaration): Check main_input_filename + and DECL_SOURCE_FILE are not NULL. + 2016-02-20 Mark Wielaard <mjw@redhat.com> PR c/28901 diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 27a073a..8b3fddc 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, static void check_global_declaration (symtab_node *snode) { + const char *decl_file; tree decl = snode->decl; /* Warn about any function declared static but not defined. We don't @@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode) || (((warn_unused_variable && ! TREE_READONLY (decl)) || (warn_unused_const_variable > 0 && TREE_READONLY (decl) && (warn_unused_const_variable == 2 - || filename_cmp (main_input_filename, - DECL_SOURCE_FILE (decl)) == 0))) + || (main_input_filename != NULL + && (decl_file = DECL_SOURCE_FILE (decl)) != NULL + && filename_cmp (main_input_filename, + decl_file) == 0)))) && TREE_CODE (decl) == VAR_DECL)) && ! DECL_IN_SYSTEM_HEADER (decl) && ! snode->referred_to_p (/*include_self=*/false)