FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-46920: Remove code made dead long ago with #if 0 by arhadthedev · Pull Request #31681 · python/cpython · GitHub

/ cpython Public

bpo-46920: Remove code made dead long ago with #if 0 - #31681

Closed
arhadthedev wants to merge 5 commits into
python:mainfrom
arhadthedev:dead-code
ConversationCommits (5)ChecksFiles changed
Closed

bpo-46920: Remove code made dead long ago with #if 0#31681
arhadthedev wants to merge 5 commits into
python:mainfrom
arhadthedev:dead-code

Conversation

arhadthedev commented Mar 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

git grep "#if 0" gives the following occurences of dead code (analyzed with git blame, removed by this commit):

  • added on 27 Apr 2020 by 2b74c83:

    cpython/Parser/pegen.h

    Lines 9 to 11 in 8f31bf4

    #if 0
    #define PyPARSE_YIELD_IS_KEYWORD 0x0001
    #endif

    cpython/Parser/pegen.h

    Lines 15 to 19 in 8f31bf4

    #if 0
    #define PyPARSE_WITH_IS_KEYWORD 0x0003
    #define PyPARSE_PRINT_IS_FUNCTION 0x0004
    #define PyPARSE_UNICODE_LITERALS 0x0008
    #endif

    Since these constants aren't mentioned anywhere else, I think it's some initial experiment now abandoned.

  • added on 23 Apr 2020 by c5fc156:

    cpython/Parser/pegen.c

    Lines 40 to 49 in 8f31bf4

    #if 0
    static const char *
    token_name(int type)
    {
    if (0 <= type && type <= N_TOKENS) {
    return _PyParser_TokenNames[type];
    }
    return "<Huh?>";
    }
    #endif

    token_name is mentioned nowhere in the CPython codebase too.

  • added on 20 Nov 2014 by d600951:

    cpython/Python/pylifecycle.c

    Lines 1843 to 1845 in 8f31bf4

    #if 0
    _PyGC_CollectIfEnabled();
    #endif

    with the following note:

    XXX This is disabled because it caused too many problems. If a __del__ or weakref callback triggers here, Python code has a hard time running, because even the sys module has been cleared out (sys.stdout is gone, sys.excepthook is gone, etc). One symptom is a sequence of information-free messages coming from threads (if a __del__ or callback is invoked, other threads can execute too, and any exception they encounter triggers a comedy of errors as subsystem after subsystem fails to find what it expects to find in sys to help report the exception and consequent unexpected failures). I've also seen segfaults then, after adding print statements to the Python code getting called.

    So the call is faulty for seven years with no progress.

  • added on 26 Oct 2013 by 8444ebb:

    cpython/Modules/sre_lib.h

    Lines 336 to 362 in 8f31bf4

    #if 0 /* not used in this release */
    LOCAL(int)
    SRE(info)(SRE_STATE* state, const SRE_CODE* pattern)
    {
    /* check if an SRE_OP_INFO block matches at the current position.
    returns the number of SRE_CODE objects to skip if successful, 0
    if no match */
    const SRE_CHAR* end = (const SRE_CHAR*) state->end;
    const SRE_CHAR* ptr = (const SRE_CHAR*) state->ptr;
    Py_ssize_t i;
    /* check minimal length */
    if (pattern[3] && end - ptr < pattern[3])
    return 0;
    /* check known prefix */
    if (pattern[2] & SRE_INFO_PREFIX && pattern[5] > 1) {
    /* <length> <skip> <prefix data> <overlap data> */
    for (i = 0; i < pattern[5]; i++)
    if ((SRE_CODE) ptr[i] != pattern[7 + i])
    return 0;
    return pattern[0] + 2 * pattern[6];
    }
    return pattern[0];
    }
    #endif

    The Secret Labs' Regular Expression Engine was vendored nine years ago so not used in this release is permanent and can be removed.

  • added on 28 Sep 2011 by d63a3b8:

    cpython/Python/formatter_unicode.c

    Lines 140 to 156 in 8f31bf4

    #if 0
    /* Occasionally useful for debugging. Should normally be commented out. */
    static void
    DEBUG_PRINT_FORMAT_SPEC(InternalFormatSpec *format)
    {
    printf("internal format spec: fill_char %d\n", format->fill_char);
    printf("internal format spec: align %d\n", format->align);
    printf("internal format spec: alternate %d\n", format->alternate);
    printf("internal format spec: sign %d\n", format->sign);
    printf("internal format spec: width %zd\n", format->width);
    printf("internal format spec: thousands_separators %d\n",
    format->thousands_separators);
    printf("internal format spec: precision %zd\n", format->precision);
    printf("internal format spec: type %c\n", format->type);
    printf("\n");
    }
    #endif

    DEBUG_PRINT_FORMAT_SPEC is used nowhere, plus putting a debugger breakpoint onto an interesting InternalFormatSpec is more convenient.

  • added on (already commented out) 11 Dec 2007 by a3534a6:

    cpython/Objects/classobject.c

    Lines 519 to 533 in 8f31bf4

    /*
    static long
    instancemethod_hash(PyObject *self)
    {
    long x, y;
    x = (long)self;
    y = PyObject_Hash(PyInstanceMethod_GET_FUNCTION(self));
    if (y == -1)
    return -1;
    x = x ^ y;
    if (x == -1)
    x = -2;
    return x;
    }
    */

  • added on 27 Aug 2007 by e226b55:

    cpython/Objects/unicodeobject.c

    Lines 13660 to 13666 in 8f31bf4

    #if 0
    static PyObject *
    unicode__decimal2ascii(PyObject *self)
    {
    return PyUnicode_TransformDecimalAndSpaceToASCII(self);
    }
    #endif

    cpython/Objects/unicodeobject.c

    Lines 14253 to 14256 in 8f31bf4

    #if 0
    /* These methods are just used for debugging the implementation. */
    {"_decimal2ascii", (PyCFunction) unicode__decimal2ascii, METH_NOARGS},
    #endif

    _decimal2ascii is mentioned nowhere else in the CPython codebase.

  • added (already commented out) on 8 Mar 2006 by d4c9320:

    cpython/Modules/_ctypes/_ctypes.c

    Lines 1420 to 1463 in 8f31bf4

    /*
    The next three functions copied from Python's typeobject.c.
    They are used to attach methods, members, or getsets to a type *after* it
    has been created: Arrays of characters have additional getsets to treat them
    as strings.
    */
    /*
    static int
    add_methods(PyTypeObject *type, PyMethodDef *meth)
    {
    PyObject *dict = type->tp_dict;
    for (; meth->ml_name != NULL; meth++) {
    PyObject *descr;
    descr = PyDescr_NewMethod(type, meth);
    if (descr == NULL)
    return -1;
    if (PyDict_SetItemString(dict, meth->ml_name, descr) < 0) {
    Py_DECREF(descr);
    return -1;
    }
    Py_DECREF(descr);
    }
    return 0;
    }
    static int
    add_members(PyTypeObject *type, PyMemberDef *memb)
    {
    PyObject *dict = type->tp_dict;
    for (; memb->name != NULL; memb++) {
    PyObject *descr;
    descr = PyDescr_NewMember(type, memb);
    if (descr == NULL)
    return -1;
    if (PyDict_SetItemString(dict, memb->name, descr) < 0) {
    Py_DECREF(descr);
    return -1;
    }
    Py_DECREF(descr);
    }
    return 0;
    }
    */

  • added on 7 Dec 2005 by 8c8836b:

    cpython/Modules/_elementtree.c

    Lines 39 to 48 in 8f31bf4

    #if 0
    static int memory = 0;
    #define ALLOC(size, comment)\
    do { memory += size; printf("%8d - %s\n", memory, comment); } while (0)
    #define RELEASE(size, comment)\
    do { memory -= size; printf("%8d - %s\n", memory, comment); } while (0)
    #else
    #define ALLOC(size, comment)
    #define RELEASE(size, comment)
    #endif

    In addition, ALLOC() and RELEASE() are removed as unconditional no-ops.

  • added on 4 Aug 2002 by 00f1e3f:

    cpython/Parser/tokenizer.c

    Lines 285 to 308 in 8f31bf4

    #if 0
    /* Disable support for UTF-16 BOMs until a decision
    is made whether this needs to be supported. */
    } else if (ch1 == 0xFE) {
    ch2 = get_char(tok);
    if (ch2 != 0xFF) {
    unget_char(ch2, tok);
    unget_char(ch1, tok);
    return 1;
    }
    if (!set_readline(tok, "utf-16-be"))
    return 0;
    tok->decoding_state = STATE_NORMAL;
    } else if (ch1 == 0xFF) {
    ch2 = get_char(tok);
    if (ch2 != 0xFE) {
    unget_char(ch2, tok);
    unget_char(ch1, tok);
    return 1;
    }
    if (!set_readline(tok, "utf-16-le"))
    return 0;
    tok->decoding_state = STATE_NORMAL;
    #endif

    /* Disable support for UTF-16 BOMs until a decision is made whether this needs to be supported */ - I have every reason to believe that the decision was Never Ever.

  • added on 23 Mar 2002 by c24ea08:

    cpython/Python/ceval.c

    Lines 7065 to 7070 in 8f31bf4

    #if 0 /* future keyword */
    if (codeflags & CO_GENERATOR_ALLOWED) {
    result = 1;
    cf->cf_flags |= CO_GENERATOR_ALLOWED;
    }
    #endif

    The /* future keyword */ is quite overdue already.

  • added on 23 Jun 2001 by 01dfdb3:

    cpython/Modules/getaddrinfo.c

    Lines 41 to 58 in 8f31bf4

    #if 0
    #include <sys/types.h>
    #include <sys/param.h>
    #include <sys/sysctl.h>
    #include <sys/socket.h>
    #include <netinet/in.h>
    #include <arpa/inet.h>
    #include <arpa/nameser.h>
    #include <netdb.h>
    #include <resolv.h>
    #include <string.h>
    #include <stdlib.h>
    #include <stddef.h>
    #include <ctype.h>
    #include <unistd.h>
    #include "addrinfo.h"
    #endif

    cpython/Modules/getnameinfo.c

    Lines 37 to 49 in 8f31bf4

    #if 0
    #include <sys/types.h>
    #include <sys/socket.h>
    #include <netinet/in.h>
    #include <arpa/inet.h>
    #include <arpa/nameser.h>
    #include <netdb.h>
    #include <resolv.h>
    #include <string.h>
    #include <stddef.h>
    #include "addrinfo.h"
    #endif

  • added on 14 Jul 1998 by 43ff868:

    cpython/Modules/_tkinter.c

    Lines 3443 to 3448 in 8f31bf4

    #if 0
    /* This was not a good idea; through <Destroy> bindings,
    Tcl_Finalize() may invoke Python code but at that point the
    interpreter and thread state have already been destroyed! */
    Py_AtExit(Tcl_Finalize);
    #endif

git grep "#if 1" gives the following:

  • added on 20 Nov 2014 by d600951:

    cpython/Python/pylifecycle.c

    Line 2427 in 8f31bf4

    #if 1 /* Disable this if you have trouble debugging bootstrap stuff */

    After seven years, no trouble was found so this always-true guard has no use.

https://bugs.python.org/issue46920

arhadthedev commented Mar 4, 2022

Copy link
Copy Markdown
Member Author

There is more potential dead code, though I don't know if it can be safely removed.

  • added 3 Dec 2021 by 99fcf15:

    cpython/Modules/getpath.c

    Lines 919 to 934 in 8f31bf4

    #if 0
    PyObject *it = PyObject_GetIter(configDict);
    for (PyObject *k = PyIter_Next(it); k; k = PyIter_Next(it)) {
    if (!strcmp("__builtins__", PyUnicode_AsUTF8(k))) {
    Py_DECREF(k);
    continue;
    }
    fprintf(stderr, "%s = ", PyUnicode_AsUTF8(k));
    PyObject *o = PyDict_GetItem(configDict, k);
    o = PyObject_Repr(o);
    fprintf(stderr, "%s\n", PyUnicode_AsUTF8(o));
    Py_DECREF(o);
    Py_DECREF(k);
    }
    Py_DECREF(it);
    #endif

    @zooba Is this recently added loop necessary or it can be removed?

  • added on 28 Feb 2021 by 73a85c4:

    cpython/Objects/stringlib/fastsearch.h

    Lines 170 to 183 in 8f31bf4

    /* Change to a 1 to see logging comments walk through the algorithm. */
    #if 0 && STRINGLIB_SIZEOF_CHAR == 1
    # define LOG(...) printf(__VA_ARGS__)
    # define LOG_STRING(s, n) printf("\"%.*s\"", (int)(n), s)
    # define LOG_LINEUP() do { \
    LOG("> "); LOG_STRING(haystack, len_haystack); LOG("\n> "); \
    LOG("%*s",(int)(window_last - haystack + 1 - len_needle), ""); \
    LOG_STRING(needle, len_needle); LOG("\n"); \
    } while(0)
    #else
    # define LOG(...)
    # define LOG_STRING(s, n)
    # define LOG_LINEUP()
    #endif

    @sweeneyde Can it be removed nowadays? LOG_*() are used thorough the file so it looks like an involved debugging facility.

  • added on 26 Jun 2001 by b6c3cea:

    cpython/Lib/test/test_generators.py

    Lines 1242 to 1243 in 8f31bf4

    # syntax_tests mostly provokes SyntaxErrors. Also fiddling with #if 0
    # hackery.

    @tim-one Can this comment (or even the whole chunks of test_generators.py introduced by b6c3cea) be removed for good?

corona10 requested a review from vstinner March 4, 2022 13:56

ericvsmith commented Mar 4, 2022

Copy link
Copy Markdown
Member

I don't think all of these should be removed. Some of them look genuinely useful for debugging, others are essentially multi-line comments.

In any event, they should be individually considered.

arhadthedev commented Mar 5, 2022

Copy link
Copy Markdown
Member Author

@ericvsmith I agree that the debugging share of code appeared for a reason so I'm open to evaluation and ready to exclude parts from the PR.

BTW I decided to remove that potentially-not-dead share because it either requires some missing harness or looks finished to serve its time after a tested feature was stabilized long ago.

vstinner approved these changes Mar 7, 2022
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

Comment thread Python/pylifecycle.c
*/
#if 0
_PyGC_CollectIfEnabled();
#endif

vstinner Mar 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I spent significant time to rework how the GC is called. The last call is now in interpreter_clear() of pystate.c.
See: https://pythondev.readthedocs.io/finalization.html

vstinner commented Mar 7, 2022

Copy link
Copy Markdown
Member

I don't think all of these should be removed. Some of them look genuinely useful for debugging, others are essentially multi-line comments.

I often work on the Python initialization and finalization code and I never needed to disble the #if 1 code. If anyone needs it, it's trivial to rewrite it. I deeply reworked the pylifecycle.c code to have shorter functions, so the code should be easier to follow than in Python 3.5.

vstinner reviewed Mar 7, 2022
View reviewed changes
Comment thread Python/pylifecycle.c
* XXX the exception and consequent unexpected failures). I've also
* XXX seen segfaults then, after adding print statements to the
* XXX Python code getting called.
*/

vstinner Mar 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This comment is oudated and must be removed.

vstinner commented Mar 7, 2022

Copy link
Copy Markdown
Member

@mdickinson @ericvsmith: Would you mind to review again the PR? If would prefer a second approval since there was disagreement on the first reviews.

sweeneyde reviewed Mar 7, 2022
View reviewed changes
Comment thread Python/compile.c
/* For debugging purposes only */
#if 0
static void
dump_instr(struct instr *i)

sweeneyde Mar 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

These have been useful occasionally, so I would rather not remove them.

arhadthedev Mar 8, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Restored.

sweeneyde commented Mar 7, 2022

Copy link
Copy Markdown
Member

Re: fastsearch.h, I'd like to keep those debugging prints. If a bug or performance curiosity ever shows up, it would be nice to flip the 0 to a 1, recompile, and then be able to tell exactly what's going on, step-by-step.

mdickinson commented Mar 7, 2022

Copy link
Copy Markdown
Member

@mdickinson @ericvsmith: Would you mind to review again the PR? If would prefer a second approval since there was disagreement on the first reviews.

I don't have any strong opinion on any of these changes besides the vendorised code changes that were already addressed (for which thank you, @arhadthedev!). I'm personally happy for the rest of the changes to go through, but other people's concerns should be taken into account.

tim-one commented Mar 7, 2022

Copy link
Copy Markdown
Member
  • Can this comment (or even the whole chunks of test_generators.py introduced by b6c3cea) be removed for good?

No. The comment is simply wrong: it's testing the behavior of "if 0:" in Python, nothing to do with "#if 0" in C.

ericvsmith commented Mar 7, 2022

Copy link
Copy Markdown
Member

Except for ones that are out of date, wrong, or misleading, I don't see much need in deleting any of these.

arhadthedev commented Mar 8, 2022

Copy link
Copy Markdown
Member Author

I found three more functions disabled by commenting them out. Since they were put into a comment from the moment of being introduced, I suspect they are obsolete so removed them too:

  • added on 11 Dec 2007 by a3534a6:

    cpython/Objects/classobject.c

    Lines 519 to 533 in 8f31bf4

    /*
    static long
    instancemethod_hash(PyObject *self)
    {
    long x, y;
    x = (long)self;
    y = PyObject_Hash(PyInstanceMethod_GET_FUNCTION(self));
    if (y == -1)
    return -1;
    x = x ^ y;
    if (x == -1)
    x = -2;
    return x;
    }
    */
  • added on 8 Mar 2006 by d4c9320:

    cpython/Modules/_ctypes/_ctypes.c

    Lines 1420 to 1463 in 8f31bf4

    /*
    The next three functions copied from Python's typeobject.c.
    They are used to attach methods, members, or getsets to a type *after* it
    has been created: Arrays of characters have additional getsets to treat them
    as strings.
    */
    /*
    static int
    add_methods(PyTypeObject *type, PyMethodDef *meth)
    {
    PyObject *dict = type->tp_dict;
    for (; meth->ml_name != NULL; meth++) {
    PyObject *descr;
    descr = PyDescr_NewMethod(type, meth);
    if (descr == NULL)
    return -1;
    if (PyDict_SetItemString(dict, meth->ml_name, descr) < 0) {
    Py_DECREF(descr);
    return -1;
    }
    Py_DECREF(descr);
    }
    return 0;
    }
    static int
    add_members(PyTypeObject *type, PyMemberDef *memb)
    {
    PyObject *dict = type->tp_dict;
    for (; memb->name != NULL; memb++) {
    PyObject *descr;
    descr = PyDescr_NewMember(type, memb);
    if (descr == NULL)
    return -1;
    if (PyDict_SetItemString(dict, memb->name, descr) < 0) {
    Py_DECREF(descr);
    return -1;
    }
    Py_DECREF(descr);
    }
    return 0;
    }
    */

ericvsmith commented Mar 9, 2022

Copy link
Copy Markdown
Member

It would be easier to review this if you opened new issues when you found other code you want removed. Ideally (for me, at least), each piece of code you want to remove would be in a separate PR, so they could be easily reviewed and merged. I'm not sure if it's worth doing that at this point, but because there are so many files that are touched in this PR, I don't think I can give it a proper review.

arhadthedev commented Mar 9, 2022

Copy link
Copy Markdown
Member Author

Ideally (for me, at least), each piece of code you want to remove would be in a separate PR, so they could be easily reviewed and merged.

Initially I didn't want to create a dosen of small PRs but now I see your point. I also got it that small PRs allow to get the most of pieces merged sooner without being blocked by more ambivalent ones.

I'll make the split tomorrow.

This was referenced Mar 11, 2022
Merged
Merged
Merged
Merged

arhadthedev commented Mar 11, 2022

Copy link
Copy Markdown
Member Author

@ericvsmith I split this PR into smaller ones to not block merging of non-controversial pieces: GH-31811, GH-31812, GH-31813, and GH-31814.

cc: @mdickinson

arhadthedev deleted the dead-code branch March 14, 2022 16:57

arhadthedev commented Mar 14, 2022

Copy link
Copy Markdown
Member Author

@vstinner Thanks for merging all the commits.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner approved these changes
sweeneyde sweeneyde left review comments
rhettinger Awaiting requested review from rhettinger
pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
markshannon Awaiting requested review from markshannon markshannon is a code owner
+1 more reviewer
mdickinson mdickinson left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting merge skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL