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

fix: backward compatibility fix for changed source positions in 3.12.5 (#82) #83

Merged
merged 4 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.5, 3.6, 3.7, 3.8, 3.9, '3.10', 3.11, 3.12-dev, pypy-3.6]
python-version: [3.8, 3.9, '3.10', 3.11, 3.12-dev]

steps:
- uses: actions/checkout@v2
Expand Down
28 changes: 28 additions & 0 deletions executing/_position_node_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ def __init__(self, frame: FrameType, stmts: Set[EnhancedAST], tree: ast.Module,
typ=typ,
)

self.result = self.fix_result(self.result, self.instruction(lasti))

self.known_issues(self.result, self.instruction(lasti))

self.test_for_decorator(self.result, lasti)
Expand Down Expand Up @@ -213,6 +215,32 @@ def test_for_decorator(self, node: EnhancedAST, index: int) -> None:
if sys.version_info < (3, 12):
index += 4

def fix_result(
self, node: EnhancedAST, instruction: dis.Instruction
) -> EnhancedAST:
if (
sys.version_info >= (3, 12, 5)
and instruction.opname in ("GET_ITER", "FOR_ITER")
and isinstance(node.parent, ast.For)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
and isinstance(node.parent, ast.For)
and isinstance(node.parent, ast.For)
and node is node.parent.iter

What about comprehensions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They node positions for comprehensions are not affected.

class Foo:
    def __iter__(self):
        assert False

a = [x for x in Foo()]

output (Python 3.12.5):

Traceback (most recent call last):
  File "/home/frank/projects/executing/codi.py", line 7, in <module>
    a = [x for x in Foo()]
        ^^^^^^^^^^^^^^^^^^
  File "/home/frank/projects/executing/codi.py", line 5, in __iter__
    assert False
           ^^^^^
AssertionError

Copy link
Owner

Choose a reason for hiding this comment

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

Based on the apparently desired behaviour for For, that seems like a Python bug lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know, For is usually a multiline statement and comprehensions are usually on a single line. I will ask 😄

and node is node.parent.iter
):
# node positions have changed in 3.12.5
# https://github.com/python/cpython/issues/93691
# `for` calls __iter__ and __next__ during execution, the calling
# expression of these calls was the ast.For node since cpython 3.11 (see test_iter).
# cpython 3.12.5 changed this to the `iter` node of the loop, to make tracebacks easier to read.
# This keeps backward compatibility with older executing versions.

# there are also cases like:
#
# for a in iter(l): pass
#
# where `iter(l)` would be otherwise the resulting node for the `iter()` call and the __iter__ call of the for implementation.
# keeping the old behaviour makes it possible to distinguish both cases.

return node.parent
return node

def known_issues(self, node: EnhancedAST, instruction: dis.Instruction) -> None:
if instruction.opname in ("COMPARE_OP", "IS_OP", "CONTAINS_OP") and isinstance(
node, types_cmp_issue
Expand Down
5 changes: 1 addition & 4 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ classifiers =
License :: OSI Approved :: MIT License
Programming Language :: Python
Programming Language :: Python :: 3
Programming Language :: Python :: 3.5
Programming Language :: Python :: 3.6
Programming Language :: Python :: 3.7
Programming Language :: Python :: 3.8
Programming Language :: Python :: 3.9
Programming Language :: Python :: 3.10
Expand All @@ -25,7 +22,7 @@ packages = executing
zip_safe = False
include_package_data = True
setup_requires = setuptools; setuptools_scm[toml]
python_requires = >=3.5
python_requires = >=3.8

[options.extras_require]
tests=
Expand Down
3 changes: 3 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,9 @@ def test_module_files(self):
or 'pyparsing.py' in filename
or 'enum' in filename
)
or sys.version_info < (3,11) and (
'python.py' in filename
)
):
continue

Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = py35,py36,py37,py38,py39,py310,py311,py312,pypy35,pypy36
envlist = py38,py39,py310,py311,py312,pypy35,pypy36

[testenv]
commands =
Expand All @@ -10,7 +10,7 @@ passenv =
ADD_EXECUTING_TESTS
EXECUTING_SLOW_TESTS

[testenv:generate_small_sample-py{35,36,37,38,39,310,311}]
[testenv:generate_small_sample-py{38,39,310,311,312}]
extras = tests
deps = pysource-minimize
commands =
Expand Down
Loading