[patch] delete old dead code

Joseph S. Myers jsm28@cam.ac.uk
Mon May 28 09:37:00 GMT 2001


On Mon, 28 May 2001, Sam TH wrote:

> The attached patch deletes #if 0 blocks that have existed since the
> beginning of egcs.  From the comments, some have existed much longer
> than that - one references gcc 2.4 in the future, and two are from
> 1994.  

In some cases, the comments inside the #if 0 block explain why you don't
want to do what the block does.  Since the blocks can serve as
documentation, but comments are better, most blocks should be replaced by
comments (possibly the ones currently there, possibly modified versions)  
explaining why you don't want to do what's in the block, to avoid someone
making the mistake of adding essentially what's in the block back again.

> Two additional questions - is there CVS history available anywhere

old-releases/old-cvs on GCC mirrors.

> from before egcs began?  And are the archives of the pre-egcs mailing
> lists available?

There were archives of the bug-reporting and announcement lists available
from ftp-mailing-list-archives.gnu.org, but that's been broken for some
time.  Ask webmaster@gnu.org about them - if you can get them to put the
mbox archives of those lists up for ftp somewhere, I think we should put
them on gcc.gnu.org, both for ftp and online run through mhonarc.  
Development was I think on a private list.

Here are comments on a few of the #if 0 blocks.  (Could you use the "-p" 
option to diff to make it clearer where the code is coming from?)  
Everywhere in the C front end I haven't commented on may need further 
consideration and possibly an appropriate comment, or you could send a 
patch for just those cases in the C front end where I made an unequivocal 
comment on what was required, so I can approve those first.

At what point do simple changes such as code removal require a copyright 
assignment?

> --- c-common.c	2001/05/25 20:00:54	1.236
> +++ c-common.c	2001/05/28 14:23:21
> @@ -2535,28 +2535,6 @@
>    if (TREE_CODE (expr) == ERROR_MARK)
>      return expr;
>  
> -#if 0 /* This appears to be wrong for C++.  */

I think removing this outright is OK.

> --- c-convert.c	2001/04/23 22:55:31	1.11
> +++ c-convert.c	2001/05/28 14:23:22
> @@ -80,12 +80,6 @@
>      }
>    if (code == VOID_TYPE)
>      return build1 (CONVERT_EXPR, type, e);
> -#if 0
> -  /* This is incorrect.  A truncation can't be stripped this way.
> -     Extensions will be stripped by the use of get_unwidened.  */

Keep some sort of comment.

> -#if 0 /* Produces wrong result if within sizeof.  */
> -  /* Don't promote the operands separately if they promote
> -     the same way.  Return the unpromoted type and let the combined
> -     value get promoted if necessary.  */

OK to eliminate altogether - I doubt someone's going to try to change this 
back.

> -#if 0 /* If something inside inhibited lvalueness, we should not override.  */
> -      /* Consider (x, y+0), which is not an lvalue since y+0 is not.  */
> -
> -      /* Strip NON_LVALUE_EXPRs since we aren't using as an lvalue.  */

Maybe keep some sort of comment.

> -  /* This warning is turned off because it causes warnings for
> -     declarations like `extern struct foo *x'.  */
> -#if 0
> -  /* Warn about incomplete structure types in this level.  */

Keep some sort of comment.

> -#if 0
> -  if (DECL_NAME (decl))
> -    {
> -      tree olddecl;
> -      olddecl = lookup_name (DECL_NAME (decl));
> -      if (pedantic && olddecl != 0 && TREE_CODE (olddecl) == TYPE_DECL)
> -	pedwarn_with_decl (decl,
> -			   "ANSI C forbids parameter `%s' shadowing typedef");
> -    }
> -#endif

OK to remove outright.

> -#if 0
> -	    /* Leave the field const or volatile as well.  */
> -	    type_quals = TYPE_UNQUALIFIED;
> -#endif

Keep comment.

> -#if 0
> -      /* In a fcn definition, arg types must be complete.  */
> -      if (funcdef_flag)
> -#endif

OK to remove, since the comment above the #if 0 block explains it.

> -#if 0
> -      /* If this function takes a variable number of arguments,
> -	 add a phony parameter to the end of the parm list,
> -	 to represent the position of the first unnamed argument.  */

While I think this can be removed outright, I'd rather understand what it 
was ever getting at first.

> -
> -  /* ??? This might be an improvement,
> -     but needs to be thought about some more.  */
> -#if 0
> -  keep_next_level_flag = 1;
> -#endif

What is this getting at?

> -
> -#if 0
> -struct try_type
> -{

I think this can go outright.

-- 
Joseph S. Myers
jsm28@cam.ac.uk



More information about the Gcc-patches mailing list