Skip to content

gh-131927: Prevent emitting optimizer warnings twice in the REPL #131993

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.

Sign up for GitHub

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 12 commits into from
Apr 12, 2025
Merged

gh-131927: Prevent emitting optimizer warnings twice in the REPL #131993

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2bd0582
Prevent emitting optimizer warnings twice in the REPL
tomasr8 Apr 1, 2025
08eab52
Improve test
tomasr8 Apr 2, 2025
2049da3
Fix tests
tomasr8 Apr 2, 2025
a922e16
Lint fix
tomasr8 Apr 2, 2025
742a4b1
Revert REPL changes
tomasr8 Apr 5, 2025
bbefbf8
Set up a warning registry for syntax warnings
tomasr8 Apr 5, 2025
d047dd3
newline
tomasr8 Apr 5, 2025
c4a27bc
Fix test
tomasr8 Apr 7, 2025
7f8d1af
PEP7 + missing decref
tomasr8 Apr 7, 2025
c15635b
Add missing decref
tomasr8 Apr 12, 2025
97d028e
Add more tests
tomasr8 Apr 12, 2025
a8fa07f
Merge remote-tracking branch 'upstream/main' into repl-warnings
tomasr8 Apr 12, 2025
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
6 changes: 6 additions & 0 deletions Include/cpython/warnings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,9 @@ PyAPI_FUNC(int) PyErr_WarnExplicitFormat(

// DEPRECATED: Use PyErr_WarnEx() instead.
#define PyErr_Warn(category, msg) PyErr_WarnEx((category), (msg), 1)

int _PyErr_WarnExplicitObjectWithContext(
PyObject *category,
PyObject *message,
PyObject *filename,
int lineno);
25 changes: 25 additions & 0 deletions Lib/test/test_pyrepl/test_interact.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import contextlib
import io
import unittest
import warnings
from unittest.mock import patch
from textwrap import dedent

Expand Down Expand Up @@ -273,3 +274,27 @@ def test_incomplete_statement(self):
code = "if foo:"
console = InteractiveColoredConsole(namespace, filename="<stdin>")
self.assertTrue(_more_lines(console, code))


class TestWarnings(unittest.TestCase):
def test_pep_765_warning(self):
"""
Test that a SyntaxWarning emitted from the
AST optimizer is only shown once in the REPL.
"""
# gh-131927
console = InteractiveColoredConsole()
code = dedent("""\
def f():
try:
return 1
finally:
return 2
""")

with warnings.catch_warnings(record=True) as caught:
console.runsource(code)

count = sum("'return' in a 'finally' block" in str(w.message)
Copy link
Member

Choose a reason for hiding this comment

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

You can use .count() here.

"'return' in a 'finally' block" may be an error in future versions. Maybe use some more long living warnings, like assert(1, 'message') or 1 is 1? BTW, they are currently emitted 3 times, not 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can use .count() here.

Hmm I don't think I can, unless I want to match the warning message exactly?

"'return' in a 'finally' block" may be an error in future versions. Maybe use some more long living warnings, like assert(1, 'message') or 1 is 1?

I can't if I want to test the new repl. The PEP 765 warning is the only one emitted from the ast optimizer which is needed to trigger this behaviour in the repl. However this does change the behaviour of all warnings that use _PyErr_EmitSyntaxWarning so I also added tests to test_compile for it.

for w in caught)
self.assertEqual(count, 1)
19 changes: 19 additions & 0 deletions Python/_warnings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,25 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message,
return 0;
}

/* Like PyErr_WarnExplicitObject, but automatically sets up context */
int
_PyErr_WarnExplicitObjectWithContext(PyObject *category, PyObject *message,
PyObject *filename, int lineno)
{
PyObject *unused_filename, *module, *registry;
int unused_lineno;
int stack_level = 1;

if (!setup_context(stack_level, NULL, &unused_filename, &unused_lineno,
&module, &registry))
return -1;

int rc = PyErr_WarnExplicitObject(category, message, filename, lineno,
module, registry);
Py_DECREF(module);
return rc;
}

int
PyErr_WarnExplicit(PyObject *category, const char *text,
const char *filename_str, int lineno,
Expand Down
4 changes: 2 additions & 2 deletions Python/errors.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1906,8 +1906,8 @@ int
_PyErr_EmitSyntaxWarning(PyObject *msg, PyObject *filename, int lineno, int col_offset,
int end_lineno, int end_col_offset)
{
if (PyErr_WarnExplicitObject(PyExc_SyntaxWarning, msg, filename,
lineno, NULL, NULL) < 0)
if (_PyErr_WarnExplicitObjectWithContext(PyExc_SyntaxWarning, msg,
filename, lineno) < 0)
{
if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) {
/* Replace the SyntaxWarning exception with a SyntaxError
Expand Down
Loading