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

Generator return value ignored in lambda function #67381

Closed
Rosuav opened this issue Jan 8, 2015 · 8 comments
Closed

Generator return value ignored in lambda function #67381

Rosuav opened this issue Jan 8, 2015 · 8 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@Rosuav
Copy link
Contributor

Rosuav commented Jan 8, 2015

BPO 23192
Nosy @gvanrossum, @vstinner, @benjaminp, @ezio-melotti, @Rosuav, @serhiy-storchaka
Files
  • 0001-lambda-generators-don-t-throw-away-stack-top.patch: Fix lambda generator throwing away sent value
  • 0002-lambda-generator-fix-try-to-add-a-dis-test.patch: Failed test tentative
  • 0001-Add-tests-for-issue-23192.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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2015-03-11.16:25:22.951>
    created_at = <Date 2015-01-08.14:42:05.907>
    labels = ['interpreter-core', 'type-bug']
    title = 'Generator return value ignored in lambda function'
    updated_at = <Date 2015-03-11.16:25:22.950>
    user = 'https://github.com/Rosuav'

    bugs.python.org fields:

    activity = <Date 2015-03-11.16:25:22.950>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-03-11.16:25:22.951>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2015-01-08.14:42:05.907>
    creator = 'Rosuav'
    dependencies = []
    files = ['38298', '38299', '38440']
    hgrepos = []
    issue_num = 23192
    keywords = ['patch']
    message_count = 8.0
    messages = ['233662', '233668', '233673', '237041', '237804', '237861', '237883', '237884']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'vstinner', 'benjamin.peterson', 'ezio.melotti', 'python-dev', 'Rosuav', 'serhiy.storchaka', 'bru']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue23192'
    versions = ['Python 3.4', 'Python 3.5']

    @Rosuav
    Copy link
    Contributor Author

    Rosuav commented Jan 8, 2015

    As yield is an expression, it's legal in a lambda function, which then
    means you have a generator function. But it's not quite the same as
    the equivalent function made with def:

    $ python3
    Python 3.5.0a0 (default:1c51f1650c42+, Dec 29 2014, 02:29:06)
    [GCC 4.7.2] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> f=lambda: (yield 5)
    >>> x=f()
    >>> next(x)
    5
    >>> x.send(123)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    StopIteration
    >>> def f(): return (yield 5)
    ...
    >>> x=f()
    >>> next(x)
    5
    >>> x.send(123)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    StopIteration: 123
    >>> x = (lambda: print((yield 1)) or 2)()
    >>> next(x)
    1
    >>> x.send(3)
    3
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    StopIteration

    The last example demonstrates that send() is working, but the return value is not getting propagated. Disassembly shows this:

    >>> dis.dis(lambda: (yield 5))
      1           0 LOAD_CONST               1 (5)
                  3 YIELD_VALUE
                  4 POP_TOP
                  5 LOAD_CONST               0 (None)
                  8 RETURN_VALUE
    >>> def f(): return (yield 5)
    ... 
    >>> dis.dis(f)
      1           0 LOAD_CONST               1 (5)
                  3 YIELD_VALUE
                  4 RETURN_VALUE

    I'm sure this is a bug that will affect very approximately zero people, but it's still a peculiar inconsistency!

    Verified with 3.5 and 3.4.

    @Rosuav Rosuav added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jan 8, 2015
    @gvanrossum
    Copy link
    Member

    Hm, looks like nobody bothered to update the lambda code generation to use the value from yield. I almost feel like there is some unnecessary check "if we are in a lambda" in the code generation for yield. Have you looked through the code generation yet?

    @Rosuav
    Copy link
    Contributor Author

    Rosuav commented Jan 8, 2015

    I'm not sure what to look for in the code generation. In compile.c lines 3456 and following, there's a LOAD_CONST None coming through, in the else branch of "if (e->v.Yield.value)", but nothing talking about lambda functions. There are constants COMPILER_SCOPE_LAMBDA and COMPILER_SCOPE_FUNCTION, but the only place where they're used is compiler_set_qualname() and I can't see anything obvious there. Hopefully someone more familiar with the code internals will be able to figure this out!

    @bru
    Copy link
    Mannequin

    bru mannequin commented Mar 2, 2015

    Here are the operations being emitted (line, macro used and eventual argument):

    >>> f = lambda: (yield 5)
    3487: ADDOP_O LOAD_CONST e->v.Num.n
    3472: ADDOP YIELD_VALUE
    1907: ADDOP_IN_SCOPE POP_TOP
    4349: ADDOP_O LOAD_CONST Py_None
    4350: ADDOP RETURN_VALUE
    1457: ADDOP_O LOAD_CONST (PyObject*)co
    1458: ADDOP_O LOAD_CONST qualname
    1459: ADDOP_I MAKE_FUNCTION args
    4349: ADDOP_O LOAD_CONST Py_None
    4350: ADDOP RETURN_VALUE
    >>> def g(): return (yield 5)
    ... 
    3487: ADDOP_O LOAD_CONST e->v.Num.n
    3472: ADDOP YIELD_VALUE
    2533: ADDOP RETURN_VALUE
    1457: ADDOP_O LOAD_CONST (PyObject*)co
    1458: ADDOP_O LOAD_CONST qualname
    1459: ADDOP_I MAKE_FUNCTION args
    4349: ADDOP_O LOAD_CONST Py_None
    4350: ADDOP RETURN_VALUE

    So there's an extra POP_TOP + LOAD_CONST Py_NONE for the lambda version that throws away the "123" (in the exemple).

    The attached patch (0001-...) fixes it. However please note that I'm not knowledgable about that part of the code and devised the patch empirically.
    Moreover a test should probably added but I did not know where (test_dis? tried and failed... see patch 0002-...).

    @serhiy-storchaka
    Copy link
    Member

    Could you please add a test based on Chris's example? And it would be good to add a test for a lambda with "yield from".

    @serhiy-storchaka serhiy-storchaka self-assigned this Mar 10, 2015
    @bru
    Copy link
    Mannequin

    bru mannequin commented Mar 11, 2015

    Here is a working test, testing yield by lambda & function as well as lambda and function yielding from those.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 11, 2015

    New changeset 2b4a04c3681b by Serhiy Storchaka in branch '3.4':
    Issue bpo-23192: Fixed generator lambdas. Patch by Bruno Cauet.
    https://hg.python.org/cpython/rev/2b4a04c3681b

    New changeset a3b889e9d3f3 by Serhiy Storchaka in branch 'default':
    Issue bpo-23192: Fixed generator lambdas. Patch by Bruno Cauet.
    https://hg.python.org/cpython/rev/a3b889e9d3f3

    @serhiy-storchaka
    Copy link
    Member

    Committed with non-dis test. Thank you for your contribution Bruno.

    @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) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants