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

Jump to bottom

ast.parse gives wrong position (col_offset) for some BinOp-s #62574

Closed
aivarannamaa mannequin opened this issue Jul 6, 2013 · 14 comments
Closed

ast.parse gives wrong position (col_offset) for some BinOp-s #62574

aivarannamaa mannequin opened this issue Jul 6, 2013 · 14 comments
Assignees
@serhiy-storchaka
Labels
3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@aivarannamaa
Copy link
Mannequin

aivarannamaa mannequin commented Jul 6, 2013

BPO 18374
Nosy @cfbolz, @benjaminp, @aivarannamaa, @serhiy-storchaka, @ilevkivskyi, @pablogsal, @miss-islington
PRs
  • bpo-18374: fix wrong col_offset of some ast.BinOp instances #14607
  • [3.8] bpo-18374: fix wrong col_offset of some ast.BinOp instances (GH-14607) #14653
  • bpo-18374: fix tests to check the correct thing about line numbers #14659
  • [3.8] bpo-18374: fix tests to check the correct thing about line numbers (GH-14659) #14672
    		•
    Files
  • binop_bug.py
  • 18374-binop-col-offset-with-test.diff
  • binop-offset.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 2019-07-09.19:37:39.904>
created_at = <Date 2013-07-06.09:35:02.611>
labels = ['interpreter-core', 'type-bug', '3.8', '3.9']
title = 'ast.parse gives wrong position (col_offset) for some BinOp-s'
updated_at = <Date 2019-07-09.19:37:39.896>
user = 'https://github.com/aivarannamaa'

    bugs.python.org fields:

    activity = <Date 2019-07-09.19:37:39.896>
actor = 'levkivskyi'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2019-07-09.19:37:39.904>
closer = 'levkivskyi'
components = ['Interpreter Core']
creation = <Date 2013-07-06.09:35:02.611>
creator = 'Aivar.Annamaa'
dependencies = []
files = ['30793', '34945', '48458']
hgrepos = []
issue_num = 18374
keywords = ['patch']
message_count = 14.0
messages = ['192394', '216696', '347294', '347313', '347319', '347320', '347324', '347325', '347326', '347370', '347509', '347510', '347554', '347560']
nosy_count = 8.0
nosy_names = ['Carl.Friedrich.Bolz', 'benjamin.peterson', 'Aivar.Annamaa', 'serhiy.storchaka', 'sam.kimbrel', 'levkivskyi', 'pablogsal', 'miss-islington']
pr_nums = ['14607', '14653', '14659', '14672']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue18374'
versions = ['Python 3.8', 'Python 3.9']
    @aivarannamaa
    Copy link
    Mannequin Author

    aivarannamaa mannequin commented Jul 6, 2013

    When parsing following program:

    1+2-3

    ast.parse gives col_offset=3 to outermost BinOp (as shown by the attached test script). Correct col_offset would be 0 for both BinOp-s

    @aivarannamaa aivarannamaa mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jul 6, 2013
    @samkimbrel
    Copy link
    Mannequin

    samkimbrel mannequin commented Apr 17, 2014

    Here's a patch that corrects col_offset for binops in both the ast module and in the compiler proper. I've incorporated Aivar's test into test_ast.py; if there are test suites for compile.c please let me know and I can add something there too.

    @PCManticore PCManticore mannequin added the type-bug An unexpected behavior, bug, or error label Aug 22, 2014
    @cfbolz
    Copy link
    Mannequin

    cfbolz mannequin commented Jul 4, 2019

    FWIW, in my opinion the col_offsets of the two nodes should be 1 and 3, respectively (the positions of the operators).

    @serhiy-storchaka
    Copy link
    Member

    I am not sure that changes in compile.c are needed now.

    Python/Python-ast.c is generated file. And perhaps it would be better to fix CST instead of AST.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes labels Jul 5, 2019
    @serhiy-storchaka serhiy-storchaka self-assigned this Jul 5, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Jul 5, 2019

    Pablo, Ivan: this old bug is able CST and AST, things that you know. You may want to have a look ;-)

    Note: This bug is discussed in https://twitter.com/cfbolz/status/1146866203975139328

    @aivarannamaa
    Copy link
    Mannequin Author

    aivarannamaa mannequin commented Jul 5, 2019

    in my opinion the col_offsets of the two nodes should be 1 and 3, respectively (the positions of the operators)

    This would not match the documentation:

    one can get the source segment of a one-line expression node using source_line[node.col_offset : node.end_col_offset]

    @serhiy-storchaka
    Copy link
    Member

    in my opinion the col_offsets of the two nodes should be 1 and 3, respectively (the positions of the operators)

    Such change was committed once, but it was quickly reverted because it was not well discussed and contradicts the documentation.

    @cfbolz
    Copy link
    Mannequin

    cfbolz mannequin commented Jul 5, 2019

    OK, fair enough. That means right now there is no way to find the position of the operator using the ast module at the moment, correct?

    @serhiy-storchaka
    Copy link
    Member

    It is between the end position of the left operand and the start position of the right operand.

    @cfbolz
    Copy link
    Mannequin

    cfbolz mannequin commented Jul 5, 2019

    I think the attached patch fixes the problem. Will create a pull request soon.

    @cfbolz cfbolz mannequin removed 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes labels Jul 5, 2019
    @ilevkivskyi
    Copy link
    Member

    New changeset 110a47c by Ivan Levkivskyi (Carl Friedrich Bolz-Tereick) in branch 'master':
    bpo-18374: fix wrong col_offset of some ast.BinOp instances (GH-14607)
    110a47c

    @miss-islington
    Copy link
    Contributor

    New changeset c7be35c by Miss Islington (bot) in branch '3.8':
    bpo-18374: fix wrong col_offset of some ast.BinOp instances (GH-14607)
    c7be35c

    @ilevkivskyi
    Copy link
    Member

    New changeset 430a9f4 by Ivan Levkivskyi (Carl Friedrich Bolz-Tereick) in branch 'master':
    bpo-18374: fix tests to check the correct thing about line numbers (GH-14659)
    430a9f4

    @ilevkivskyi
    Copy link
    Member

    New changeset 68bd9c5 by Ivan Levkivskyi (Miss Islington (bot)) in branch '3.8':
    bpo-18374: fix tests to check the correct thing about line numbers (GH-14659) (GH-14672)
    68bd9c5

    @ilevkivskyi ilevkivskyi added 3.8 only security fixes 3.9 only security fixes labels Jul 9, 2019
    @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
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @vstinner @serhiy-storchaka @ilevkivskyi @miss-islington