diff mbox

Rewrite check_global_declarations() generically

Message ID 5541ABEA.1050301@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez April 30, 2015, 4:13 a.m. UTC 
On 04/29/2015 12:33 PM, Jason Merrill wrote:
> On 04/28/2015 09:01 PM, Aldy Hernandez wrote:
>
> The approach looks good to me.
>
>> -  analyze_functions ();
>> +  analyze_functions (true);
>
> In the C++ front end at least we comment anonymous boolean arguments, i.e.
>
>   analyze_functions (/*first_time*/true);
>
> Let's do that here, too.  Similarly for the calls to referred_to_p (false).

Done.

>
>> +      /* ?? Why are we looking at TREE_USED?  Shouldn't the call to
>> +     referred_to_p above be enough?  Apparently not, because the
>> +     `__unused__' attribute is not being considered for
>> +     referred_to_p.  */
>
> Seems like you answered your question.  :)

I've adjusted the comment.

>
>> +      /* Global ctors and dtors are called by the runtime.  */
>> +      && (TREE_CODE (decl) != FUNCTION_DECL
>> +      || (!DECL_STATIC_CONSTRUCTOR (decl)
>> +          && !DECL_STATIC_DESTRUCTOR (decl)))
>
> Maybe check snode->needed_p instead?

I thought so too, but it's a bit too restrictive.  Particularly, it 
causes this test to fail:

static void foo (void) {} /* { dg-warning "'foo' defined but not used" } */
static void bar (void) { bar (); } /* { dg-warning "'bar' defined but 
not used" } */

...because force_output is true, which unfortunately is set here:

   /* When not optimizing, also output the static functions. (see
      PR24561), but don't do so for always_inline functions, functions
      declared inline and nested functions.  These were optimized out
      in the original implementation and it is unclear whether we want
      to change the behavior here.  */
   if ((!opt_for_fn (decl, optimize)
        && !node->cpp_implicit_alias
        && !DECL_DISREGARD_INLINE_LIMITS (decl)
        && !DECL_DECLARED_INLINE_P (decl)
        && !(DECL_CONTEXT (decl)
	    && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL))
       && !DECL_COMDAT (decl) && !DECL_EXTERNAL (decl))
     node->force_output = 1;

I've left it as is, and am committing the attached incremental patch.

Thanks.
Aldy
commit 5ffe67af1604495d4cafeae4b3e948bf2eac77b3
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Wed Apr 29 18:04:58 2015 -0700

    Provide comments for some boolean arguments.
    
    Use needed_p instead of looking at DECL_*_CONSTRUCTOR.
diff mbox

Patch

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 0ee1d2b..0873162 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2467,13 +2467,13 @@  symbol_table::finalize_compilation_unit (void)
 
   /* Gimplify and lower all functions, compute reachability and
      remove unreachable nodes.  */
-  analyze_functions (true);
+  analyze_functions (/*first_time=*/true);
 
   /* Mark alias targets necessary and emit diagnostics.  */
   handle_alias_pairs ();
 
   /* Gimplify and lower thunks.  */
-  analyze_functions (false);
+  analyze_functions (/*first_time=*/false);
 
   /* Emit early debug for reachable functions, and by consequence,
      locally scoped symbols.  */
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 3155595..9a27ca2 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -505,8 +505,6 @@  wrapup_global_declarations (tree *vec, int len)
 void
 check_global_declaration (tree decl)
 {
-  // ?? Perhaps we should avoid all DECL_ARTIFICIALs here?
-
   /* Warn about any function declared static but not defined.  We don't
      warn about variables, because many programs have static variables
      that exist only to get some text into the object file.  */
@@ -518,9 +516,9 @@  check_global_declaration (tree decl)
       && ! TREE_NO_WARNING (decl)
       && ! TREE_PUBLIC (decl)
       && (warn_unused_function
-	  || snode->referred_to_p (false)))
+	  || snode->referred_to_p (/*include_self=*/false)))
     {
-      if (snode->referred_to_p (false))
+      if (snode->referred_to_p (/*include_self=*/false))
 	pedwarn (input_location, 0, "%q+F used but never defined", decl);
       else
 	warning (OPT_Wunused_function, "%q+F declared %<static%> but never defined", decl);
@@ -535,11 +533,10 @@  check_global_declaration (tree decl)
        || (warn_unused_variable
 	   && TREE_CODE (decl) == VAR_DECL && ! TREE_READONLY (decl)))
       && ! DECL_IN_SYSTEM_HEADER (decl)
-      && ! snode->referred_to_p (false)
-      /* ?? Why are we looking at TREE_USED?  Shouldn't the call to
-	 referred_to_p above be enough?  Apparently not, because the
-	 `__unused__' attribute is not being considered for
-	 referred_to_p.  */
+      && ! snode->referred_to_p (/*include_self=*/false)
+      /* This TREE_USED check is needed in addition to referred_to_p
+	 above, because the `__unused__' attribute is not being
+	 considered for referred_to_p.  */
       && ! TREE_USED (decl)
       /* The TREE_USED bit for file-scope decls is kept in the identifier,
 	 to handle multiple external decls in different scopes.  */