Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dead code should be removed #61718

Closed
vstinner opened this issue Mar 22, 2013 · 11 comments
Closed

Dead code should be removed #61718

vstinner opened this issue Mar 22, 2013 · 11 comments
Labels
type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

BPO 17516
Nosy @birkenfeld, @rhettinger, @vstinner, @ezio-melotti, @bitdancer
Files
  • deadcode.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-03-26.00:18:33.808>
    created_at = <Date 2013-03-22.01:03:52.331>
    labels = ['type-feature']
    title = 'Dead code should be removed'
    updated_at = <Date 2013-03-26.00:18:33.807>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2013-03-26.00:18:33.807>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-03-26.00:18:33.808>
    closer = 'vstinner'
    components = []
    creation = <Date 2013-03-22.01:03:52.331>
    creator = 'vstinner'
    dependencies = []
    files = ['29541']
    hgrepos = []
    issue_num = 17516
    keywords = ['patch']
    message_count = 11.0
    messages = ['184934', '184935', '184937', '184938', '184943', '185148', '185239', '185240', '185251', '185254', '185255']
    nosy_count = 7.0
    nosy_names = ['georg.brandl', 'rhettinger', 'vstinner', 'ezio.melotti', 'r.david.murray', 'tshepang', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17516'
    versions = ['Python 3.4']

    @vstinner
    Copy link
    Member Author

    Using my astoptimizer project and the hook proposed in the issue bpo-17515, I found the following dead code.
    https://bitbucket.org/haypo/astoptimizer

    Some code is not dead or unreachable in the general case, but only on a specific platform. I ran astoptimizer with all optimizations enabled, so os.name is replaced with "posix" for example. So "return os.name" is seen as dead code in the following function:

    def _get_default_scheme():
        if os.name == 'posix':
            # the default scheme for posix is posix_prefix
            return 'posix_prefix'
        return os.name

    Unreachable
    ===========

    Lib/_pyio.py:2057: Assign(targets=[Name(id='initial_value', ctx=Store())], value=Call(func=Name(id='str', ctx=Load()), ...
    Lib/sre_compile.py:68: Expr(value=Call(func=Name(id='emit', ctx=Load()), args=[Subscript(value=Name(id='OPCODES', ctx=Load(...
    Lib/sre_compile.py:69: Assign(targets=[Name(id='skip', ctx=Store())], value=Call(func=Name(id='_len', ctx=Load()), args=[Na...
    Lib/sre_compile.py:69: Expr(value=Call(func=Name(id='emit', ctx=Load()), args=[Num(n=0)], keywords=[], starargs=None, kwarg...
    Lib/sre_compile.py:70: Expr(value=Call(func=Name(id='emit', ctx=Load()), args=[Subscript(value=Name(id='av', ctx=Load()), s...
    Lib/sre_compile.py:71: Expr(value=Call(func=Name(id='emit', ctx=Load()), args=[Subscript(value=Name(id='av', ctx=Load()), s...
    Lib/sre_compile.py:72: Expr(value=Call(func=Name(id='_compile', ctx=Load()), args=[Name(id='code', ctx=Load()), Subscript(v...
    Lib/sre_compile.py:73: Expr(value=Call(func=Name(id='emit', ctx=Load()), args=[Subscript(value=Name(id='OPCODES', ctx=Load(...
    Lib/sre_compile.py:74: Assign(targets=[Subscript(value=Name(id='code', ctx=Load()), slice=Index(value=Name(id='skip', ctx=L...
    Lib/sysconfig.py:185: Return(value=Str(s='posix'))
    Lib/test/test_posix.py:682: FunctionDef(name='_create_and_do_getcwd', args=arguments(args=[arg(arg='dirname', annotation=None), ...
    Lib/test/test_posix.py:697: Expr(value=Call(func=Name(id='_create_and_do_getcwd', ctx=Load()), args=[Name(id='dirname', ctx=Load...
    Lib/test/test_unicode.py:1938: Expr(value=Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertRaises', ctx=Load()), ...

    Dead code
    =========

    Lib/datetime.py:2128: Expr(value=Str(s='\nSome time zone algebra. For a datetime x, let\n x.n = x stripped of its time...
    Lib/email/_header_value_parser.py:1337: Expr(value=Str(s="Parse strings according to RFC822/2047/2822/5322 rules.\n\nThis is a stateless par...
    Lib/pickletools.py:148: Expr(value=Str(s='\n"A pickle" is a program for a virtual pickle machine (PM, but more accurately\nc...
    Tools/scripts/reindent.py:56: Expr(value=Str(s='A specified newline to be used in the output (set by --newline option)'))

    Almost Dead code
    ================

    Lib/sqlite3/test/dump.py:44: Expr(value=ListComp(elt=Call(func=Attribute(value=Attribute(value=Name(id='self', ctx=Load()), attr=...
    Lib/sqlite3/test/dump.py:49: Expr(value=ListComp(elt=Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertEqual', c...
    Lib/test/test_grammar.py:369: Expr(value=Num(n=1))
    Lib/test/test_grammar.py:370: Expr(value=Tuple(elts=[Num(n=1), Num(n=2), Num(n=3)], ctx=Load()))
    Lib/test/test_grammar.py:474: Expr(value=Tuple(elts=[Num(n=1), Yield(value=Num(n=1))], ctx=Load()))
    Lib/test/test_grammar.py:475: Expr(value=Tuple(elts=[Num(n=1), YieldFrom(value=Tuple(elts=[], ctx=Load()))], ctx=Load()))
    Lib/test/test_optparse.py:731: Expr(value=Tuple(elts=[Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertParseOK', ...
    Lib/test/test_optparse.py:741: Expr(value=Tuple(elts=[Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertParseOK', ...
    Lib/test/test_optparse.py:746: Expr(value=Tuple(elts=[Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertParseOK', ...
    Lib/test/test_optparse.py:749: Expr(value=Tuple(elts=[Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertParseOK', ...
    Lib/test/test_optparse.py:754: Expr(value=Tuple(elts=[Call(func=Attribute(value=Name(id='self', ctx=Load()), attr='assertParseOK', ...
    Lib/test/test_peepholer.py:116: Expr(value=Tuple(elts=[UnaryOp(op=Invert(), operand=List(elts=[Num(n=0), Num(n=1), Num(n=2), Num(n=3...
    Lib/test/test_peepholer.py:50: Expr(value=NameConstant(value=None))
    Lib/test/test_peepholer.py:51: Expr(value=NameConstant(value=None))
    Lib/test/test_peepholer.py:54: Expr(value=NameConstant(value=True))
    Lib/test/test_peepholer.py:57: Expr(value=NameConstant(value=False))
    Lib/test/test_posix.py:1046: Expr(value=Attribute(value=Name(id='posix', ctx=Load()), attr='RTLD_LAZY', ctx=Load()))
    Lib/test/test_posix.py:1047: Expr(value=Attribute(value=Name(id='posix', ctx=Load()), attr='RTLD_NOW', ctx=Load()))
    Lib/test/test_posix.py:1048: Expr(value=Attribute(value=Name(id='posix', ctx=Load()), attr='RTLD_GLOBAL', ctx=Load()))
    Lib/test/test_posix.py:1049: Expr(value=Attribute(value=Name(id='posix', ctx=Load()), attr='RTLD_LOCAL', ctx=Load()))
    Lib/test/test_pow.py:102: Expr(value=BinOp(left=NameConstant(value=None), op=Pow(), right=Call(func=Name(id='TestRpow', ctx=Lo...
    Lib/test/test_raise.py:320: Expr(value=Name(id='xyzzy', ctx=Load()))
    Lib/test/test_richcmp.py:218: Expr(value=UnaryOp(op=Not(), operand=Name(id='bad', ctx=Load())))
    Lib/test/test_scope.py:578: Expr(value=ListComp(elt=Name(id='bad', ctx=Load()), generators=[comprehension(target=Name(id='s', ct...
    Lib/test/test_scope.py:590: Expr(value=Name(id='x', ctx=Load()))
    Lib/test/test_set.py:613: Expr(value=Compare(left=Name(id='myset', ctx=Load()), ops=[Lt()], comparators=[Name(id='myobj', ctx=...
    Lib/test/test_set.py:617: Expr(value=Compare(left=Name(id='myset', ctx=Load()), ops=[Gt()], comparators=[Name(id='myobj', ctx=...
    Lib/test/test_set.py:621: Expr(value=Compare(left=Name(id='myset', ctx=Load()), ops=[LtE()], comparators=[Name(id='myobj', ctx...
    Lib/test/test_set.py:625: Expr(value=Compare(left=Name(id='myset', ctx=Load()), ops=[GtE()], comparators=[Name(id='myobj', ctx...
    Lib/test/test_socket.py:1316: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='AF_CAN', ctx=Load()))
    Lib/test/test_socket.py:1317: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='PF_CAN', ctx=Load()))
    Lib/test/test_socket.py:1318: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_RAW', ctx=Load()))
    Lib/test/test_socket.py:1323: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM', ctx=Load()))
    Lib/test/test_socket.py:1326: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_TX_SETUP', ctx=Load()))
    Lib/test/test_socket.py:1327: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_TX_DELETE', ctx=Load()))
    Lib/test/test_socket.py:1328: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_TX_READ', ctx=Load()))
    Lib/test/test_socket.py:1329: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_TX_SEND', ctx=Load()))
    Lib/test/test_socket.py:1330: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_RX_SETUP', ctx=Load()))
    Lib/test/test_socket.py:1331: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_RX_DELETE', ctx=Load()))
    Lib/test/test_socket.py:1332: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_RX_READ', ctx=Load()))
    Lib/test/test_socket.py:1333: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_TX_STATUS', ctx=Load()))
    Lib/test/test_socket.py:1334: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_TX_EXPIRED', ctx=Load()))
    Lib/test/test_socket.py:1335: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_RX_STATUS', ctx=Load()))
    Lib/test/test_socket.py:1336: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_RX_TIMEOUT', ctx=Load()))
    Lib/test/test_socket.py:1337: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='CAN_BCM_RX_CHANGED', ctx=Load()))
    Lib/test/test_socket.py:1476: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='AF_RDS', ctx=Load()))
    Lib/test/test_socket.py:1477: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='PF_RDS', ctx=Load()))
    Lib/test/test_socket.py:727: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='AF_INET', ctx=Load()))
    Lib/test/test_socket.py:728: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SOCK_STREAM', ctx=Load()))
    Lib/test/test_socket.py:729: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SOCK_DGRAM', ctx=Load()))
    Lib/test/test_socket.py:730: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SOCK_RAW', ctx=Load()))
    Lib/test/test_socket.py:731: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SOCK_RDM', ctx=Load()))
    Lib/test/test_socket.py:732: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SOCK_SEQPACKET', ctx=Load()))
    Lib/test/test_socket.py:733: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SOL_SOCKET', ctx=Load()))
    Lib/test/test_socket.py:734: Expr(value=Attribute(value=Name(id='socket', ctx=Load()), attr='SO_REUSEADDR', ctx=Load()))
    Lib/test/test_ssl.py:105: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='PROTOCOL_SSLv23', ctx=Load()))
    Lib/test/test_ssl.py:106: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='PROTOCOL_SSLv3', ctx=Load()))
    Lib/test/test_ssl.py:107: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='PROTOCOL_TLSv1', ctx=Load()))
    Lib/test/test_ssl.py:108: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='CERT_NONE', ctx=Load()))
    Lib/test/test_ssl.py:109: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='CERT_OPTIONAL', ctx=Load()))
    Lib/test/test_ssl.py:110: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='CERT_REQUIRED', ctx=Load()))
    Lib/test/test_ssl.py:111: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='OP_CIPHER_SERVER_PREFERENCE', ctx=Load(...
    Lib/test/test_ssl.py:112: Expr(value=Attribute(value=Name(id='ssl', ctx=Load()), attr='OP_SINGLE_DH_USE', ctx=Load()))
    Lib/test/test_string.py:10: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='ascii_uppercase', ctx=Load()))
    Lib/test/test_string.py:11: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='ascii_letters', ctx=Load()))
    Lib/test/test_string.py:12: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='digits', ctx=Load()))
    Lib/test/test_string.py:13: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='hexdigits', ctx=Load()))
    Lib/test/test_string.py:14: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='octdigits', ctx=Load()))
    Lib/test/test_string.py:15: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='punctuation', ctx=Load()))
    Lib/test/test_string.py:16: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='printable', ctx=Load()))
    Lib/test/test_string.py:8: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='whitespace', ctx=Load()))
    Lib/test/test_string.py:9: Expr(value=Attribute(value=Name(id='string', ctx=Load()), attr='ascii_lowercase', ctx=Load()))
    Lib/test/test_super.py:79: Expr(value=Name(id='self', ctx=Load()))
    Lib/test/test_sys_settrace.py:243: Expr(value=GeneratorExp(elt=Name(id='o', ctx=Load()), generators=[comprehension(target=Name(id='o', ...
    Lib/test/test_time.py:25: Expr(value=Attribute(value=Name(id='time', ctx=Load()), attr='altzone', ctx=Load()))
    Lib/test/test_time.py:26: Expr(value=Attribute(value=Name(id='time', ctx=Load()), attr='daylight', ctx=Load()))
    Lib/test/test_time.py:27: Expr(value=Attribute(value=Name(id='time', ctx=Load()), attr='timezone', ctx=Load()))
    Lib/test/test_time.py:28: Expr(value=Attribute(value=Name(id='time', ctx=Load()), attr='tzname', ctx=Load()))
    Lib/test/test_weakref.py:129: Expr(value=Attribute(value=Name(id='proxy', ctx=Load()), attr='bar', ctx=Load()))
    Lib/test/test_weakref.py:461: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='J', ctx=Load()))
    Lib/test/test_weakref.py:513: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='J', ctx=Load()))
    Lib/test/test_weakref.py:533: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='me', ctx=Load()))
    Lib/test/test_weakref.py:534: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='c1', ctx=Load()))
    Lib/test/test_weakref.py:535: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='wr', ctx=Load()))
    Lib/test/test_weakref.py:558: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='me', ctx=Load()))
    Lib/test/test_weakref.py:559: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='c1', ctx=Load()))
    Lib/test/test_weakref.py:560: Expr(value=Attribute(value=Name(id='self', ctx=Load()), attr='wr', ctx=Load()))
    Lib/test/test_xml_etree.py:168: Expr(value=Subscript(value=Name(id='string', ctx=Load()), slice=Slice(lower=None, upper=Num(n=0), st...

    @vstinner
    Copy link
    Member Author

    "Almost Dead code" is code generally considered as useless, but many Python unit tests use such code. Dummy example:

    try:
    obj.attr
    except AttributeError:
    pass

    I don't think that we should touch this code, it's just for information.

    @bitdancer
    Copy link
    Member

    Your "dead code" looks like multiline comments (at least it is in the email case). I thought those were optimized away when the pyc is written.

    What is "almost dead code"? If it is the stuff that is only applicable to a given platform, then presumably it is needed for that platform and why it it worth mentioning?

    @vstinner
    Copy link
    Member Author

    Here is a concrete patch.

    Your "dead code" looks like multiline comments (at least it is
    in the email case). I thought those were optimized away when
    the pyc is written.

    Oh, it looks like you are right: useless strings are already removed during compilation. But it looks a little bit surprising to me to use a multiline string for a comment. I prefer classic # comments.

    What is "almost dead code"? If it is the stuff that is only
    applicable to a given platform, then presumably it is needed
    for that platform and why it it worth mentioning?

    It's just for information, just ignore these "warnings" :-)

    @rhettinger
    Copy link
    Contributor

    The code in test_peepholer should be removed. That code isn't run directly. Instead, it is tested for a correct disassembly.

    @ezio-melotti ezio-melotti added the type-feature A feature request or enhancement label Mar 24, 2013
    @bitdancer
    Copy link
    Member

    Oh, it looks like you are right: useless strings are already removed
    during compilation. But it looks a little bit surprising to me to use a
    multiline string for a comment. I prefer classic # comments.

    I was surprised by this as well. I think the comment in _header_value_parser was originally a docstring and got moved around during editing. It should probably get changed to a block comment.

    I could have sworn that when I learned about the optimization it was in the context of documentation that specifically said the optimization was done so that triple quoted strings could be used as multi-line comments, but I cannot find it through google or guessing where I might have seen it in our docs, so perhaps I was imagining things.

    @vstinner
    Copy link
    Member Author

    For the dead code in test_posix.py, see issue bpo-9246.

    @birkenfeld
    Copy link
    Member

    The instance in reindent.py uses the "docstring for attributes" feature that some documentation tools supports, which is a unused string *after* the attribute.

    When you turn that into a comment, put the comment *before* the attribute.

    @vstinner
    Copy link
    Member Author

    test_create_autospec_unbound_methods() of Lib/unittest/test/testmock/testhelpers.py contains dead code because of a known issue. I created issue bpo-17548 to track this function.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 26, 2013

    New changeset 313bcff51900 by Victor Stinner in branch 'default':
    Issue bpo-17516: use comment syntax for comments, instead of multiline string
    http://hg.python.org/cpython/rev/313bcff51900

    New changeset 33bdd0a985b9 by Victor Stinner in branch 'default':
    Issue bpo-17516: do not create useless tuple: remove dummy commas in tests
    http://hg.python.org/cpython/rev/33bdd0a985b9

    New changeset 9dba3bc7c2a6 by Victor Stinner in branch 'default':
    Issue bpo-17516: remove dead code
    http://hg.python.org/cpython/rev/9dba3bc7c2a6

    @vstinner
    Copy link
    Member Author

    Thanks for the different reviews.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants