Skip to content

[3.13] gh-123142: Fix too wide source locations in tracebacks of exceptions from broken iterables in comprehensions (GH-123173). #123209

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

Merged
merged 1 commit into from
Aug 22, 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
17 changes: 16 additions & 1 deletion Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
"Py_DEBUG", "exceeds_recursion_limit", "get_c_recursion_limit",
"skip_on_s390x",
"without_optimizer",
"force_not_colorized"
"force_not_colorized",
"BrokenIter",
]


Expand Down Expand Up @@ -2672,3 +2673,17 @@ def initialized_with_pyrepl():
"""Detect whether PyREPL was used during Python initialization."""
# If the main module has a __file__ attribute it's a Python module, which means PyREPL.
return hasattr(sys.modules["__main__"], "__file__")


class BrokenIter:
def __init__(self, init_raises=False, next_raises=False):
if init_raises:
1/0
self.next_raises = next_raises

def __next__(self):
if self.next_raises:
1/0

def __iter__(self):
return self
4 changes: 2 additions & 2 deletions Lib/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ def return_genexp():
x
in
y)
genexp_lines = [0, 2, 0]
genexp_lines = [0, 4, 2, 0, 4]

genexp_code = return_genexp.__code__.co_consts[1]
code_lines = self.get_code_lines(genexp_code)
Expand Down Expand Up @@ -1627,7 +1627,7 @@ def test_multiline_generator_expression(self):
self.assertOpcodeSourcePositionIs(compiled_code, 'JUMP_BACKWARD',
line=1, end_line=2, column=1, end_column=8, occurrence=1)
self.assertOpcodeSourcePositionIs(compiled_code, 'RETURN_CONST',
line=1, end_line=6, column=0, end_column=32, occurrence=1)
line=4, end_line=4, column=7, end_column=14, occurrence=1)

def test_multiline_async_generator_expression(self):
snippet = textwrap.dedent("""\
Expand Down
31 changes: 31 additions & 0 deletions Lib/test/test_dictcomps.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import traceback
import unittest

from test.support import BrokenIter

# For scope testing.
g = "Global variable"

Expand Down Expand Up @@ -127,6 +130,34 @@ def test_star_expression(self):
self.assertEqual({i: i*i for i in [*range(4)]}, expected)
self.assertEqual({i: i*i for i in (*range(4),)}, expected)

def test_exception_locations(self):
# The location of an exception raised from __init__ or
# __next__ should should be the iterator expression
def init_raises():
try:
{x:x for x in BrokenIter(init_raises=True)}
except Exception as e:
return e

def next_raises():
try:
{x:x for x in BrokenIter(next_raises=True)}
except Exception as e:
return e

for func, expected in [(init_raises, "BrokenIter(init_raises=True)"),
(next_raises, "BrokenIter(next_raises=True)"),
]:
with self.subTest(func):
exc = func()
f = traceback.extract_tb(exc.__traceback__)[0]
indent = 16
co = func.__code__
self.assertEqual(f.lineno, co.co_firstlineno + 2)
self.assertEqual(f.end_lineno, co.co_firstlineno + 2)
self.assertEqual(f.line[f.colno - indent : f.end_colno - indent],
expected)


if __name__ == "__main__":
unittest.main()
22 changes: 5 additions & 17 deletions Lib/test/test_iter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from test.support import cpython_only
from test.support.os_helper import TESTFN, unlink
from test.support import check_free_after_iterating, ALWAYS_EQ, NEVER_EQ
from test.support import BrokenIter
import pickle
import collections.abc
import functools
Expand Down Expand Up @@ -1148,35 +1149,22 @@ def test_exception_locations(self):
# The location of an exception raised from __init__ or
# __next__ should should be the iterator expression

class Iter:
def __init__(self, init_raises=False, next_raises=False):
if init_raises:
1/0
self.next_raises = next_raises

def __next__(self):
if self.next_raises:
1/0

def __iter__(self):
return self

def init_raises():
try:
for x in Iter(init_raises=True):
for x in BrokenIter(init_raises=True):
pass
except Exception as e:
return e

def next_raises():
try:
for x in Iter(next_raises=True):
for x in BrokenIter(next_raises=True):
pass
except Exception as e:
return e

for func, expected in [(init_raises, "Iter(init_raises=True)"),
(next_raises, "Iter(next_raises=True)"),
for func, expected in [(init_raises, "BrokenIter(init_raises=True)"),
(next_raises, "BrokenIter(next_raises=True)"),
]:
with self.subTest(func):
exc = func()
Expand Down
32 changes: 32 additions & 0 deletions Lib/test/test_listcomps.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import doctest
import textwrap
import traceback
import types
import unittest

from test.support import BrokenIter


doctests = """
########### Tests borrowed from or inspired by test_genexps.py ############
Expand Down Expand Up @@ -711,6 +714,35 @@ def test_multiple_comprehension_name_reuse(self):
self._check_in_scopes(code, {"x": 2, "y": [3]}, ns={"x": 3}, scopes=["class"])
self._check_in_scopes(code, {"x": 2, "y": [2]}, ns={"x": 3}, scopes=["function", "module"])

def test_exception_locations(self):
# The location of an exception raised from __init__ or
# __next__ should should be the iterator expression

def init_raises():
try:
[x for x in BrokenIter(init_raises=True)]
except Exception as e:
return e

def next_raises():
try:
[x for x in BrokenIter(next_raises=True)]
except Exception as e:
return e

for func, expected in [(init_raises, "BrokenIter(init_raises=True)"),
(next_raises, "BrokenIter(next_raises=True)"),
]:
with self.subTest(func):
exc = func()
f = traceback.extract_tb(exc.__traceback__)[0]
indent = 16
co = func.__code__
self.assertEqual(f.lineno, co.co_firstlineno + 2)
self.assertEqual(f.end_lineno, co.co_firstlineno + 2)
self.assertEqual(f.line[f.colno - indent : f.end_colno - indent],
expected)

__test__ = {'doctests' : doctests}

def load_tests(loader, tests, pattern):
Expand Down
32 changes: 32 additions & 0 deletions Lib/test/test_setcomps.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import doctest
import traceback
import unittest

from test.support import BrokenIter


doctests = """
########### Tests mostly copied from test_listcomps.py ############
Expand Down Expand Up @@ -148,6 +151,35 @@

"""

class SetComprehensionTest(unittest.TestCase):
def test_exception_locations(self):
# The location of an exception raised from __init__ or
# __next__ should should be the iterator expression

def init_raises():
try:
{x for x in BrokenIter(init_raises=True)}
except Exception as e:
return e

def next_raises():
try:
{x for x in BrokenIter(next_raises=True)}
except Exception as e:
return e

for func, expected in [(init_raises, "BrokenIter(init_raises=True)"),
(next_raises, "BrokenIter(next_raises=True)"),
]:
with self.subTest(func):
exc = func()
f = traceback.extract_tb(exc.__traceback__)[0]
indent = 16
co = func.__code__
self.assertEqual(f.lineno, co.co_firstlineno + 2)
self.assertEqual(f.end_lineno, co.co_firstlineno + 2)
self.assertEqual(f.line[f.colno - indent : f.end_colno - indent],
expected)

__test__ = {'doctests' : doctests}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix too-wide source location in exception tracebacks coming from broken
iterables in comprehensions.
5 changes: 3 additions & 2 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -5384,14 +5384,15 @@ compiler_sync_comprehension_generator(struct compiler *c, location loc,
}
if (IS_LABEL(start)) {
VISIT(c, expr, gen->iter);
ADDOP(c, loc, GET_ITER);
ADDOP(c, LOC(gen->iter), GET_ITER);
}
}
}

if (IS_LABEL(start)) {
depth++;
USE_LABEL(c, start);
ADDOP_JUMP(c, loc, FOR_ITER, anchor);
ADDOP_JUMP(c, LOC(gen->iter), FOR_ITER, anchor);
}
VISIT(c, expr, gen->target);

Expand Down
Loading