Skip to content

bpo-33211: Change line number of decorated nodes back to def #6460

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

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ Other Language Changes
* Added support of ``\N{name}`` escapes in :mod:`regular expressions <re>`.
(Contributed by Jonathan Eunice and Serhiy Storchaka in :issue:`30688`.)

* Corrected the AST to give the actual line number of a decorated definition.
The first line number of the code object still points to the first
decorator.
(Contributed by Ethan Smith in :issue:`33211`.)


New Modules
===========
Expand Down
12 changes: 12 additions & 0 deletions Lib/test/inspect_fodder2.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,15 @@ def func136():
def func137():
never_reached1
never_reached2

#line 141
def func142():
@wrap
def inner():
pass

#line 147
def func148():
@wrap
class A:
pass
53 changes: 52 additions & 1 deletion Lib/test/test_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import dis
import os
import sys
import textwrap
import unittest
import weakref

Expand Down Expand Up @@ -43,12 +44,52 @@ def to_tuple(t):
"def f(**kwargs): pass",
# FunctionDef with all kind of args and docstring
"def f(a, b=1, c=None, d=[], e={}, *args, f=42, **kwargs): 'doc for f()'",
# Decorated FunctionDef
textwrap.dedent("""
@deco
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you use textwrap.dedent() it would clearer if add an indentation of the inner code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please increase the indentation of these fragments.

def f():
pass
"""),
# Multiple decorators on a FunctionDef
textwrap.dedent("""
@deco1
@deco2()
def f():
pass
"""),
# decorated closure
textwrap.dedent("""
def f():
@deco
def g():
pass
"""),
# ClassDef
"class C:pass",
# ClassDef with docstring
"class C: 'docstring for class C'",
# ClassDef, new style class
"class C(object): pass",
# Decorated ClassDef
textwrap.dedent("""
@foo
class A:
pass
"""),
# Multiple decorators on a ClassDef
textwrap.dedent("""
@foo1
@bar(4)
class A:
pass
"""),
# Decorated method
textwrap.dedent("""
class A:
@deco
def m(self):
pass
"""),
# Return
"def f():return 1",
# Delete
Expand Down Expand Up @@ -207,7 +248,11 @@ def _assertTrueorder(self, ast_node, parent_pos):
parent_pos = (ast_node.lineno, ast_node.col_offset)
for name in ast_node._fields:
value = getattr(ast_node, name)
if isinstance(value, list):
if name == 'decorator_list':
# Decorators come before a FunctionDef
for decorator in value:
self.assertTrue(decorator.lineno < ast_node.lineno)
elif isinstance(value, list):
for child in value:
self._assertTrueorder(child, parent_pos)
elif value is not None:
Expand Down Expand Up @@ -1133,9 +1178,15 @@ def main():
('Module', [('FunctionDef', (1, 0), 'f', ('arguments', [], ('arg', (1, 7), 'args', None), [], [], None, []), [('Pass', (1, 14))], [], None, None)], None),
('Module', [('FunctionDef', (1, 0), 'f', ('arguments', [], None, [], [], ('arg', (1, 8), 'kwargs', None), []), [('Pass', (1, 17))], [], None, None)], None),
('Module', [('FunctionDef', (1, 0), 'f', ('arguments', [('arg', (1, 6), 'a', None), ('arg', (1, 9), 'b', None), ('arg', (1, 14), 'c', None), ('arg', (1, 22), 'd', None), ('arg', (1, 28), 'e', None)], ('arg', (1, 35), 'args', None), [('arg', (1, 41), 'f', None)], [('Num', (1, 43), 42)], ('arg', (1, 49), 'kwargs', None), [('Num', (1, 11), 1), ('NameConstant', (1, 16), None), ('List', (1, 24), [], ('Load',)), ('Dict', (1, 30), [], [])]), [], [], None, 'doc for f()')], None),
('Module', [('FunctionDef', (3, 0), 'f', ('arguments', [], None, [], [], None, []), [('Pass', (4, 4))], [('Name', (2, 1), 'deco', ('Load',))], None, None)], None),
('Module', [('FunctionDef', (4, 0), 'f', ('arguments', [], None, [], [], None, []), [('Pass', (5, 4))], [('Name', (2, 1), 'deco1', ('Load',)), ('Call', (3, 0), ('Name', (3, 1), 'deco2', ('Load',)), [], [])], None, None)], None),
('Module', [('FunctionDef', (2, 0), 'f', ('arguments', [], None, [], [], None, []), [('FunctionDef', (4, 4), 'g', ('arguments', [], None, [], [], None, []), [('Pass', (5, 8))], [('Name', (3, 5), 'deco', ('Load',))], None, None)], [], None, None)], None),
('Module', [('ClassDef', (1, 0), 'C', [], [], [('Pass', (1, 8))], [], None)], None),
('Module', [('ClassDef', (1, 0), 'C', [], [], [], [], 'docstring for class C')], None),
('Module', [('ClassDef', (1, 0), 'C', [('Name', (1, 8), 'object', ('Load',))], [], [('Pass', (1, 17))], [], None)], None),
('Module', [('ClassDef', (3, 0), 'A', [], [], [('Pass', (4, 4))], [('Name', (2, 1), 'foo', ('Load',))], None)], None),
('Module', [('ClassDef', (4, 0), 'A', [], [], [('Pass', (5, 4))], [('Name', (2, 1), 'foo1', ('Load',)), ('Call', (3, 1), ('Name', (3, 1), 'bar', ('Load',)), [('Num', (3, 5), 4)], [])], None)], None),
('Module', [('ClassDef', (2, 0), 'A', [], [], [('FunctionDef', (4, 4), 'm', ('arguments', [('arg', (4, 10), 'self', None)], None, [], [], None, []), [('Pass', (5, 8))], [('Name', (3, 5), 'deco', ('Load',))], None, None)], [], None)], None),
('Module', [('FunctionDef', (1, 0), 'f', ('arguments', [], None, [], [], None, []), [('Return', (1, 8), ('Num', (1, 15), 1))], [], None, None)], None),
('Module', [('Delete', (1, 0), [('Name', (1, 4), 'v', ('Del',))])], None),
('Module', [('Assign', (1, 0), [('Name', (1, 0), 'v', ('Store',))], ('Num', (1, 4), 1))], None),
Expand Down
4 changes: 4 additions & 0 deletions Lib/test/test_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,10 @@ def test_getsource_unwrap(self):
def test_decorator_with_lambda(self):
self.assertSourceEqual(mod2.func114, 113, 115)

def test_inner_decorated(self):
self.assertSourceEqual(mod2.func142, 142, 145)
self.assertSourceEqual(mod2.func148, 148, 151)

class TestOneliners(GetSourceBase):
fodderModule = mod2
def test_oneline_lambda(self):
Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,7 @@ Václav Šmilauer
Allen W. Smith
Christopher Smith
Eric V. Smith
Ethan Smith
Gregory P. Smith
Mark Smith
Nathaniel J. Smith
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Change the line number of decorated defs back to the def or class token in the
AST, not the first decorator. Have the change of line number happen in the
compiler.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add "Patch by *yourname"." And add your name in Misc/ACKS.


Patch by Ethan Smith
23 changes: 9 additions & 14 deletions Python/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -1672,7 +1672,6 @@ static stmt_ty
ast_for_decorated(struct compiling *c, const node *n)
{
/* decorated: decorators (classdef | funcdef | async_funcdef) */
stmt_ty thing = NULL;
asdl_seq *decorator_seq = NULL;

REQ(n, decorated);
Expand All @@ -1686,19 +1685,15 @@ ast_for_decorated(struct compiling *c, const node *n)
TYPE(CHILD(n, 1)) == classdef);

if (TYPE(CHILD(n, 1)) == funcdef) {
thing = ast_for_funcdef(c, CHILD(n, 1), decorator_seq);
} else if (TYPE(CHILD(n, 1)) == classdef) {
thing = ast_for_classdef(c, CHILD(n, 1), decorator_seq);
} else if (TYPE(CHILD(n, 1)) == async_funcdef) {
thing = ast_for_async_funcdef(c, CHILD(n, 1), decorator_seq);
}
/* we count the decorators in when talking about the class' or
* function's line number */
if (thing) {
thing->lineno = LINENO(n);
thing->col_offset = n->n_col_offset;
}
return thing;
return ast_for_funcdef(c, CHILD(n, 1), decorator_seq);
}
if (TYPE(CHILD(n, 1)) == classdef) {
return ast_for_classdef(c, CHILD(n, 1), decorator_seq);
}
if (TYPE(CHILD(n, 1)) == async_funcdef) {
return ast_for_async_funcdef(c, CHILD(n, 1), decorator_seq);
}
Py_UNREACHABLE();
}

static expr_ty
Expand Down
26 changes: 24 additions & 2 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,24 @@ compiler_default_arguments(struct compiler *c, arguments_ty args)
return funcflags;
}

static int
corrected_firstlineno(struct compiler *c, stmt_ty s, asdl_seq * decos)
{
/* To keep the ability to get the relevant source of a decorated item
using inspect.getsource, we need to keep the first line number
of the code object as the first line number of the first decorator.
This used to be done via the AST, but it is better to hide this
internally.
*/
if (asdl_seq_LEN(decos)) {
expr_ty first_decorator = asdl_seq_GET(decos, 0);
return first_decorator->lineno;
}
else {
return s->lineno;
}
}

static int
compiler_function(struct compiler *c, stmt_ty s, int is_async)
{
Expand Down Expand Up @@ -1988,6 +2006,8 @@ compiler_function(struct compiler *c, stmt_ty s, int is_async)
if (!compiler_decorators(c, decos))
return 0;

int first_lineno = corrected_firstlineno(c, s, decos);

funcflags = compiler_default_arguments(c, args);
if (funcflags == -1) {
return 0;
Expand All @@ -2001,7 +2021,7 @@ compiler_function(struct compiler *c, stmt_ty s, int is_async)
funcflags |= 0x04;
}

if (!compiler_enter_scope(c, name, scope_type, (void *)s, s->lineno)) {
if (!compiler_enter_scope(c, name, scope_type, (void *)s, first_lineno)) {
return 0;
}

Expand Down Expand Up @@ -2050,6 +2070,8 @@ compiler_class(struct compiler *c, stmt_ty s)
if (!compiler_decorators(c, decos))
return 0;

int first_lineno = corrected_firstlineno(c, s, decos);

/* ultimately generate code for:
<name> = __build_class__(<func>, <name>, *<bases>, **<keywords>)
where:
Expand All @@ -2064,7 +2086,7 @@ compiler_class(struct compiler *c, stmt_ty s)

/* 1. compile the class body into a code object */
if (!compiler_enter_scope(c, s->v.ClassDef.name,
COMPILER_SCOPE_CLASS, (void *)s, s->lineno))
COMPILER_SCOPE_CLASS, (void *)s, first_lineno))
return 0;
/* this block represents what we do in the new scope */
{
Expand Down
Loading