From 88b9e20560132a5479d5d34979e7a28e3d3f9bf2 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Wed, 15 May 2024 09:19:18 +0300 Subject: [PATCH 1/8] gh-119049: Fix incorrect display of warning which is constructed by C API --- Lib/test/test_capi/test_exceptions.py | 25 ++++++++++++++++++++++++- Modules/_testcapimodule.c | 10 ++++++++++ Python/_warnings.c | 5 ++--- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 1d158e3586e98d..2f5524898f0076 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -3,11 +3,12 @@ import re import sys import unittest +import textwrap from test import support from test.support import import_helper from test.support.os_helper import TESTFN, TESTFN_UNDECODABLE -from test.support.script_helper import assert_python_failure +from test.support.script_helper import assert_python_failure, assert_python_ok from test.support.testcase import ExceptionIsLikeMixin from .test_misc import decode_stderr @@ -68,6 +69,28 @@ def test_exc_info(self): else: self.assertTrue(False) + def test_warn_with_stacklevel(self): + code = textwrap.dedent('''\ + import _testcapi + + def foo(): + _testcapi.test_warn() + + foo() # line 6 + + + foo() # line 9 + ''') + proc = assert_python_ok("-c", code) + warnings = proc.err.splitlines() + self.assertEqual(len(warnings), 4) + self.assertTrue( + warnings[1].startswith(b" foo() # line 6") + ) + self.assertTrue( + warnings[3].startswith(b" foo() # line 9") + ) + class Test_FatalError(unittest.TestCase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ff31724c0e9ff9..065fc9acfc9a77 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3303,6 +3303,15 @@ test_reftracer(PyObject *ob, PyObject *Py_UNUSED(ignored)) return NULL; } +static PyObject * +test_warn(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + if (PyErr_WarnEx(PyExc_RuntimeWarning, "Testing PyErr_WarnEx", 2)) { + return NULL; + } + Py_RETURN_NONE; +} + static PyMethodDef TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, {"test_config", test_config, METH_NOARGS}, @@ -3444,6 +3453,7 @@ static PyMethodDef TestMethods[] = { {"function_set_closure", function_set_closure, METH_VARARGS, NULL}, {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS}, {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, + {"test_warn", test_warn, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Python/_warnings.c b/Python/_warnings.c index 793cbc657f3184..17404d33c1cc9b 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -569,10 +569,9 @@ call_show_warning(PyThreadState *tstate, PyObject *category, PyObject *show_fn, *msg, *res, *warnmsg_cls = NULL; PyInterpreterState *interp = tstate->interp; - /* If the source parameter is set, try to get the Python implementation. - The Python implementation is able to log the traceback where the source + /* The Python implementation is able to log the traceback where the source was allocated, whereas the C implementation doesn't. */ - show_fn = GET_WARNINGS_ATTR(interp, _showwarnmsg, source != NULL); + show_fn = GET_WARNINGS_ATTR(interp, _showwarnmsg, 1); if (show_fn == NULL) { if (PyErr_Occurred()) return -1; From f6da0053208134fbfedda471bd173d8759c446f7 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Wed, 15 May 2024 15:29:17 +0300 Subject: [PATCH 2/8] Change name --- Lib/test/test_capi/test_exceptions.py | 2 +- Modules/_testcapimodule.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 2f5524898f0076..6436c55f3df8cc 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -74,7 +74,7 @@ def test_warn_with_stacklevel(self): import _testcapi def foo(): - _testcapi.test_warn() + _testcapi.function_set_warning() foo() # line 6 diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 065fc9acfc9a77..f99ebf0dde4f9e 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3304,7 +3304,7 @@ test_reftracer(PyObject *ob, PyObject *Py_UNUSED(ignored)) } static PyObject * -test_warn(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +function_set_warning(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) { if (PyErr_WarnEx(PyExc_RuntimeWarning, "Testing PyErr_WarnEx", 2)) { return NULL; @@ -3453,7 +3453,7 @@ static PyMethodDef TestMethods[] = { {"function_set_closure", function_set_closure, METH_VARARGS, NULL}, {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS}, {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, - {"test_warn", test_warn, METH_NOARGS}, + {"function_set_warning", function_set_warning, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From ee4dca9cf9ca6f2b1bf9068137ff3480fe977c42 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Thu, 16 May 2024 13:31:56 +0300 Subject: [PATCH 3/8] Add a test case with finalization --- Lib/test/test_capi/test_exceptions.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 6436c55f3df8cc..70ee668d72cd3d 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -91,6 +91,23 @@ def foo(): warnings[3].startswith(b" foo() # line 9") ) + def test_warn_during_finalization(self): + code = textwrap.dedent('''\ + import _testcapi + + class Foo: + def __del__(self): + _testcapi.function_set_warning() + + ref = Foo() + ''') + proc = assert_python_ok("-c", code) + warnings = proc.err.splitlines() + # Due to the finalization of the interpreter, the source will be ommited + # because the ``warnings`` module cannot be imported at this time + self.assertEqual(len(warnings), 1) + self.assertEqual(warnings[0], b':0: RuntimeWarning: Testing PyErr_WarnEx') + class Test_FatalError(unittest.TestCase): From fcc49ed9c89b892fcaa2d27637f88bdf959f9fc6 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Thu, 16 May 2024 14:29:05 +0300 Subject: [PATCH 4/8] Update test case --- Lib/test/test_capi/test_exceptions.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 70ee668d72cd3d..c3a430ea81456a 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -83,13 +83,14 @@ def foo(): ''') proc = assert_python_ok("-c", code) warnings = proc.err.splitlines() + expected = [ + b':6: RuntimeWarning: Testing PyErr_WarnEx', + b' foo() # line 6', + b':9: RuntimeWarning: Testing PyErr_WarnEx', + b' foo() # line 9' + ] self.assertEqual(len(warnings), 4) - self.assertTrue( - warnings[1].startswith(b" foo() # line 6") - ) - self.assertTrue( - warnings[3].startswith(b" foo() # line 9") - ) + self.assertEqual(warnings, expected) def test_warn_during_finalization(self): code = textwrap.dedent('''\ From f75f5cb287c469e1bdeabfce33ace1ff6a6da2a1 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Thu, 16 May 2024 22:18:33 +0300 Subject: [PATCH 5/8] Update Lib/test/test_capi/test_exceptions.py Co-authored-by: Serhiy Storchaka --- Lib/test/test_capi/test_exceptions.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index c3a430ea81456a..f501dc644ae021 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -83,14 +83,12 @@ def foo(): ''') proc = assert_python_ok("-c", code) warnings = proc.err.splitlines() - expected = [ + self.assertEqual(warnings, [ b':6: RuntimeWarning: Testing PyErr_WarnEx', b' foo() # line 6', b':9: RuntimeWarning: Testing PyErr_WarnEx', - b' foo() # line 9' - ] - self.assertEqual(len(warnings), 4) - self.assertEqual(warnings, expected) + b' foo() # line 9', + ]) def test_warn_during_finalization(self): code = textwrap.dedent('''\ From 86dda917b3ab8ce28d67f219f9c6c3bdfe999f95 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Thu, 16 May 2024 22:22:25 +0300 Subject: [PATCH 6/8] Address review --- Lib/test/test_capi/test_exceptions.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index f501dc644ae021..66b128aecc9388 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -95,8 +95,10 @@ def test_warn_during_finalization(self): import _testcapi class Foo: - def __del__(self): + def foo(self): _testcapi.function_set_warning() + def __del__(self): + self.foo() ref = Foo() ''') @@ -104,8 +106,7 @@ def __del__(self): warnings = proc.err.splitlines() # Due to the finalization of the interpreter, the source will be ommited # because the ``warnings`` module cannot be imported at this time - self.assertEqual(len(warnings), 1) - self.assertEqual(warnings[0], b':0: RuntimeWarning: Testing PyErr_WarnEx') + self.assertEqual(warnings, [b':7: RuntimeWarning: Testing PyErr_WarnEx']) class Test_FatalError(unittest.TestCase): From 733724586b9f86f323ff8d87405e31ec3e6adbe6 Mon Sep 17 00:00:00 2001 From: Kirill Podoprigora Date: Thu, 16 May 2024 22:28:23 +0300 Subject: [PATCH 7/8] too long line --- Lib/test/test_capi/test_exceptions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 66b128aecc9388..c475b6d78d0c56 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -106,7 +106,9 @@ def __del__(self): warnings = proc.err.splitlines() # Due to the finalization of the interpreter, the source will be ommited # because the ``warnings`` module cannot be imported at this time - self.assertEqual(warnings, [b':7: RuntimeWarning: Testing PyErr_WarnEx']) + self.assertEqual(warnings, [ + b':7: RuntimeWarning: Testing PyErr_WarnEx', + ]) class Test_FatalError(unittest.TestCase): From b53c6edad0a8e3e724869b5d32fa8b4fec4be706 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 16 May 2024 23:04:08 +0300 Subject: [PATCH 8/8] Add a NEWS entry. --- .../2024-05-16-23-02-03.gh-issue-119049.qpd_S-.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-05-16-23-02-03.gh-issue-119049.qpd_S-.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-16-23-02-03.gh-issue-119049.qpd_S-.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-16-23-02-03.gh-issue-119049.qpd_S-.rst new file mode 100644 index 00000000000000..1d7aad8d1e5be6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-16-23-02-03.gh-issue-119049.qpd_S-.rst @@ -0,0 +1,2 @@ +Fix displaying the source line for warnings created by the C API if the +:mod:`warnings` module had not yet been imported.