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

add program passed as string to dis module. #88571

Closed
CCLDArjun mannequin opened this issue Jun 13, 2021 · 12 comments
Closed

add program passed as string to dis module. #88571

CCLDArjun mannequin opened this issue Jun 13, 2021 · 12 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@CCLDArjun
Copy link
Mannequin

CCLDArjun mannequin commented Jun 13, 2021

BPO 44405
Nosy @terryjreedy, @ncoghlan, @stevendaprano, @serhiy-storchaka, @1st1, @eamanu, @CCLDArjun
PRs
  • bpo-44405: make dis cli "official"  #26714
  • Files
  • dis.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 = None
    created_at = <Date 2021-06-13.00:44:23.977>
    labels = ['type-feature', 'library', '3.11']
    title = 'add program passed as string to dis module.'
    updated_at = <Date 2022-03-18.15:17:19.473>
    user = 'https://github.com/CCLDArjun'

    bugs.python.org fields:

    activity = <Date 2022-03-18.15:17:19.473>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2021-06-13.00:44:23.977>
    creator = 'CCLDArjun'
    dependencies = []
    files = ['50107']
    hgrepos = []
    issue_num = 44405
    keywords = ['patch']
    message_count = 9.0
    messages = ['395720', '395722', '395723', '395724', '395725', '395727', '395731', '398046', '415505']
    nosy_count = 7.0
    nosy_names = ['terry.reedy', 'ncoghlan', 'steven.daprano', 'serhiy.storchaka', 'yselivanov', 'eamanu', 'CCLDArjun']
    pr_nums = ['26714']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue44405'
    versions = ['Python 3.11']

    @CCLDArjun
    Copy link
    Mannequin Author

    CCLDArjun mannequin commented Jun 13, 2021

    python dis module should have a program passed in as string.

    Currently, you can do this:
    $ echo "x = 6" | python -m dis /dev/stdin
    1 0 LOAD_CONST 0 (6)
    2 STORE_NAME 0 (x)
    4 LOAD_CONST 1 (None)
    6 RETURN_VALUE

    would be convenient like this:
    $ ./python.exe -m dis -c "x=6"
    1 0 LOAD_CONST 0 (6)
    2 STORE_NAME 0 (x)
    4 LOAD_CONST 1 (None)
    6 RETURN_VALUE

    these changes seem to be working.

    @CCLDArjun CCLDArjun mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jun 13, 2021
    @terryjreedy
    Copy link
    Member

    I checked https://docs.python.org/3/library/dis.html and there is no mention of dis having a command-line interface. This suggests that _test is likely present only for testing the module, not for using it. This was common a couple of decades ago before we had unittests. If _test were considered obsolete, it could be removed.

    By suggesting that the command line interface by upgraded, I suspect that you are implicitly suggesting that it be considered 'official'. If so, that '_test' should become '_main' (where the underscore leaves it out of __all__), and the CLI documented as in other modules with a CLI. (I am not up on the details and policy around this issue.)

    It may be appropriate to start a discussion of python-ideas.

    @terryjreedy
    Copy link
    Member

    What is unusual, I think, for a CLI test function is that it requires a passed in argument. As a sanity-check test, 'python -m dis' could (should) disassemble itself. Perhaps there once were some true tests that were moved to unittests. (I started IDLE unittests by doing this with module test functions.) If you know git, you could check the history and checkin messages.

    @CCLDArjun
    Copy link
    Mannequin Author

    CCLDArjun mannequin commented Jun 13, 2021

    Huh, that's actually weird that the "exisisting command line interface" (quotes because it was added as a test) isn't official. I've found it to be convinient as a newcomer to the cpython codebase.

    What is unusual, I think, for a CLI test function is that it requires a passed in argument

    git blame shows the commit hash is 0956689, which is for bpo: https://bugs.python.org/issue18538 "python -m dis now uses argparse". The Misc/HISTORY file references the change in section "What's New in Python 3.4.0 Alpha 3?"

    Regardless of the cli being official, personally I think, it's a good feature to add. But also, maybe I should start a discussion in python-ideas about making it official?

    @CCLDArjun
    Copy link
    Mannequin Author

    CCLDArjun mannequin commented Jun 13, 2021

    If _test were considered obsolete, it could be removed.

    Yup, originally it was added 2 decades ago (in 2000) originally as a test: 1fdae12

    @CCLDArjun
    Copy link
    Mannequin Author

    CCLDArjun mannequin commented Jun 13, 2021

    If _test were considered obsolete, it could be removed.

    removing _test: CCLDArjun@8b3b8cc does not break dis tests.

    @stevendaprano stevendaprano added 3.11 only security fixes labels Jun 13, 2021
    @serhiy-storchaka
    Copy link
    Member

    I often use the command-line interface of dis, ast, and sometimes tokenize modules. They are mature enough and not only for self-testing. If they are not documented, we need to document them.

    Since they read from stdin by default (no need to specify /dev/stdin or CON explicitly) I never needed the -c option. It would not be particularly useful in any case because Python code is usually multi-line.

    BTW sometimes I want to implement GUI for these modules in IDLE.

    @ncoghlan
    Copy link
    Contributor

    I suspect the only reason the dis CLI isn't documented is because it's been around for so long (22 years!) that nobody previously noticed it was missing: 1fdae12

    Folks then either didn't know about it, or already knew how to use it.

    Even the migration to use argparse was a code cleanup from a user that happened to be reading the module source code and noticed that it didn't already do so: https://bugs.python.org/issue18538

    I can see value in supporting -c and -m options to dis to align with the main interpreter CLI.

    @serhiy-storchaka
    Copy link
    Member

    Neither of tokenize, ast or symtable modules support passing the source string as argument. If add this feature in the dis module, we will need to add it in all other modules with similar CLI. I do not think it is practical. You always can pass the source string via stdin. And you do not even need to pass /dev/stdin as argument, stdin is the default.

    I usually use it with rlwrap. It allows to use some editing and history.

    rlwrap ./python -m dis
    

    I propose to close this issue and open a new issue for documenting the CLI of the dis module.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @hugovk
    Copy link
    Member

    hugovk commented Sep 3, 2023

    New issue for documenting the CLI: #108826

    And PR: #108827

    @serhiy-storchaka
    Copy link
    Member

    I suggest to close this issue.

    @serhiy-storchaka serhiy-storchaka added the pending The issue will be closed if no feedback is provided label Oct 9, 2024
    @picnixz
    Copy link
    Member

    picnixz commented Dec 2, 2024

    We documented the dis module, but the question on whether we use dis with a -c argument remains. I find it actually helpful to do something like python -m dis -c "return 1" just to check some simple constructions and the output but apart from that, programs would be multiline and wouldn't benefit from this.

    If we want to support inline arguments for dis, we should also support it for tokenize, ast and symtable in one go and this should warrant a DPO thread: https://discuss.python.org/c/ideas/6.

    Closing as not planned for now.

    @picnixz picnixz closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2024
    @picnixz picnixz removed 3.11 only security fixes pending The issue will be closed if no feedback is provided labels Dec 2, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants