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

inspect.getsourcelines finds wrong lines when lambda used argument to decorator #65416

Closed
ballingt mannequin opened this issue Apr 14, 2014 · 22 comments
Closed

inspect.getsourcelines finds wrong lines when lambda used argument to decorator #65416

ballingt mannequin opened this issue Apr 14, 2014 · 22 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ballingt
Copy link
Mannequin

ballingt mannequin commented Apr 14, 2014

BPO 21217
Nosy @terryjreedy, @ncoghlan, @pitrou, @meadori, @PCManticore, @1st1
Files
  • issue21217.patch
  • issue21217-v2.patch
  • issue21217-v3.patch
  • issue21217-v4.patch
  • issue21217-v5.patch
  • issue21217-v6.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 2015-04-14.22:42:37.704>
    created_at = <Date 2014-04-14.17:31:00.288>
    labels = ['type-bug', 'library']
    title = 'inspect.getsourcelines finds wrong lines when lambda used argument to decorator'
    updated_at = <Date 2015-07-24.04:00:15.970>
    user = 'https://bugs.python.org/ballingt'

    bugs.python.org fields:

    activity = <Date 2015-07-24.04:00:15.970>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-04-14.22:42:37.704>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2014-04-14.17:31:00.288>
    creator = 'ballingt'
    dependencies = []
    files = ['34857', '34882', '34885', '38959', '38965', '38970']
    hgrepos = []
    issue_num = 21217
    keywords = ['patch']
    message_count = 22.0
    messages = ['216127', '216128', '216245', '216344', '216345', '216351', '216352', '216357', '240738', '240749', '240787', '240791', '241050', '241051', '241077', '241079', '245868', '245872', '245876', '245915', '247202', '247245']
    nosy_count = 9.0
    nosy_names = ['terry.reedy', 'ncoghlan', 'pitrou', 'meador.inge', 'Claudiu.Popa', 'python-dev', 'yselivanov', 'akaptur', 'ballingt']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue21217'
    versions = ['Python 3.5']

    @ballingt
    Copy link
    Mannequin Author

    ballingt mannequin commented Apr 14, 2014

    https://gist.github.com/thomasballinger/10666031

    """
    inspect.getsourcelines incorrectly guesses what lines correspond
    to the function foo

    see getblock in inspect.py
    once it finds a lambda, def or class it finishes it then stops
    so get getsourcelines returns only the first two noop decorator
    lines of bar, while normal behavior is to return all decorators
    as it does for foo
    """
    import inspect
    from pprint import pprint

    def noop(arg):
    def inner(func):
    return func
    return inner

    @noop(1)
    @noop(2)
    def foo():
    return 1

    @noop(1)
    @noop(lambda: None)
    @noop(1)
    def bar():
    return 1

    pprint(inspect.getsourcelines(foo))
    pprint(inspect.getsourcelines(bar))

    @ballingt ballingt mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Apr 14, 2014
    @ballingt
    Copy link
    Mannequin Author

    ballingt mannequin commented Apr 14, 2014

    "The code object's co_lnotab is how inspect should be getting the sourcelines of the code, instead of looking for the first codeblock."

    I'm looking at this now, thanks to Yhg1s for the above.

    @akaptur
    Copy link
    Mannequin

    akaptur mannequin commented Apr 14, 2014

    This patch adds tests demonstrating broken behavior inspect.getsource and inspect.getsourcelines of decorators containing lambda functions, and modifies inspect.getsourcelines to behave correctly.

    We use co_lnotab to extract line numbers on all objects with a code object. inspect.getsourcelines can also take a class, which cannot use co_lnotab as there is no associated code object.

    @ballingt and I paired on this patch.

    Some open questions about inspect.getsource not created or addressed by this patch:

    • Is this a bug that should be patched in previous versions as well?
    • the docs for say it can take a traceback. What is the correct behavior here? There aren't any tests at the moment. We suggest the line of code that caused the traceback, i.e. the line at tb.tb_lineno
    • We added tests of decorated classes. The source of decorated classes does not include the decorators, which is different than the usual behavior of decorated functions. What is the correct behavior here?
    • inspect.getblock and inspect.BlockFinder use the term "block" in a way that is inconsistent with its typical use in the interpreter (that is, in ceval.c). Should this be renamed? If so, to what? ("chunk"?)

    @akaptur
    Copy link
    Mannequin

    akaptur mannequin commented Apr 15, 2014

    v2 of the patch incorporating the comments at http://bugs.python.org/review/21217/

    @1st1
    Copy link
    Member

    1st1 commented Apr 15, 2014

    Apart from one nit, the patch is looking good.

    Also, could you please sign the contributor agreement, as described here: https://docs.python.org/devguide/coredev.html#sign-a-contributor-agreement

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Apr 15, 2014

    "- We added tests of decorated classes. The source of decorated classes does not include the decorators, which is different than the usual behavior of decorated functions. What is the correct behavior here?"

    There is an open issue for this, http://bugs.python.org/issue1764286. It has a patch which uses inspect.unwrap in order to unwrap the decorated functions.

    @1st1
    Copy link
    Member

    1st1 commented Apr 15, 2014

    Claudiu: I'll take a look at your patch, thanks!

    @akaptur
    Copy link
    Mannequin

    akaptur mannequin commented Apr 15, 2014

    v3 of patch, including misc/news update, docstring for function, and removing class decorator tests, since it sounds like those are better handled in http://bugs.python.org/issue1764286.

    @ballingt
    Copy link
    Mannequin Author

    ballingt mannequin commented Apr 13, 2015

    v4 of patch, with tests updated for changed lines in inspect fodder file

    @ballingt
    Copy link
    Mannequin Author

    ballingt mannequin commented Apr 13, 2015

    Patch reformatted to be non-git style, NEWS item removed

    @ballingt
    Copy link
    Mannequin Author

    ballingt mannequin commented Apr 13, 2015

    Use dis.findlinestarts() to find lines of function instead of grubbing with co_lnotab manually, making dis module dependency non-optional.

    @ncoghlan
    Copy link
    Contributor

    It sounds like at least a somewhat functional dis module is a pragmatic requirement for a Python implementation to support introspection, so +1 for reverting to the mandatory dependency on dis rather than duplicating its logic.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 14, 2015

    New changeset ac86e5b2d45b by Antoine Pitrou in branch 'default':
    Issue bpo-21217: inspect.getsourcelines() now tries to compute the start and
    https://hg.python.org/cpython/rev/ac86e5b2d45b

    @pitrou
    Copy link
    Member

    pitrou commented Apr 14, 2015

    I've committed the latest patch. Thank you, Thomas!

    @pitrou pitrou closed this as completed Apr 14, 2015
    @ballingt
    Copy link
    Mannequin Author

    ballingt mannequin commented Apr 15, 2015

    Thanks Antoine! Could you add Allison Kaptur to NEWS and ACKS? This was an update to her original patch, and we paired on the whole thing.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 15, 2015

    New changeset 582e8e71f635 by Benjamin Peterson in branch 'default':
    add Allison Kaptur (bpo-21217)
    https://hg.python.org/cpython/rev/582e8e71f635

    @terryjreedy
    Copy link
    Member

    I strongly suspect that ac86e5b2d45b is the cause of the regression reported in bpo-24485.

    def outer():
        def inner():
            inner1
    from inspect import getsource
    print(getsource(outer))

    omits the body of inner. Ditto if outer is a method. All is okay if outer is a method and the source of the class is requested.

    Could the authors, Allison and Thomas, take a look and try to fix _line_number_helper to pass the added test (and possibly incorporate it into findsource)? Since there is a new issue already, this one can be left closed and further work posted to bpo-24485.

    @1st1
    Copy link
    Member

    1st1 commented Jun 26, 2015

    Here's an update on bpo-24485 regression.

    Looks like getsource() is now using code objects instead of tokenizer to determine blocks first/last lines.

    The problem with this particular case is that "inner" function's code object is completely independent from "outer"'s.

    So, for an outer() function below:

    def outer():
        def inner():
            never_reached1
            never_reached2

    the code object contains the following opcodes:

    71 0 LOAD_CONST 1 (<code object inner ...>)
    3 LOAD_CONST 2 ('outer1.<locals>.inner')
    6 MAKE_FUNCTION 0
    9 STORE_FAST 0 (inner)
    12 LOAD_CONST 0 (None)
    15 RETURN_VALUE

    The correct solution is to use co_lnotab along with co_firstlineno to iterate through opcodes recursively accounting for MAKE_FUNCTION's code objects.

    *However*, I don't think we can solve this for classes, i.e.

    def outer_with_class():
       class Foo:
          b = 1
          a = 2

    there is no way (as far as I know) to get information about the Foo class start/end lineno.

    I think that the only way we can solve this is to revert the patch for this issue.

    @meadori
    Copy link
    Member

    meadori commented Jun 27, 2015

    I think that the only way we can solve this is to revert the patch for this issue.

    I agree with this. It seems like doing this analysis at the bytecode level is the
    wrong approach. Perhaps the syntactical analysis being used before should be beefed
    up to handle things like the lambda case.

    @meadori
    Copy link
    Member

    meadori commented Jun 28, 2015

    FYI, I posted a patch to handle this case and the regression
    noted in bpo-24485 on bpo-24485.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 23, 2015

    New changeset 4e42a62d5648 by Yury Selivanov in branch '3.5':
    Issue bpo-24485: Revert backwards compatibility breaking changes of bpo-21217.
    https://hg.python.org/cpython/rev/4e42a62d5648

    New changeset 98a2bbf2cce2 by Yury Selivanov in branch 'default':
    Merge 3.5 (issues bpo-21217, bpo-24485).
    https://hg.python.org/cpython/rev/98a2bbf2cce2

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 24, 2015

    New changeset 5400e21e92a7 by Meador Inge in branch '3.5':
    Issue bpo-24485: Function source inspection fails on closures.
    https://hg.python.org/cpython/rev/5400e21e92a7

    New changeset 0e7d64595223 by Meador Inge in branch 'default':
    Issue bpo-24485: Function source inspection fails on closures.
    https://hg.python.org/cpython/rev/0e7d64595223

    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants