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

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

Merged
merged 3 commits into from Feb 8, 2022

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jan 29, 2022

Copy link
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.

Lib/codeop.py Show resolved Hide resolved
Lib/codeop.py Show resolved Hide resolved
lel.py Outdated Show resolved Hide resolved
@terryjreedy
Copy link
Member

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.

Lib/codeop.py Outdated Show resolved Hide resolved
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 1, 2022
@bedevere-bot
Copy link

🤖 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
Lib/codeop.py Outdated
@@ -64,7 +40,11 @@

__all__ = ["compile_command", "Compile", "CommandCompiler"]

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
Member

Choose a reason for hiding this comment

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

/pourpose/purpose/

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

Lib/codeop.py Outdated Show resolved Hide resolved
@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
Contributor

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

@miss-islington
Copy link
Contributor

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
Copy link

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
…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
…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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants