From d7156e65ab4e87017974da9a7dc8aaccb273ea36 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sun, 9 Apr 2023 22:09:03 +0100 Subject: [PATCH 1/4] gh-77757: replace exception wrapping by PEP-678 notes in typeobject's __set_name__ --- Lib/test/test_functools.py | 4 ++-- Lib/test/test_subclassinit.py | 22 ++++++++++------------ Objects/typeobject.c | 19 +++++++++++++++++-- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 57db96d37ee369..af286052a7d560 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2980,7 +2980,7 @@ class MyClass(metaclass=MyMeta): 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 @@ def a(self): 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 0ad7d17fbd4ddd..310473a4a2fe58 100644 --- a/Lib/test/test_subclassinit.py +++ b/Lib/test/test_subclassinit.py @@ -134,30 +134,28 @@ class Descriptor: 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/Objects/typeobject.c b/Objects/typeobject.c index 995547e7915f77..3fd437e52bc2b9 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9137,13 +9137,28 @@ type_new_set_names(PyTypeObject *type) Py_DECREF(set_name); if (res == NULL) { - _PyErr_FormatFromCause(PyExc_RuntimeError, + /* Add note to the exception */ + PyObject *exc = PyErr_GetRaisedException(); + assert(exc != NULL); + PyObject *note = PyUnicode_FromFormat( "Error calling __set_name__ on '%.100s' instance %R " "in '%.100s'", Py_TYPE(value)->tp_name, key, type->tp_name); + + if (note) { + int res = _PyException_AddNote(exc, note); + Py_DECREF(note); + if (res == 0) { + PyErr_SetRaisedException(exc); + goto error; + } + } + _PyErr_ChainExceptions1(exc); goto error; } - Py_DECREF(res); + else { + Py_DECREF(res); + } } Py_DECREF(names_to_set); From beaf88a188fe46f3e6be4b161d3ab1264cd86ae0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sun, 9 Apr 2023 22:24:32 +0100 Subject: [PATCH 2/4] add news --- Doc/whatsnew/3.12.rst | 4 ++++ .../2023-04-09-22-21-57.gh-issue-77757._Ow-u2.rst | 3 +++ 2 files changed, 7 insertions(+) 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 651caed864fef7..8a7bef326e4e4f 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/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 00000000000000..85c8ecf7de8d1b --- /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. From 0068e9c7b6b402d68a2027d3162a5f2032997485 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 10 Apr 2023 18:43:06 +0100 Subject: [PATCH 3/4] refactor common code into a new _PyErr_FormatNote function --- Include/internal/pycore_pyerrors.h | 2 ++ Objects/typeobject.c | 16 +--------------- Python/codecs.c | 28 +++------------------------- Python/errors.c | 27 +++++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 40 deletions(-) diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index 1bb4a9aa103898..4620a269644917 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/Objects/typeobject.c b/Objects/typeobject.c index 3fd437e52bc2b9..b03b1214969151 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9137,24 +9137,10 @@ type_new_set_names(PyTypeObject *type) Py_DECREF(set_name); if (res == NULL) { - /* Add note to the exception */ - PyObject *exc = PyErr_GetRaisedException(); - assert(exc != NULL); - PyObject *note = PyUnicode_FromFormat( + _PyErr_FormatNote( "Error calling __set_name__ on '%.100s' instance %R " "in '%.100s'", Py_TYPE(value)->tp_name, key, type->tp_name); - - if (note) { - int res = _PyException_AddNote(exc, note); - Py_DECREF(note); - if (res == 0) { - PyErr_SetRaisedException(exc); - goto error; - } - } - _PyErr_ChainExceptions1(exc); - goto error; } else { Py_DECREF(res); diff --git a/Python/codecs.c b/Python/codecs.c index 9d800f9856c2f7..1983f56ba204c1 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 ab14770c6e810c..0ff6a0d5985f0f 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) { From 31d4202998794fdb6bbc0ebfead4eacb1bf63075 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 10 Apr 2023 18:45:31 +0100 Subject: [PATCH 4/4] put back 'goto error' --- Objects/typeobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b03b1214969151..9ea458f30394e3 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9141,6 +9141,7 @@ type_new_set_names(PyTypeObject *type) "Error calling __set_name__ on '%.100s' instance %R " "in '%.100s'", Py_TYPE(value)->tp_name, key, type->tp_name); + goto error; } else { Py_DECREF(res);