From owner-freebsd-arch@FreeBSD.ORG Wed Aug 31 12:32:32 2005 Return-Path: X-Original-To: freebsd-arch@freebsd.org Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CAA1E16A41F for ; Wed, 31 Aug 2005 12:32:32 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailout1.pacific.net.au (mailout1.pacific.net.au [61.8.0.84]) by mx1.FreeBSD.org (Postfix) with ESMTP id A4ADF43D46 for ; Wed, 31 Aug 2005 12:32:31 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.0.86]) by mailout1.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j7VCWBo7006989; Wed, 31 Aug 2005 22:32:11 +1000 Received: from epsplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j7VCW8JO026584; Wed, 31 Aug 2005 22:32:10 +1000 Date: Wed, 31 Aug 2005 22:32:08 +1000 (EST) From: Bruce Evans X-X-Sender: bde@epsplex.bde.org To: Craig Rodrigues In-Reply-To: <20050831112720.GA55376@crodrigues.org> Message-ID: <20050831215640.S1678@epsplex.bde.org> References: <20050810005323.GA42721@crodrigues.org> <20050810032308.GA80916@dragon.NUXI.org> <20050827235140.GA3063@crodrigues.org> <20050828172712.T86328@delplex.bde.org> <20050831112720.GA55376@crodrigues.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-arch@freebsd.org Subject: Re: [RFC] -Wredundant-decls: keep it or remove it? X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Aug 2005 12:32:32 -0000 On Wed, 31 Aug 2005, Craig Rodrigues wrote: > On Sun, Aug 28, 2005 at 05:36:01PM +1000, Bruce Evans wrote: >> It should warn about static variable decls iff they are redundant. This >> requires determining if the new declaration adds info. I couldn't find >> any macros to help determine this, not even ones to say if the new >> declaration has an initializer and the old one doesn't. > > DECL_INITIAL is the macro to tell if a node is part of an > initializer or not. Ah. I see that gcc already uses this macro a lot for similar things and not just for warnings -- changing the initializer from its default of 0 (when there is no explicit initializer) to a non default require checking whether the previous initializer is the default since there cannot be more than 1 explicit initializer. > I updated the patch to GCC to use DECL_INITIAL and submitted a testcase here: > > http://gcc.gnu.org/ml/gcc-patches/2005-08/msg01812.html % Index: c-decl.c % =================================================================== % RCS file: /cvsroot/gcc/gcc/gcc/c-decl.c,v % retrieving revision 1.630.6.16 % diff -u -u -r1.630.6.16 c-decl.c % --- c-decl.c 16 Aug 2005 20:34:19 -0000 1.630.6.16 % +++ c-decl.c 31 Aug 2005 04:55:40 -0000 % @@ -1559,7 +1559,11 @@ % && !(DECL_EXTERNAL (olddecl) && !DECL_EXTERNAL (newdecl)) % /* Don't warn about forward parameter decls. */ % && !(TREE_CODE (newdecl) == PARM_DECL % - && TREE_ASM_WRITTEN (olddecl) && !TREE_ASM_WRITTEN (newdecl))) % + && TREE_ASM_WRITTEN (olddecl) && !TREE_ASM_WRITTEN (newdecl)) % + /* Don't warn about forward static variable decls. */ This should something like say "Don't warn about a static variable definition following a declaration." Many of the preceding comments should be similarly reworded. % + && !(TREE_CODE (newdecl) == VAR_DECL % + && !TREE_PUBLIC (olddecl) && !TREE_PUBLIC (newdecl) % + && DECL_INITIAL (newdecl) && !DECL_INITIAL (olddecl))) This seems reasonable. Is it necessary to check TREE_PUBLIC () explicitly? We have already avoided warning for externs, so only weird cases are left. I can't see any reason not to use simply: /* Don't warn about a definition following a declaration. */ if (DECL_INITIAL (newdecl) && !DECL_INITIAL (olddecl))) since a definition (i.e., a declaration with an initializer) following a declaration (i.e., a tentative definition) can never be redundant. % { % warning ("%Jredundant redeclaration of %qD", newdecl, newdecl); % warned = true; Bruce