From patchwork Thu Apr 30 04:13:30 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 466366 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 22B8B140310 for ; Thu, 30 Apr 2015 14:13:43 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Nq6twYYN; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=hN7762MzJ4tfqoQlc CTu4lzBArZaTvqCmFNreRsN/erQIUQVoXibPRrujN5xw2GQDyzAg2kFuCj4veu/v RvmPzVQb4NjCtjm5TWemHzB7K9gG+iG9wCO7P7Ga3MJOU0lDrwDpzLxLZuBvlWQE saVs6OaNr7CNrHLieSXYeOY6wg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=kKu7cq5qF+QNgq+g9wOXLx3 P6eI=; b=Nq6twYYNtVcmDEBLh+9FgA2Tvm26UuD2VCpxVQabBIdsrXrZSFi9T4R RhEzytq7q/w6ujhQehtwxveZE5WDIXS2Oyj7n7W9qDLxStlzHI89SCMej0PYCeQC LqzPuRSLiLTS13CLHyccMv/J/oy3Qz8uzYPLOdjfirexjE3zyg94= Received: (qmail 4786 invoked by alias); 30 Apr 2015 04:13:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 4770 invoked by uid 89); 30 Apr 2015 04:13:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL, BAYES_20, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 30 Apr 2015 04:13:34 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3U4DW1B024728 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 30 Apr 2015 00:13:32 -0400 Received: from reynosa.quesejoda.com (vpn-48-175.rdu2.redhat.com [10.10.48.175]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3U4DV7v012555; Thu, 30 Apr 2015 00:13:31 -0400 Message-ID: <5541ABEA.1050301@redhat.com> Date: Wed, 29 Apr 2015 21:13:30 -0700 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Jason Merrill , Richard Biener CC: gcc-patches Subject: Re: [patch] Rewrite check_global_declarations() generically References: <55402D60.7020103@redhat.com> <554131EE.1020000@redhat.com> In-Reply-To: <554131EE.1020000@redhat.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 commit 5ffe67af1604495d4cafeae4b3e948bf2eac77b3 Author: Aldy Hernandez 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 --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 % 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. */