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

incorrect LOAD/STORE_GLOBAL generation #43467

Closed
Yhg1s opened this issue Jun 6, 2006 · 6 comments
Closed

incorrect LOAD/STORE_GLOBAL generation #43467

Yhg1s opened this issue Jun 6, 2006 · 6 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker

Comments

@Yhg1s
Copy link
Member

Yhg1s commented Jun 6, 2006

BPO 1501934
Nosy @Yhg1s, @nascheme
Files
  • augassign_fix.txt: minimal fix
  • test_ast_fix.txt
  • 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 = 'https://github.com/nascheme'
    closed_at = <Date 2006-07-09.16:17:21.000>
    created_at = <Date 2006-06-06.23:57:54.000>
    labels = ['interpreter-core', 'release-blocker']
    title = 'incorrect LOAD/STORE_GLOBAL generation'
    updated_at = <Date 2006-07-09.16:17:21.000>
    user = 'https://github.com/Yhg1s'

    bugs.python.org fields:

    activity = <Date 2006-07-09.16:17:21.000>
    actor = 'nascheme'
    assignee = 'nascheme'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2006-06-06.23:57:54.000>
    creator = 'twouters'
    dependencies = []
    files = ['2023', '2024']
    hgrepos = []
    issue_num = 1501934
    keywords = []
    message_count = 6.0
    messages = ['28729', '28730', '28731', '28732', '28733', '28734']
    nosy_count = 2.0
    nosy_names = ['twouters', 'nascheme']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1501934'
    versions = ['Python 2.5']

    @Yhg1s
    Copy link
    Member Author

    Yhg1s commented Jun 6, 2006

    Python 2.5 compiles the following piece of code
    differently than Python 2.4:

    g = 1
    def f():
        g += 1

    In Python 2.4, this raises an UnboundLocalError. In
    current svn trunk, it will increment the global g by 1.
    (dis.dis shows that it actually compiles into
    LOAD/STORE_GLOBAL opcodes.) It seems the compiler
    doesn't treat augmented assignment as assignment for
    the purpose of determining locals, as this still fails
    correctly:

    g = 1
    def f():
        g += 1
        g = 5

    I can't find where this optimization happens nowadays,
    but it feels like a short fix.

    @Yhg1s Yhg1s closed this as completed Jun 6, 2006
    @Yhg1s Yhg1s added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jun 6, 2006
    @Yhg1s Yhg1s added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jun 6, 2006
    @Yhg1s
    Copy link
    Member Author

    Yhg1s commented Jun 19, 2006

    Logged In: YES
    user_id=34209

    Possibly related is the discovery of free variables (used
    when forming closures) and optimized-out codeblocks:

    >>> def foo(x):
    ...     def bar():
    ...         if 0:
    ...             print x
    ...     return bar
    
    In 2.4, there is no closure:
    >>> foo.func_code.co_cellvars
    ()
    >>> foo(5).func_closure
    >>>
    
    In 2.5, there is:
    >>> foo.func_code.co_cellvars
    ('x',)
    >>> foo(5).func_closure
    (<cell at 0x2b9abf6d7e30: int object at 0x6b6580>,)

    (I don't think it's unreasonable to declare the old
    behaviour bugged, though :-)

    @nascheme
    Copy link
    Member

    Logged In: YES
    user_id=35752

    Here are some notes in case I wear out before finding a fix.
    analyze_name() gets to the SET_SCOPE(dict, name,
    GLOBAL_IMPLICIT) line because the name does not have the
    DEF_LOCAL flag set as it should. It does not have DEF_LOCAL
    because Name.ctx for 'g' is Load. I believe there should be
    a set_context() call in ast_for_expr_stmt, as flagged as
    TODO by Jeremy. Maybe set_context(..., Store, ...) would
    work or maybe things need to be changed to allow ctx to have
    AugAssign as a value.

    @nascheme
    Copy link
    Member

    Logged In: YES
    user_id=35752

    I've got a simple fix that seems to work. I feel this part
    of the compiler could use some more serious cleanups but
    probably not for 2.5. Note that test_ast fails after
    applying my patch. I haven't had time to look into that yet
    but I think it's shallow.

    @nascheme
    Copy link
    Member

    Logged In: YES
    user_id=35752

    Adding a patch to "fix" test_ast.py. I have no idea what
    the test is trying to verify. It looks like it was written
    by martin.v.loewis so maybe he can comment.

    @nascheme
    Copy link
    Member

    nascheme commented Jul 9, 2006

    Logged In: YES
    user_id=35752

    Checked in as SVN rev 50493.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants