[RFC] -Wredundant-decls: keep it or remove it?

[Bruce Evans]

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