This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Generator expression has wrong line/col info when inside a Call object
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, lys.nikolaou, miss-islington, pablogsal, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2020-01-06 19:27 by lys.nikolaou, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17925 merged gvanrossum, 2020-01-09 17:55
PR 17926 merged serhiy.storchaka, 2020-01-09 19:03
PR 17927 merged miss-islington, 2020-01-09 19:19
Messages (9)
msg359454 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2020-01-06 19:27
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.
msg359455 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-01-06 19:31
I think that was introduced in  https://bugs.python.org/issue31241
msg359456 - (view) Author: Lysandros Nikolaou (lys.nikolaou) * (Python committer) Date: 2020-01-06 19:37
Do you think it is okay to just remove the call to copy_location?
msg359458 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-01-06 19:43
>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.
msg359690 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-01-09 17:42
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.
msg359692 - (view) Author: miss-islington (miss-islington) Date: 2020-01-09 19:18
New changeset a796d8ef9dd1af65f7e4d7a857b56f35b7cb6e78 by Miss Islington (bot) (Guido van Rossum) in branch 'master':
bpo-39235: Fix end location for genexp in call args (GH-17925)
https://github.com/python/cpython/commit/a796d8ef9dd1af65f7e4d7a857b56f35b7cb6e78
msg359693 - (view) Author: miss-islington (miss-islington) Date: 2020-01-09 19:39
New changeset 33e033da3c1472b0aa2ae3cff06649a1ae4aa37f by Miss Islington (bot) in branch '3.8':
bpo-39235: Fix end location for genexp in call args (GH-17925)
https://github.com/python/cpython/commit/33e033da3c1472b0aa2ae3cff06649a1ae4aa37f
msg359708 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-01-10 08:13
New changeset 850a8856e120f8cba15e557a0e791f93b43d6989 by Serhiy Storchaka in branch 'master':
bpo-39235: Check end_lineno and end_col_offset of AST nodes. (GH-17926)
https://github.com/python/cpython/commit/850a8856e120f8cba15e557a0e791f93b43d6989
msg359709 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-01-10 08:15
If somebody want to backport tests to 3.8, he must check manually all end positions.
History
Date User Action Args
2022-04-11 14:59:25adminsetgithub: 83416
2020-01-10 08:15:16serhiy.storchakasetstatus: open -> closed
versions: - Python 3.7
messages: + msg359709

resolution: fixed
stage: patch review -> resolved
2020-01-10 08:13:21serhiy.storchakasetmessages: + msg359708
2020-01-09 19:39:09miss-islingtonsetmessages: + msg359693
2020-01-09 19:19:02miss-islingtonsetpull_requests: + pull_request17334
2020-01-09 19:18:54miss-islingtonsetnosy: + miss-islington
messages: + msg359692
2020-01-09 19:03:37serhiy.storchakasetpull_requests: + pull_request17333
2020-01-09 17:55:55gvanrossumsetkeywords: + patch
stage: patch review
pull_requests: + pull_request17332
2020-01-09 17:42:12gvanrossumsetnosy: + gvanrossum
messages: + msg359690
2020-01-06 19:43:35pablogsalsetnosy: + serhiy.storchaka
messages: + msg359458
2020-01-06 19:37:10lys.nikolaousetmessages: + msg359456
2020-01-06 19:31:40pablogsalsetnosy: + pablogsal
messages: + msg359455
2020-01-06 19:27:27lys.nikolaoucreate