bpo-18374: fix wrong col_offset of some ast.BinOp instances#14607
Conversation
Nested BinOp instances (e.g. a+b+c) had a wrong col_offset for the second BinOp (e.g. 2 instead of 0 in the example). Fix it by using the correct st node to copy the line and col_offset from in ast.c.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Our records indicate we have not received your CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
There was a problem hiding this comment.
the output looks correct
validating this change using astpretty
before
$ astpretty /dev/stdin <<< '1 + 2 + 3'
Module(
body=[
Expr(
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=9,
value=BinOp(
lineno=1,
col_offset=6,
end_lineno=1,
end_col_offset=9,
left=BinOp(
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=5,
left=Constant(lineno=1, col_offset=0, end_lineno=1, end_col_offset=1, value=1, kind=None),
op=Add(),
right=Constant(lineno=1, col_offset=4, end_lineno=1, end_col_offset=5, value=2, kind=None),
),
op=Add(),
right=Constant(lineno=1, col_offset=8, end_lineno=1, end_col_offset=9, value=3, kind=None),
),
),
],
type_ignores=[],
)roughly looks like this
1 + 2 + 3
AAA # outer BinOp
BBBBB # inner BinOpafter
$ astpretty /dev/stdin <<< '1 + 2 + 3'
Module(
body=[
Expr(
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=9,
value=BinOp(
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=9,
left=BinOp(
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=5,
left=Constant(lineno=1, col_offset=0, end_lineno=1, end_col_offset=1, value=1, kind=None),
op=Add(),
right=Constant(lineno=1, col_offset=4, end_lineno=1, end_col_offset=5, value=2, kind=None),
),
op=Add(),
right=Constant(lineno=1, col_offset=8, end_lineno=1, end_col_offset=9, value=3, kind=None),
),
),
],
type_ignores=[],
)roughly looks like this
1 + 2 + 3
AAAAAAAAA # outer BinOp
BBBBB # inner BinOp
|
@cfbolz Did you sign the CLA? |
|
@ilevkivskyi yes, signed it last Friday when I opened the pull request. |
|
Thanks @cfbolz for the PR, and @ilevkivskyi for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8. |
|
GH-14653 is a backport of this pull request to the 3.8 branch. |
|
Sorry, @cfbolz and @ilevkivskyi, I could not cleanly backport this to |
…-14607) Nested BinOp instances (e.g. a+b+c) had a wrong col_offset for the second BinOp (e.g. 2 instead of 0 in the example). Fix it by using the correct st node to copy the line and col_offset from in ast.c. (cherry picked from commit 110a47c) Co-authored-by: Carl Friedrich Bolz-Tereick <cfbolz@gmx.de>
| self.assertEqual(child_binop.col_offset, 0) | ||
| self.assertEqual(parent_binop.lineno, 1) | ||
| self.assertEqual(child_binop.end_col_offset, 2) | ||
| self.assertEqual(parent_binop.end_lineno, 2) |
There was a problem hiding this comment.
Oh, after merging this I noticed this block of asserts checks parent for lineno and end_lineno.
| self.assertEqual(grandchild_binop.col_offset, 0) | ||
| self.assertEqual(parent_binop.lineno, 1) | ||
| self.assertEqual(grandchild_binop.end_col_offset, 3) | ||
| self.assertEqual(parent_binop.end_lineno, 2) |
There was a problem hiding this comment.
Same in this block, it should be granchild_binop in two asserts in this block. @cfbolz could you please make another PR with a fix?
Nested BinOp instances (e.g. a+b+c) had a wrong col_offset for the second BinOp (e.g. 2 instead of 0 in the example). Fix it by using the correct st node to copy the line and col_offset from in ast.c. (cherry picked from commit 110a47c) Co-authored-by: Carl Friedrich Bolz-Tereick <cfbolz@gmx.de>
|
Thanks a lot, Ivan and Anthony! |
|
Yep, will fix the problems and ping you. |
…-14607) Nested BinOp instances (e.g. a+b+c) had a wrong col_offset for the second BinOp (e.g. 2 instead of 0 in the example). Fix it by using the correct st node to copy the line and col_offset from in ast.c.
…-14607) Nested BinOp instances (e.g. a+b+c) had a wrong col_offset for the second BinOp (e.g. 2 instead of 0 in the example). Fix it by using the correct st node to copy the line and col_offset from in ast.c.
6 participants
Nested BinOp instances (e.g. a+b+c) had a wrong col_offset for the second BinOp (e.g. 2 instead of 0 in the example). Fix it by using the correct st node to copy the line and col_offset from in ast.c.
https://bugs.python.org/issue18374