X Tutup
Skip to content

bpo-46521: Fix codeop to use a new partial-input mode of the parser#31010

Merged
pablogsal merged 3 commits intopython:mainfrom
pablogsal:incomplete
Feb 8, 2022
Merged

bpo-46521: Fix codeop to use a new partial-input mode of the parser#31010
pablogsal merged 3 commits intopython:mainfrom
pablogsal:incomplete

Conversation

@pablogsal
Copy link
Copy Markdown
Member

@pablogsal pablogsal commented Jan 29, 2022 •
edited by bedevere-bot
Loading

terryjreedy
terryjreedy reviewed Jan 30, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am overall pleased, pending manual tests. The 4 failures use one tricky tkinter-based test fixture that has been problematic before. They only occur with GHA Windows and Ubuntu but not GHA Mac nor any system with Pipelines. The latter plays with stdout and that might be a clue. When I get back to this, I hope in a few hours, I will start with manual tests, including with the code in the passing and failing tests, to see if the failure is in live IDLE or only the test simulation thereof.

@terryjreedy
Copy link
Copy Markdown
Member

terryjreedy commented Jan 30, 2022

Live test: any incomplete line fails due to new flags parameter.

raceback (most recent call last):
  File "F:\dev\3x\Lib\tkinter\__init__.py", line 1920, in __call__
    return self.func(*args)
           ^^^^^^^^^^^^^^^^
  File "F:\dev\3x\Lib\idlelib\multicall.py", line 176, in handler
    r = l[i](event)
        ^^^^^^^^^^^
  File "F:\dev\3x\Lib\idlelib\pyshell.py", line 1330, in enter_callback
    self.runit()
    ^^^^^^^^^^^^
  File "F:\dev\3x\Lib\idlelib\pyshell.py", line 1368, in runit
    input_is_complete = self.interp.runsource(line)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "F:\dev\3x\Lib\idlelib\pyshell.py", line 702, in runsource
    return InteractiveInterpreter.runsource(self, source, filename)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "F:\dev\3x\Lib\code.py", line 63, in runsource
    code = self.compile(source, filename, symbol)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "F:\dev\3x\Lib\codeop.py", line 174, in __call__
    return _maybe_compile(self.compiler, source, filename, symbol)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "F:\dev\3x\Lib\codeop.py", line 91, in _maybe_compile
    code1 = compiler(source + "\n", filename, symbol, flags=ast.PyCF_ALLOW_INCOMPLETE_INPUT)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Compile.__call__() got an unexpected keyword argument 'flags'

test_codeop does not cover all of the preliminary paths. Apparent fix is to change beginning of Command.__call same way you did _compile, to

    def __call__(self, source, filename, symbol, flags=0):
        codeob = compile(source, filename, symbol, self.flags | flags, True)

With this, test_idle passes for me.

terryjreedy
terryjreedy reviewed Jan 30, 2022
View reviewed changes
terryjreedy
terryjreedy reviewed Jan 30, 2022
View reviewed changes
@pablogsal pablogsal marked this pull request as ready for review February 1, 2022 19:33
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 1, 2022
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Feb 1, 2022

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 20677c3 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 1, 2022
terryjreedy
terryjreedy reviewed Feb 2, 2022
View reviewed changes
Lib/codeop.py Outdated

PyCF_DONT_IMPLY_DEDENT = 0x200 # Matches pythonrun.h.
# The following flags match the values from Include/cpython/compile.h
# Caveat emptor: These flags are undocummented on pourpose and depending
Copy link
Copy Markdown
Member

@terryjreedy terryjreedy Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/pourpose/purpose/

Copy link
Copy Markdown
Member Author

@pablogsal pablogsal Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

pablogsal
pablogsal commented Feb 2, 2022
View reviewed changes
@pablogsal pablogsal merged commit 69e1097 into python:main Feb 8, 2022
@pablogsal pablogsal deleted the incomplete branch February 8, 2022 11:54
@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Feb 8, 2022
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Feb 8, 2022

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Feb 8, 2022

Sorry @pablogsal, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 69e10976b2e7682c6d57f4272932ebc19f8e8859 3.10

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Feb 8, 2022
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Feb 8, 2022

GH-31213 is a backport of this pull request to the 3.10 branch.

pablogsal added a commit to pablogsal/cpython that referenced this pull request Feb 8, 2022
@pablogsal
[3.10] bpo-46521: Fix codeop to use a new partial-input mode of the p…
82d5141 
…arser (pythonGH-31010).

(cherry picked from commit 69e1097)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
pablogsal added a commit that referenced this pull request Feb 8, 2022
@pablogsal
[3.10] bpo-46521: Fix codeop to use a new partial-input mode of the p…
5b58db7 
…arser (GH-31010). (GH-31213)

(cherry picked from commit 69e1097)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy terryjreedy left review comments

@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pablogsal @terryjreedy @bedevere-bot @miss-islington @the-knights-who-say-ni
X Tutup