A normal generator expression like (i for i in a) produces the following AST:

Module(
body=[
Expr(
value=GeneratorExp(
elt=Name(
id="i", ctx=Load(), lineno=1, col_offset=1, end_lineno=1, end_col_offset=2
),
generators=[
comprehension(
target=Name(
id="i",
ctx=Store(),
lineno=1,
col_offset=7,
end_lineno=1,
end_col_offset=8,
),
iter=Name(
id="a",
ctx=Load(),
lineno=1,
col_offset=12,
end_lineno=1,
end_col_offset=13,
),
ifs=[],
is_async=0,
)
],
lineno=1,
*col_offset=0,*
end_lineno=1,
*end_col_offset=14,*
),
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=14,
)
],
type_ignores=[],
)

But when calling a function with a generator expression as an argument, something is off:

Module(
body=[
Expr(
value=Call(
func=Name(
id="f", ctx=Load(), lineno=1, col_offset=0, end_lineno=1, end_col_offset=1
),
args=[
GeneratorExp(
elt=Name(
id="i",
ctx=Load(),
lineno=1,
col_offset=2,
end_lineno=1,
end_col_offset=3,
),
generators=[
comprehension(
target=Name(
id="i",
ctx=Store(),
lineno=1,
col_offset=8,
end_lineno=1,
end_col_offset=9,
),
iter=Name(
id="a",
ctx=Load(),
lineno=1,
col_offset=13,
end_lineno=1,
end_col_offset=14,
),
ifs=[],
is_async=0,
)
],
lineno=1,
*col_offset=1,*
end_lineno=1,
*end_col_offset=2,*
)
],
keywords=[],
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=15,
),
lineno=1,
col_offset=0,
end_lineno=1,
end_col_offset=15,
)
],
type_ignores=[],
)

I'm not sure if this is intentional or not, because there is a call to copy_location in Python/ast.c:3149. If this call to copy_location is removed, the inconsistency goes away.

I think that was introduced in https://bugs.python.org/issue31241

Do you think it is okay to just remove the call to copy_location?

Do you think it is okay to just remove the call to copy_location?

I think that because in that case the parenthesis is collapsed with function call parenthesis, the position of the AST node for the generator expression should point to the left parenthesis.

But I would suggest asking Serhiy (I added him to the noisy list) as he was the author of the PR.

OK, so I looked into this (sine we can't wait for Serhiy).

The problem was probably introduced when Ivan added end lineno/column attributes to parse tree nodes. The offending copy_location() call copies the begin and end locations from the given node (maybegenbeg). But in the problematic case, maybegenbeg is just the left paren in the function call, not the node representing the surrounding () group.

But there's a separate argument to ast_for_call(), closepar, that should be used for the end lineno/offset.

Looking at the various calls to ast_for_call() in Python/ast.c, we find these:

• ast_for_decorator -- passes the open and close parentheses- ast_for_trailer -- passes the open and close parentheses
• ast_for_classdef -- passes NULL (because no genexpr allowed here)

So instead of calling copy_location() we should manually set the begin and end positions.

An alternative would be to add a second node* argument to copy_location(). That would require us to modify all 5 calls to it, but it still feels the better way to go. I'll see if I can produce a PR for that.

New changeset a796d8e by Miss Islington (bot) (Guido van Rossum) in branch 'master':
bpo-39235: Fix end location for genexp in call args (GH-17925)
a796d8e

New changeset 33e033d by Miss Islington (bot) in branch '3.8':
bpo-39235: Fix end location for genexp in call args (GH-17925)
33e033d

New changeset 850a885 by Serhiy Storchaka in branch 'master':
bpo-39235: Check end_lineno and end_col_offset of AST nodes. (GH-17926)
850a885

If somebody want to backport tests to 3.8, he must check manually all end positions.