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

lib2to3 generates invalid code with filter and ternary operator #83052

Closed
ZoranSimic mannequin opened this issue Nov 20, 2019 · 7 comments
Closed

lib2to3 generates invalid code with filter and ternary operator #83052

ZoranSimic mannequin opened this issue Nov 20, 2019 · 7 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes topic-2to3 type-bug An unexpected behavior, bug, or error

Comments

@ZoranSimic
Copy link
Mannequin

ZoranSimic mannequin commented Nov 20, 2019

BPO 38871
Nosy @vstinner, @benjaminp, @corona10, @miss-islington
PRs
  • bpo-38871: Fix lib2to3 to be able to use in edge cases of filter #17780
  • [3.8] bpo-38871: Fix lib2to3 for filter-based statements that contain lambda (GH-17780) #17899
  • [3.7] bpo-38871: Fix lib2to3 for filter-based statements that contain lambda (GH-17780) #17900
    		•

    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 2020-01-07.17:32:59.853>
created_at = <Date 2019-11-20.23:12:36.660>
labels = ['3.7', '3.8', 'type-bug', 'expert-2to3', '3.9']
title = 'lib2to3 generates invalid code with filter and ternary operator'
updated_at = <Date 2020-01-07.17:52:11.436>
user = 'https://bugs.python.org/ZoranSimic'

    bugs.python.org fields:

    activity = <Date 2020-01-07.17:52:11.436>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2020-01-07.17:32:59.853>
closer = 'vstinner'
components = ['2to3 (2.x to 3.x conversion tool)']
creation = <Date 2019-11-20.23:12:36.660>
creator = 'Zoran Simic'
dependencies = []
files = []
hgrepos = []
issue_num = 38871
keywords = ['patch']
message_count = 7.0
messages = ['357112', '357133', '357384', '359525', '359526', '359529', '359530']
nosy_count = 5.0
nosy_names = ['vstinner', 'benjamin.peterson', 'corona10', 'miss-islington', 'Zoran Simic']
pr_nums = ['17780', '17899', '17900']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38871'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']
    @ZoranSimic
    Copy link
    Mannequin Author

    ZoranSimic mannequin commented Nov 20, 2019

    This code snippet exposes a small edge case in lib2to3, where syntactically invalid code is generated:

    data = [1, 2, 3, 4, 5]
x = filter(lambda x: True if x > 2 else False, data)
print(x)

    lib2to3 transforms 2nd line above to:
    x = [x for x in data if True if x > 2 else False]

    which is invalid (would be OK with parentheses after 1st 'if')

    Admittedly, the original code here is more complex that it should be ('True if foo else False' being equivalent to just 'foo'), but walked into this in "the real world" and wanted to report it.

    @ZoranSimic ZoranSimic mannequin added 3.7 (EOL) end of life 3.8 only security fixes topic-2to3 type-bug An unexpected behavior, bug, or error labels Nov 20, 2019
    @corona10
    Copy link
    Member

    I can reproduce on the latest master branch.

    ./python Tools/scripts/2to3 2.py
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Refactored 2.py
--- 2.py	(original)
+++ 2.py	(refactored)
@@ -1,3 +1,3 @@
 data = [1, 2, 3, 4, 5]
-x = filter(lambda x: True if x > 2 else False, data)
+x = [x for x in data if True if x > 2 else False]
 print(x)
RefactoringTool: Files that need to be modified:
RefactoringTool: 2.py

    @corona10
    Copy link
    Member

    Dear core developers,

    I 'd like to discuss fixing this issue.
    IMHO, There are 2 options to fix this issue.

    1. Add parenthesize when creating ListComp.
The change will be as follow:
             new = ListComp(results.get("fp").clone(),
                            results.get("fp").clone(),
                            results.get("it").clone(),
-                           results.get("xp").clone())
+                           parenthesize(results.get("xp").clone()))

    But generated codes are not pretty.

    1. Transform the lambda function as same as a normal function.
      The only needed is removing lambda specific logics on fixer.

    The generated code will be like this in this case.
    x = filter(lambda x: True if x > 2 else False, data)
    x = list(filter(lambda x: True if x > 2 else False, data))

    Personally, I prefer the 2nd option because it's more clear.
    If the proposal is accepted, I 'd like to fix this issue.

    If there are any ideas, please let me know.
    Thank you always!

    @vstinner
    Copy link
    Member

    vstinner commented Jan 7, 2020

    New changeset b821173 by Victor Stinner (Dong-hee Na) in branch 'master':
    bpo-38871: Fix lib2to3 for filter-based statements that contain lambda (GH-17780)
    b821173

    @vstinner
    Copy link
    Member

    vstinner commented Jan 7, 2020

    Thanks Zoran Simic to the bug report, and thanks Dong-hee Na for fixing it.

    It's fixed in the master branch and backports to 3.7 and 3.8 will land as soon as the CI tests pass.

    @vstinner vstinner added the 3.9 only security fixes label Jan 7, 2020
    @vstinner vstinner closed this as completed Jan 7, 2020
    @miss-islington
    Copy link
    Contributor

    New changeset 535a3c4 by Miss Islington (bot) in branch '3.7':
    bpo-38871: Fix lib2to3 for filter-based statements that contain lambda (GH-17780)
    535a3c4

    @miss-islington
    Copy link
    Contributor

    New changeset 39a5c88 by Miss Islington (bot) in branch '3.8':
    bpo-38871: Fix lib2to3 for filter-based statements that contain lambda (GH-17780)
    39a5c88

    @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
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes topic-2to3 type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @corona10 @miss-islington