I see dead code

[toddr]

During a code review, I discovered quite a bit of code with #if 0 code in it. Unless we have a good reason we should remove them.

When time allows, I would like to remove this code and see what breaks, possibly for merge.

I'm a little worried people have patches built around this where they do s/#if 0/#if 1/

[jkeenan]

On 7/29/20 7:51 PM, Todd Rinaldo wrote: During a code review, I discovered quite a bit of code with significant |#if 0| code in it. Unless we have a good reason we should remove them. When time allows, I would like to remove this code and see what breaks, possibly for merge. I'm a little worried people have patches built around this where they do |s/#if 0/#if 1/|

Could you provide some examples? Thank you very much. Jim Keenan

[atoomic]

here are a few samples from git grep '#if 0'

╰─> git grep '#if 0'
Configure:17918:#if 0
Configure:19196:#if 0 /* strtoul() for /^-/ strings is undefined. */
Configure:19209:#if 0 /* strtoul() for /^-/ strings is undefined. */
Configure:19275:#if 0 /* strtoull() for /^-/ strings is undefined. */
Configure:19329:#if 0 /* strtouq() for /^-/ strings is undefined. */
NetWare/nw5thread.h:146:#if 0
NetWare/nw5thread.h:154:#endif  //#if 0
cpan/Compress-Raw-Bzip2/Bzip2.xs:167:#if 0
cpan/Compress-Raw-Bzip2/Bzip2.xs:359:#if 0
cpan/Compress-Raw-Bzip2/Bzip2.xs:415:#if 0
cpan/Compress-Raw-Zlib/Zlib.xs:109:#if 0
cpan/Compress-Raw-Zlib/Zlib.xs:198:#if 0 /* VM/CMS */
cpan/Compress-Raw-Zlib/Zlib.xs:214:#if 0 /* Z-System */
cpan/Compress-Raw-Zlib/Zlib.xs:218:#if 0 /* CP/M */
cpan/Compress-Raw-Zlib/Zlib.xs:230:#if 0 /* QDOS */
cpan/Compress-Raw-Zlib/Zlib.xs:234:#if 0 /* Acorn RISCOS */
cpan/Compress-Raw-Zlib/Zlib.xs:238:#if 0 /* ???  */
cpan/Compress-Raw-Zlib/Zlib.xs:438:#if 0
cpan/Compress-Raw-Zlib/Zlib.xs:645:#if 0
cpan/Encode/Encode.xs:655:#if 0
cpan/ExtUtils-Constant/lib/ExtUtils/Constant/Base.pm:81:        return $macro ? "#ifdef $macro\n" : "#if 0\n";
cpan/Win32API-File/const2perl.h:191://    return "#if 0";
dist/Devel-PPPort/PPPort_pm.PL:749:#if 0
dist/Devel-PPPort/parts/embed.fnc:2002:#if 0    /* Not currently used, but may be needed in the future */
dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm:211:          # comment inside #if 0, as the C standard says that the source
dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm:222:          print("#if 0\n  \"Skipped embedded POD.\"\n#endif\n");
dist/Storable/Storable.xs:3068:#if 0
embed.fnc:2360:#if 0    /* Not currently used, but may be needed in the future */
ext/File-Glob/bsd_glob.c:226:#if 0
ext/File-Glob/bsd_glob.c:449:#if 0
ext/PerlIO-encoding/encoding.xs:649:#if 0
ext/PerlIO-mmap/mmap.xs:61:#if 0 && defined(HAS_MADVISE) && defined(MADV_SEQUENTIAL)
ext/PerlIO-mmap/mmap.xs:64:#if 0 && defined(HAS_MADVISE) && defined(MADV_WILLNEED)
ext/PerlIO-via/via.xs:49:#if 0
ext/SDBM_File/sdbm.c:514:#if 0
ext/attributes/attributes.xs:205:#if 0  /* this was probably a bad idea */
fakesdio.h:86:#if 0
locale.c:1222:#if 0   /* Code that was to emulate thread-safe locales on platforms that
op.c:5346:#if 0
op.c:5907:#if 0
op.c:8013:#if 0   /* code that added excess above-255 chars at the end of the table, in
op_reg_common.h:110:#if 0
os2/OS2/OS2-Process/Process.xs:239:#if 0
os2/OS2/OS2-Process/Process.xs:349:#if 0                /* Need to have the entry points described in the parent */
os2/OS2/OS2-Process/Process.xs:638:#if 0
os2/OS2/OS2-Process/Process.xs:670:#if 0                        /*  Does not work.  */
os2/OS2/OS2-Process/Process.xs:715:#if 0
os2/OS2/OS2-Process/Process.xs:724:#if 0                        /*  Does not work.  */
os2/OS2/OS2-Process/Process.xs:941:#if 0        /* buf=323552247, full=1118455, partial=0 */
os2/OS2/OS2-Process/Process.xs:1692:#if 0
os2/OS2/OS2-Process/Process.xs:1827:#if 0
os2/OS2/OS2-REXX/REXX.xs:9:#if 0
os2/OS2/OS2-REXX/REXX.xs:130:#if 0                                      /* Do we want to restore these? */
os2/OS2/OS2-REXX/REXX.xs:173:#if 0
os2/os2.c:553:#if 0
os2/os2.c:1104:#if 0
os2/os2.c:2052:#if 0
os2/os2.c:4788:#if 0 /* _atexit_n is not exported */
perlio.c:1069:#if 0
perlio.c:2050:#if 0
perlio.c:2415:#if 0
perlio.c:3227:#if 0
perlio.c:3486:#if 0
perlio.c:4530:#if 0
perlio.c:4706:#if 0
proto.h:4487:#if 0      /* Not currently used, but may be needed in the future */
regcomp.c:5728:#if 0
regcomp.c:5834:#if 0
regcomp.c:12133:#if 0  /* Removing this doesn't cause failures in the test suite -- khw */
regcomp.c:12783:#if 0                           /* Now runtime fix should be reliable. */
regcomp.c:18538:#if 0   /* Have decided not to deal with multi-char folds in inverted classes,
regcomp.c:18591:#if 0
regcomp.c:25438:#if 0
regexec.c:376:     * a better location to since this code can #if 0-ed out
sv.c:15742:#if 0
t/lib/h2ph.h:75:#if 0
t/porting/diag.t:403:    # inside an #if 0 block.
utf8.c:858:#if 0
utf8.c:3152:#if 0       /* Not currently used, but may be needed in the future */
vms/vms.c:406:#if 0 /* I do not see longer sequences supported by OpenVMS */
win32/include/dirent.h:44:#if 0         /* these have moved to win32iop.h */
win32/vmem.h:235:#if 0
win32/vmem.h:880:#if 0
win32/win32.c:4714:#if 0
win32/win32io.c:308:#if 0

[iabyn]

On Wed, Jul 29, 2020 at 04:51:22PM -0700, Todd Rinaldo wrote: During a code review, I discovered quite a bit of code with significant `#if 0` code in it. Unless we have a good reason we should remove them.

In my experience this is usually done as a bandaid: "this block of code is wrong but I don't know how / don't have the time to fix it". I wouldn't like these blocks to be removed without the underlying issues being addressed.

…

-- Little fly, thy summer's play my thoughtless hand has terminated with extreme prejudice. (with apologies to William Blake)

[benkasminbullock]

• In regcomp.c,

This

c277df4#diff-dc8c62daf844c4b19510dfce777053a52ac4821852ef9818e3de8912dd3c4edbR519-R533

was added to the codebase in 1997, complete with the #if 0, so it was never commented out but added to the codebase as it was.

This
9b139d0#diff-dc8c62daf844c4b19510dfce777053a52ac4821852ef9818e3de8912dd3c4edbR4087

is a printf debugging which was left in the code during the commit.

The other four cases of #if 0 were added by @khwilliamson, and they have comments on them.

• In op.c, there are two #if 0 which are both from this commit from 2002:

d2be0de

and one from about 14 months ago, again by khwilliamson, which is clearly commented.

• In perlio.c,

some of the #if 0 are just wrapping old printf debugging and could easily be removed. Four such statements were added in this commit:

5e2ab84

Most of them were removed but there is one left still in the code.

Another one, from this commit:

3b4bd3f

by Nick Ing-Simmons from 2001, is commented with "Test code - delete when it works ..."

Another one dates from this commit:

88b61e1

again by Nick Ing-Simmons, from 2000. It has a comment about FIXME but I doubt that will be useful now.

[Leont]

In my experience this is usually done as a bandaid: "this block of code is
wrong but I don't know how / don't have the time to fix it".

I wouldn't like these blocks to be removed without the underlying issues
being addressed.

Yeah. Taking a good look over them is probably a good idea, probably some can be removed and many undoubtedly could use an explanation, but blanket removal is unlikely to be helpful.

[benkasminbullock]

The single #if 0 in sv.c was added here:

dd1a3ba

Out of all the os2/OS2/OS2-Process/Process.xs cases, the youngest one is fifteen years old, and the oldest is 23 years old:

https://github.com/Perl/perl5/blame/blead/os2/OS2/OS2-Process/Process.xs

They are marked with comments like "does not work". All of them were added in commits by Ilya Zakharevich.

Of the five in Configure,

https://github.com/Perl/perl5/blame/blead/Configure

one was commented out two years ago, and the others all date from a single commit 21 years ago by jhi:

c11ecd6