From 2bd058257b703062c6d8473027142db01329ccb9 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 1 Apr 2025 09:43:42 +0200 Subject: [PATCH 01/11] Prevent emitting optimizer warnings twice in the REPL --- Lib/_pyrepl/console.py | 43 ++++++++++++++++++++++----- Lib/test/test_pyrepl/test_interact.py | 25 ++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py index 8956fb1242e52a..bd10edf36c14df 100644 --- a/Lib/_pyrepl/console.py +++ b/Lib/_pyrepl/console.py @@ -28,6 +28,7 @@ from dataclasses import dataclass, field import os.path import sys +import warnings TYPE_CHECKING = False @@ -188,16 +189,19 @@ def runcode(self, code): def runsource(self, source, filename="", symbol="single"): try: - tree = self.compile.compiler( - source, - filename, - "exec", - ast.PyCF_ONLY_AST, - incomplete_input=False, - ) + with warnings.catch_warnings(record=True) as all_warnings: + warnings.simplefilter("always") + tree = self.compile.compiler( + source, + filename, + "exec", + ast.PyCF_ONLY_AST, + incomplete_input=False, + ) except (SyntaxError, OverflowError, ValueError): self.showsyntaxerror(filename, source=source) return False + _replay_warnings(all_warnings) if tree.body: *_, last_stmt = tree.body for stmt in tree.body: @@ -205,7 +209,9 @@ def runsource(self, source, filename="", symbol="single"): the_symbol = symbol if stmt is last_stmt else "exec" item = wrapper([stmt]) try: - code = self.compile.compiler(item, filename, the_symbol) + with warnings.catch_warnings(record=True) as stmt_warnings: + warnings.simplefilter("always") + code = self.compile.compiler(item, filename, the_symbol) linecache._register_code(code, source, filename) except SyntaxError as e: if e.args[0] == "'await' outside function": @@ -220,6 +226,16 @@ def runsource(self, source, filename="", symbol="single"): self.showsyntaxerror(filename, source=source) return False + # Only emit new warnings and throw away those + # which were already emitted when the source + # was compiled to AST. + # This prevents emitting duplicate warnings which are + # raised in the AST optimizer. + all_warnings = {str(w.message) for w in all_warnings} + new_warnings = [w for w in stmt_warnings + if str(w.message) not in all_warnings] + _replay_warnings(new_warnings) + if code is None: return True @@ -227,3 +243,14 @@ def runsource(self, source, filename="", symbol="single"): if result is self.STATEMENT_FAILED: break return False + + +def _replay_warnings(ws): + for w in ws: + warnings.warn_explicit( + message=w.message, + category=w.category, + filename=w.filename, + lineno=w.lineno, + source=w.source, + ) diff --git a/Lib/test/test_pyrepl/test_interact.py b/Lib/test/test_pyrepl/test_interact.py index af5d4d0e67632a..2fecd22069b325 100644 --- a/Lib/test/test_pyrepl/test_interact.py +++ b/Lib/test/test_pyrepl/test_interact.py @@ -273,3 +273,28 @@ def test_incomplete_statement(self): code = "if foo:" console = InteractiveColoredConsole(namespace, filename="") 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 + """) + + f = io.StringIO() + with contextlib.redirect_stderr(f): + result = console.runsource(code) + self.assertFalse(result) + self.assertEqual(f.getvalue(), + ":5: SyntaxWarning: " + "'return' in a 'finally' block\n") From 08eab527599dbc2b349532f9e9d197feaffefae7 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Wed, 2 Apr 2025 19:58:40 +0200 Subject: [PATCH 02/11] Improve test --- Lib/test/test_pyrepl/test_interact.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_pyrepl/test_interact.py b/Lib/test/test_pyrepl/test_interact.py index 2fecd22069b325..a8ebe2b884f3f7 100644 --- a/Lib/test/test_pyrepl/test_interact.py +++ b/Lib/test/test_pyrepl/test_interact.py @@ -295,6 +295,7 @@ def f(): with contextlib.redirect_stderr(f): result = console.runsource(code) self.assertFalse(result) - self.assertEqual(f.getvalue(), - ":5: SyntaxWarning: " - "'return' in a 'finally' block\n") + warning = ":5: SyntaxWarning: 'return' in a 'finally' block" + output = f.getvalue() + # Check that the warning is shown only once + self.assertEqual(output.count(warning), 1) From 2049da302ec4c5b89f5abea0c29d523487d2c602 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Wed, 2 Apr 2025 22:08:00 +0200 Subject: [PATCH 03/11] Fix tests --- Lib/_pyrepl/console.py | 4 ++-- Lib/test/test_pyrepl/test_interact.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py index bd10edf36c14df..3b5496e513f79a 100644 --- a/Lib/_pyrepl/console.py +++ b/Lib/_pyrepl/console.py @@ -231,9 +231,9 @@ def runsource(self, source, filename="", symbol="single"): # was compiled to AST. # This prevents emitting duplicate warnings which are # raised in the AST optimizer. - all_warnings = {str(w.message) for w in all_warnings} + all_warnings = {str(w) for w in all_warnings} new_warnings = [w for w in stmt_warnings - if str(w.message) not in all_warnings] + if str(w) not in all_warnings] _replay_warnings(new_warnings) if code is None: diff --git a/Lib/test/test_pyrepl/test_interact.py b/Lib/test/test_pyrepl/test_interact.py index a8ebe2b884f3f7..36301d08b39298 100644 --- a/Lib/test/test_pyrepl/test_interact.py +++ b/Lib/test/test_pyrepl/test_interact.py @@ -1,6 +1,7 @@ import contextlib import io import unittest +import warnings from unittest.mock import patch from textwrap import dedent @@ -291,11 +292,10 @@ def f(): return 2 """) - f = io.StringIO() - with contextlib.redirect_stderr(f): - result = console.runsource(code) - self.assertFalse(result) - warning = ":5: SyntaxWarning: 'return' in a 'finally' block" - output = f.getvalue() - # Check that the warning is shown only once - self.assertEqual(output.count(warning), 1) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + console.runsource(code) + + count = sum("'return' in a 'finally' block" in str(w.message) + for w in caught) + self.assertEqual(count, 1) \ No newline at end of file From a922e165fcc4a9ad3c1b789c11fed4ea8d8d72f6 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Wed, 2 Apr 2025 22:34:57 +0200 Subject: [PATCH 04/11] Lint fix --- Lib/test/test_pyrepl/test_interact.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_pyrepl/test_interact.py b/Lib/test/test_pyrepl/test_interact.py index 36301d08b39298..62e4a7c3c4da65 100644 --- a/Lib/test/test_pyrepl/test_interact.py +++ b/Lib/test/test_pyrepl/test_interact.py @@ -298,4 +298,4 @@ def f(): count = sum("'return' in a 'finally' block" in str(w.message) for w in caught) - self.assertEqual(count, 1) \ No newline at end of file + self.assertEqual(count, 1) From 742a4b1b8852ffafa4b52bd5f621741465b9bba4 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 5 Apr 2025 12:57:07 +0200 Subject: [PATCH 05/11] Revert REPL changes --- Lib/_pyrepl/console.py | 43 ++++++++---------------------------------- 1 file changed, 8 insertions(+), 35 deletions(-) diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py index 3b5496e513f79a..8956fb1242e52a 100644 --- a/Lib/_pyrepl/console.py +++ b/Lib/_pyrepl/console.py @@ -28,7 +28,6 @@ from dataclasses import dataclass, field import os.path import sys -import warnings TYPE_CHECKING = False @@ -189,19 +188,16 @@ def runcode(self, code): def runsource(self, source, filename="", symbol="single"): try: - with warnings.catch_warnings(record=True) as all_warnings: - warnings.simplefilter("always") - tree = self.compile.compiler( - source, - filename, - "exec", - ast.PyCF_ONLY_AST, - incomplete_input=False, - ) + tree = self.compile.compiler( + source, + filename, + "exec", + ast.PyCF_ONLY_AST, + incomplete_input=False, + ) except (SyntaxError, OverflowError, ValueError): self.showsyntaxerror(filename, source=source) return False - _replay_warnings(all_warnings) if tree.body: *_, last_stmt = tree.body for stmt in tree.body: @@ -209,9 +205,7 @@ def runsource(self, source, filename="", symbol="single"): the_symbol = symbol if stmt is last_stmt else "exec" item = wrapper([stmt]) try: - with warnings.catch_warnings(record=True) as stmt_warnings: - warnings.simplefilter("always") - code = self.compile.compiler(item, filename, the_symbol) + code = self.compile.compiler(item, filename, the_symbol) linecache._register_code(code, source, filename) except SyntaxError as e: if e.args[0] == "'await' outside function": @@ -226,16 +220,6 @@ def runsource(self, source, filename="", symbol="single"): self.showsyntaxerror(filename, source=source) return False - # Only emit new warnings and throw away those - # which were already emitted when the source - # was compiled to AST. - # This prevents emitting duplicate warnings which are - # raised in the AST optimizer. - all_warnings = {str(w) for w in all_warnings} - new_warnings = [w for w in stmt_warnings - if str(w) not in all_warnings] - _replay_warnings(new_warnings) - if code is None: return True @@ -243,14 +227,3 @@ def runsource(self, source, filename="", symbol="single"): if result is self.STATEMENT_FAILED: break return False - - -def _replay_warnings(ws): - for w in ws: - warnings.warn_explicit( - message=w.message, - category=w.category, - filename=w.filename, - lineno=w.lineno, - source=w.source, - ) From bbefbf8d3de6e2b66d8f35bed52915b368c31e48 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 5 Apr 2025 13:01:47 +0200 Subject: [PATCH 06/11] Set up a warning registry for syntax warnings --- Include/cpython/warnings.h | 6 ++++++ Lib/test/test_pyrepl/test_interact.py | 1 - Python/_warnings.c | 19 +++++++++++++++++++ Python/errors.c | 4 ++-- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/Include/cpython/warnings.h b/Include/cpython/warnings.h index 4e3eb88e8ff447..f590f3232692ad 100644 --- a/Include/cpython/warnings.h +++ b/Include/cpython/warnings.h @@ -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); \ No newline at end of file diff --git a/Lib/test/test_pyrepl/test_interact.py b/Lib/test/test_pyrepl/test_interact.py index 62e4a7c3c4da65..261540f4d4aae0 100644 --- a/Lib/test/test_pyrepl/test_interact.py +++ b/Lib/test/test_pyrepl/test_interact.py @@ -293,7 +293,6 @@ def f(): """) with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") console.runsource(code) count = sum("'return' in a 'finally' block" in str(w.message) diff --git a/Python/_warnings.c b/Python/_warnings.c index f9dd00f3ec23aa..efbe46c476a4a0 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -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, ®istry)) + 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, diff --git a/Python/errors.c b/Python/errors.c index 14999d6dbaf72e..2b088e2f3888a1 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -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 From d047dd3d07f5aa3d162974a2d370ee9f8fddc47c Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 5 Apr 2025 13:04:06 +0200 Subject: [PATCH 07/11] newline --- Include/cpython/warnings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/cpython/warnings.h b/Include/cpython/warnings.h index f590f3232692ad..8731fd2e96b716 100644 --- a/Include/cpython/warnings.h +++ b/Include/cpython/warnings.h @@ -23,4 +23,4 @@ int _PyErr_WarnExplicitObjectWithContext( PyObject *category, PyObject *message, PyObject *filename, - int lineno); \ No newline at end of file + int lineno); From c4a27bce5e3910a048e2bd6a0fd5facb51045978 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 7 Apr 2025 22:54:49 +0200 Subject: [PATCH 08/11] Fix test --- Lib/test/test_pyrepl/test_interact.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_pyrepl/test_interact.py b/Lib/test/test_pyrepl/test_interact.py index 261540f4d4aae0..a20719033fc9b7 100644 --- a/Lib/test/test_pyrepl/test_interact.py +++ b/Lib/test/test_pyrepl/test_interact.py @@ -293,6 +293,7 @@ def f(): """) with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("default") console.runsource(code) count = sum("'return' in a 'finally' block" in str(w.message) From 7f8d1afa471fd1975dfd9d092ecd1b788b91695b Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Mon, 7 Apr 2025 22:55:27 +0200 Subject: [PATCH 09/11] PEP7 + missing decref --- Python/_warnings.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Python/_warnings.c b/Python/_warnings.c index efbe46c476a4a0..681e764d23d6b0 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1369,18 +1369,20 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, /* Like PyErr_WarnExplicitObject, but automatically sets up context */ int _PyErr_WarnExplicitObjectWithContext(PyObject *category, PyObject *message, - PyObject *filename, int lineno) + 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, ®istry)) + &module, ®istry)) { return -1; + } int rc = PyErr_WarnExplicitObject(category, message, filename, lineno, module, registry); + Py_DECREF(unused_filename); Py_DECREF(module); return rc; } From c15635bc06ef4ec8c5116ad72545425a3740cf31 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 12 Apr 2025 11:26:10 +0200 Subject: [PATCH 10/11] Add missing decref --- Python/_warnings.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/_warnings.c b/Python/_warnings.c index 681e764d23d6b0..e16153ef254cd0 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1383,6 +1383,7 @@ _PyErr_WarnExplicitObjectWithContext(PyObject *category, PyObject *message, int rc = PyErr_WarnExplicitObject(category, message, filename, lineno, module, registry); Py_DECREF(unused_filename); + Py_DECREF(registry); Py_DECREF(module); return rc; } From 97d028e239a44f5c9d9c9a6da80661bd41c79c63 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 12 Apr 2025 11:43:11 +0200 Subject: [PATCH 11/11] Add more tests --- Lib/test/test_compile.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index ce9c060641d6c5..fb784898669f04 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1636,6 +1636,24 @@ async def name_4(): pass [[]] + def test_compile_warnings(self): + # See gh-131927 + # Compile warnings originating from the same file and + # line are now only emitted once. + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("default") + compile('1 is 1', '', 'eval') + compile('1 is 1', '', 'eval') + + self.assertEqual(len(caught), 1) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + compile('1 is 1', '', 'eval') + compile('1 is 1', '', 'eval') + + self.assertEqual(len(caught), 2) + class TestBooleanExpression(unittest.TestCase): class Value: def __init__(self):