10000 bpo-33211: Change line number of decorated nodes back to def by emmatyping · Pull Request #6460 · python/cpython · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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

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

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
b60b6a6
Change line number of decorated nodes back to def
emmatyping Apr 12, 2018
ae578b0
Whitespace fixes
emmatyping Apr 12, 2018
191bd38
Fix test_dis
emmatyping Apr 12, 2018
da42832
Change fistlineno of decorated nodes in the compiler
emmatyping Apr 12, 2018
649506c
Remove unused var
emmatyping Apr 12, 2018
29b0d60
Fix indentation bug
emmatyping Apr 12, 2018
68b2228
Run patcheck
emmatyping Apr 13, 2018
09d8ab2
Merge remote tracking branch into deflineno
emmatyping Apr 22, 2018
df9fbe2
Change tests back to original state
emmatyping Apr 23, 2018
9840f84
Fix line number in compiler
emmatyping Apr 23, 2018
0842c20
Use unreachable macro
emmatyping Apr 23, 2018
7f4fcdf
Fix silly indentation mistake
emmatyping Apr 23, 2018
e4e7f1a
Test ast line numbers for decorated nodes
emmatyping Apr 24, 2018
a8e747d
Fix true order tests for decorators
emmatyping Apr 24, 2018
3df5862
Fix bad indentation
emmatyping Apr 24, 2018
44a3cd9
Respond to review
emmatyping Apr 24, 2018
afd3b7e
Fix regression with inner decorated nodes
emmatyping Apr 24, 2018
bdc9942
Remove redundant line number setting
emmatyping May 7, 2018
914d0a0
Fix style in compile.c
emmatyping May 7, 2018
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
Prev Previous commit
Next Next commit
Fix line number in compiler
  • Loading branch information
@emmatyping
emmatyping committed Apr 23, 2018
commit 9840f842ae84d304b8de5d9405b85c2d26637f66
34 changes: 27 additions & 7 deletions Python/compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,26 @@ compiler_default_arguments(struct compiler *c, arguments_ty args)
return funcflags;
}

static int corrected_firstlineno(struct compiler *c, stmt_ty s,
asdl_seq * decos)
Copy link
Member

Choose a reason for hiding this comment

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

Why weird indentation on a separate line?

Copy link
Member Author
@emmatyping emmatyping Apr 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think patchcheck messed up some of this up, sorry for the noise.

On the Py_UNREACHABLE() line, I originally had double tab indentation (I forgot to change Visual Studio's settings), and its seemed to change that to triple groups of 4 spaces, so I'm guessing when I ran patchcheck it caused other problems. I'll manually verify in future.

{
/* To keep the ability to get the relevant source of a decorated item
Copy link
Member

Choose a reason for hiding this comment

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

Us 4 spaces for indentation.

using inspect.getsource, we need to keep the first line number
Copy link
Member

Choose a reason for hiding this comment

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

Align wrapped lines to "To keep", not to "/*".

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) > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

> 0 is not needed. Using just asdl_seq_LEN(decos) in the boolean context is common.

expr_ty first_decorator = asdl_seq_GET(decos, 0);
c->u->u_firstlineno = first_decorator->lineno;
Copy link
Member

Choose a reason for hiding this comment

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

Seems c->u still refers to the outer scope.

Add a regression test for these cases:

def f():
    @deco
    def g(): pass
class A:
    @deco
    def m(self): pass

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems c->u still refers to the outer scope.

I'm probably missing something, but the generated AST is:

('Module',
 [('ClassDef',
   (1, 0),
   'A',
   [],
   [],
   [('FunctionDef',
     (3, 4),
     'm',
     ('arguments', [('arg', (3, 10), 'self', None)], None, [], [], None, []),
     [('Pass', (3, 17))],
     [('Name', (2, 5), 'deco', ('Load',))],
     None,
     None)],
   [],
   None)],
 None)

which seems correct. Perhaps I am misunderstanding?

return first_decorator->lineno;
}
else {
return s->lineno;
}

Copy link
Member

Choose a reason for hiding this comment

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

This empty line is unnecessary.

}

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

// fixup decorators to be the first line number internally.
if (asdl_seq_LEN(decos) > 0) {
expr_ty first_decorator = asdl_seq_GET(decos, 0);
c->u->u_firstlineno = first_decorator->lineno;
}

Copy link
Member
@ilevkivskyi ilevkivskyi Apr 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Too many empty lines.

int first_lineno = corrected_firstlineno(c, s, decos);
Copy link
Member

Choose a reason for hiding this comment

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

Indentation is misleading, shift left.



Copy link
Member

Choose a reason for hiding this comment

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

Too many empty lines

funcflags = compiler_default_arguments(c, args);
if (funcflags == -1) {
Expand All @@ -2007,7 +2025,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 @@ -2056,6 +2074,8 @@ compiler_class(struct compiler *c, stmt_ty s)
if (!compiler_decorators(c, decos))
return 0;

int first_lineno = corrected_firstlineno(c, s, decos);
Copy link
Member

Choose a reason for hiding this comment

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

Wrong indentation.


/* ultimately generate code for:
<name> = __build_class__(<func>, <name>, *<bases>, **<keywords>)
where:
Expand All @@ -2070,7 +2090,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
0