Re: [patch] Rewrite check_global_declarations() generically

[Aldy Hernandez <aldyh at redhat dot com>]

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

Attachment: curr
Description: Text document