From 55c99d97e14618dfce41472dd4446f763b0da13f Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Tue, 11 Apr 2023 11:53:06 +0100 Subject: [PATCH] gh-77757: replace exception wrapping by PEP-678 notes in typeobject's __set_name__ (#103402) --- Doc/whatsnew/3.12.rst | 4 +++ Include/internal/pycore_pyerrors.h | 2 ++ Lib/test/test_functools.py | 4 +-- Lib/test/test_subclassinit.py | 22 +++++++-------- ...3-04-09-22-21-57.gh-issue-77757._Ow-u2.rst | 3 ++ Objects/typeobject.c | 6 ++-- Python/codecs.c | 28 ++----------------- Python/errors.c | 27 ++++++++++++++++++ 8 files changed, 55 insertions(+), 41 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-04-09-22-21-57.gh-issue-77757._Ow-u2.rst diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 0eb16367267..c7fc7d229cd 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -192,6 +192,10 @@ Other Language Changes * :class:`slice` objects are now hashable, allowing them to be used as dict keys and set items. (Contributed by Will Bradshaw and Furkan Onder in :gh:`101264`.) +* Exceptions raised in a typeobject's ``__set_name__`` method are no longer + wrapped by a :exc:`RuntimeError`. Context information is added to the + exception as a :pep:`678` note. (Contributed by Irit Katriel in :gh:`77757`.) + New Modules =========== diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index 1bb4a9aa103..4620a269644 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -109,6 +109,8 @@ extern PyObject* _Py_Offer_Suggestions(PyObject* exception); PyAPI_FUNC(Py_ssize_t) _Py_UTF8_Edit_Cost(PyObject *str_a, PyObject *str_b, Py_ssize_t max_cost); +void _PyErr_FormatNote(const char *format, ...); + #ifdef __cplusplus } #endif diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 57db96d37ee..af286052a7d 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2980,7 +2980,7 @@ class TestCachedProperty(unittest.TestCase): def test_reuse_different_names(self): """Disallow this case because decorated function a would not be cached.""" - with self.assertRaises(RuntimeError) as ctx: + with self.assertRaises(TypeError) as ctx: class ReusedCachedProperty: @py_functools.cached_property def a(self): @@ -2989,7 +2989,7 @@ class TestCachedProperty(unittest.TestCase): b = a self.assertEqual( - str(ctx.exception.__context__), + str(ctx.exception), str(TypeError("Cannot assign the same cached_property to two different names ('a' and 'b').")) ) diff --git a/Lib/test/test_subclassinit.py b/Lib/test/test_subclassinit.py index 0ad7d17fbd4..310473a4a2f 100644 --- a/Lib/test/test_subclassinit.py +++ b/Lib/test/test_subclassinit.py @@ -134,30 +134,28 @@ class Test(unittest.TestCase): def __set_name__(self, owner, name): 1/0 - with self.assertRaises(RuntimeError) as cm: + with self.assertRaises(ZeroDivisionError) as cm: class NotGoingToWork: attr = Descriptor() - exc = cm.exception - self.assertRegex(str(exc), r'\bNotGoingToWork\b') - self.assertRegex(str(exc), r'\battr\b') - self.assertRegex(str(exc), r'\bDescriptor\b') - self.assertIsInstance(exc.__cause__, ZeroDivisionError) + notes = cm.exception.__notes__ + self.assertRegex(str(notes), r'\bNotGoingToWork\b') + self.assertRegex(str(notes), r'\battr\b') + self.assertRegex(str(notes), r'\bDescriptor\b') def test_set_name_wrong(self): class Descriptor: def __set_name__(self): pass - with self.assertRaises(RuntimeError) as cm: + with self.assertRaises(TypeError) as cm: class NotGoingToWork: attr = Descriptor() - exc = cm.exception - self.assertRegex(str(exc), r'\bNotGoingToWork\b') - self.assertRegex(str(exc), r'\battr\b') - self.assertRegex(str(exc), r'\bDescriptor\b') - self.assertIsInstance(exc.__cause__, TypeError) + notes = cm.exception.__notes__ + self.assertRegex(str(notes), r'\bNotGoingToWork\b') + self.assertRegex(str(notes), r'\battr\b') + self.assertRegex(str(notes), r'\bDescriptor\b') def test_set_name_lookup(self): resolved = [] diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-09-22-21-57.gh-issue-77757._Ow-u2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-22-21-57.gh-issue-77757._Ow-u2.rst new file mode 100644 index 00000000000..85c8ecf7de8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-09-22-21-57.gh-issue-77757._Ow-u2.rst @@ -0,0 +1,3 @@ +Exceptions raised in a typeobject's ``__set_name__`` method are no longer +wrapped by a :exc:`RuntimeError`. Context information is added to the +exception as a :pep:`678` note. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 995547e7915..9ea458f3039 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9137,13 +9137,15 @@ type_new_set_names(PyTypeObject *type) Py_DECREF(set_name); if (res == NULL) { - _PyErr_FormatFromCause(PyExc_RuntimeError, + _PyErr_FormatNote( "Error calling __set_name__ on '%.100s' instance %R " "in '%.100s'", Py_TYPE(value)->tp_name, key, type->tp_name); goto error; } - Py_DECREF(res); + else { + Py_DECREF(res); + } } Py_DECREF(names_to_set); diff --git a/Python/codecs.c b/Python/codecs.c index 9d800f9856c..1983f56ba20 100644 --- a/Python/codecs.c +++ b/Python/codecs.c @@ -11,6 +11,7 @@ Copyright (c) Corporation for National Research Initiatives. #include "Python.h" #include "pycore_call.h" // _PyObject_CallNoArgs() #include "pycore_interp.h" // PyInterpreterState.codec_search_path +#include "pycore_pyerrors.h" // _PyErr_FormatNote() #include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_ucnhash.h" // _PyUnicode_Name_CAPI #include @@ -382,29 +383,6 @@ PyObject *PyCodec_StreamWriter(const char *encoding, return codec_getstreamcodec(encoding, stream, errors, 3); } -static void -add_note_to_codec_error(const char *operation, - const char *encoding) -{ - PyObject *exc = PyErr_GetRaisedException(); - if (exc == NULL) { - return; - } - PyObject *note = PyUnicode_FromFormat("%s with '%s' codec failed", - operation, encoding); - if (note == NULL) { - _PyErr_ChainExceptions1(exc); - return; - } - int res = _PyException_AddNote(exc, note); - Py_DECREF(note); - if (res < 0) { - _PyErr_ChainExceptions1(exc); - return; - } - PyErr_SetRaisedException(exc); -} - /* Encode an object (e.g. a Unicode object) using the given encoding and return the resulting encoded object (usually a Python string). @@ -425,7 +403,7 @@ _PyCodec_EncodeInternal(PyObject *object, result = PyObject_Call(encoder, args, NULL); if (result == NULL) { - add_note_to_codec_error("encoding", encoding); + _PyErr_FormatNote("%s with '%s' codec failed", "encoding", encoding); goto onError; } @@ -470,7 +448,7 @@ _PyCodec_DecodeInternal(PyObject *object, result = PyObject_Call(decoder, args, NULL); if (result == NULL) { - add_note_to_codec_error("decoding", encoding); + _PyErr_FormatNote("%s with '%s' codec failed", "decoding", encoding); goto onError; } if (!PyTuple_Check(result) || diff --git a/Python/errors.c b/Python/errors.c index ab14770c6e8..0ff6a0d5985 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1200,6 +1200,33 @@ PyErr_Format(PyObject *exception, const char *format, ...) } +/* Adds a note to the current exception (if any) */ +void +_PyErr_FormatNote(const char *format, ...) +{ + PyObject *exc = PyErr_GetRaisedException(); + if (exc == NULL) { + return; + } + va_list vargs; + va_start(vargs, format); + PyObject *note = PyUnicode_FromFormatV(format, vargs); + va_end(vargs); + if (note == NULL) { + goto error; + } + int res = _PyException_AddNote(exc, note); + Py_DECREF(note); + if (res < 0) { + goto error; + } + PyErr_SetRaisedException(exc); + return; +error: + _PyErr_ChainExceptions1(exc); +} + + PyObject * PyErr_NewException(const char *name, PyObject *base, PyObject *dict) {