Inconsistency with lineno and col_offset info when parsing elif

[lysnikolaou]

BPO	39031
Nosy	@lysnikolaou, @pablogsal, @miss-islington
PRs	bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt #17582 [3.8] bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt (GH-17582) #17583 [3.7] bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt (GH-17582) #17584 [3.7] bpo-39031: Include elif keyword when producing lineno/col-offse… #17585 [3.8] bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt (GH-17582) #17589 bpo-39031: Fix elif start column offset when there is an else following #17596 [3.8] bpo-39031: Fix elif start column offset when there is an else following (GH-17596) #17600 [3.7] bpo-39031: Fix elif start column offset when there is an else following (GH-17596). #17601

^{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 2019-12-14.10:55:43.510>
created_at = <Date 2019-12-12.20:15:06.983>
labels = ['interpreter-core', '3.8', 'type-bug', '3.7', '3.9']
title = 'Inconsistency with lineno and col_offset info when parsing elif'
updated_at = <Date 2019-12-14.10:55:43.378>
user = 'https://github.com/lysnikolaou'

bugs.python.org fields:

activity = <Date 2019-12-14.10:55:43.378>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2019-12-14.10:55:43.510>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2019-12-12.20:15:06.983>
creator = 'lys.nikolaou'
dependencies = []
files = []
hgrepos = []
issue_num = 39031
keywords = ['patch']
message_count = 8.0
messages = ['358304', '358306', '358308', '358309', '358332', '358338', '358376', '358387']
nosy_count = 3.0
nosy_names = ['lys.nikolaou', 'pablogsal', 'miss-islington']
pr_nums = ['17582', '17583', '17584', '17585', '17589', '17596', '17600', '17601']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39031'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[lysnikolaou]

While working on pegen, we came across an inconsistency on how line number and column offset info is stored for (el)if nodes. When parsing a very simple if-elif construct like

if a:
    pass
elif b:
    pass

the following parse tree gets generated:

Module(
body=[
If(
test=Name(id="a", ctx=Load(), lineno=1, col_offset=3, end_lineno=1, end_col_offset=4),
body=[Pass(lineno=2, col_offset=4, end_lineno=2, end_col_offset=8)],
orelse=[
If(
test=Name(
id="b", ctx=Load(), lineno=3, col_offset=5, end_lineno=3, end_col_offset=6
),
body=[Pass(lineno=4, col_offset=4, end_lineno=4, end_col_offset=8)],
orelse=[],
lineno=3,
col_offset=5,
end_lineno=4,
end_col_offset=8,
)
],
lineno=1,
col_offset=0,
end_lineno=4,
end_col_offset=8,
)
],
type_ignores=[],
)

There is the inconsistency that the column offset for the if statement is 0, thus the if statement starts with the keyword if, whereas the column offset for elif if 5, which means that the elif keyword is skipped.

As Guido suggests over at we-like-parsers/pegen_experiments#107 (comment) we could very easily change Python/ast.c so that the elif statement start with the elif keyword as well.

I have a PR ready!

[miss-islington]

New changeset 025a602 by Miss Islington (bot) (Lysandros Nikolaou) in branch 'master':
bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt (GH-17582)
025a602

[pablogsal]

Hmmm, there is some problem with the CI and the 3.7 branch. Seems like Travis CI is giving some problems again.... I will investigate or maybe ask Brett to make the check not required again (we still have Azure Pipelines, testing the same thing).

[pablogsal]

The same thing happens with 3.8.....

[pablogsal]

New changeset 0ed45d0 by Pablo Galindo in branch '3.7':
[3.7] bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt (GH-17582) (bpo-17584)
0ed45d0

[pablogsal]

New changeset 3b18b17 by Pablo Galindo (Miss Islington (bot)) in branch '3.8':
bpo-39031: Include elif keyword when producing lineno/col-offset info for if_stmt (GH-17582) (GH-17589)
3b18b17

[lysnikolaou]

There was a bug in my last PR, hopefully I will get a fix some time later today.

The bug is as follows: I only updated the asdl_seq_SET call for an elif without an else, if an else is included then the behavior is like before.

After my last PR it looks like this, parsing

2:if a:
3: pass
4:elif b:
5: pass

outputs the following AST:

Module(
body=[
If(
test=Name(id="a", ctx=Load(), lineno=2, col_offset=3, end_lineno=2, end_col_offset=4),
body=[Pass(lineno=3, col_offset=4, end_lineno=3, end_col_offset=8)],
orelse=[
If(
test=Name(
id="b", ctx=Load(), lineno=4, col_offset=5, end_lineno=4, end_col_offset=6
),
body=[Pass(lineno=5, col_offset=4, end_lineno=5, end_col_offset=8)],
orelse=[],
lineno=4,
col_offset=0,
end_lineno=5,
end_col_offset=8,
)
],
lineno=2,
col_offset=0,
end_lineno=5,
end_col_offset=8,
)
],
type_ignores=[],
)

On the other hand parsing

2:if a:
3: pass
4:elif b:
5: pass
6:else:
7: pass

outputs

Module(
body=[
If(
test=Name(
id="a",
ctx=Load(),
lineno=2,
col_offset=3,
end_lineno=2,
end_col_offset=4,
),
body=[Pass(lineno=3, col_offset=4, end_lineno=3, end_col_offset=8)],
orelse=[
If(
test=Name(
id="b",
ctx=Load(),
lineno=4,
col_offset=5,
end_lineno=4,
end_col_offset=6,
),
body=[Pass(lineno=5, col_offset=4, end_lineno=5, end_col_offset=8)],
orelse=[
Pass(lineno=7, col_offset=4, end_lineno=7, end_col_offset=8)
],
lineno=4,
col_offset=5,
end_lineno=7,
end_col_offset=8,
)
],
lineno=2,
col_offset=0,
end_lineno=7,
end_col_offset=8,
)
],
type_ignores=[],
)

[pablogsal]

Thanks, Lysandros for the quick fix!