Skip to content
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.

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

gh-77757: replace exception wrapping by PEP-678 notes in typeobject's __set_name__ #103402

Merged
merged 6 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
===========

Expand Down
2 changes: 2 additions & 0 deletions Include/internal/pycore_pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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')."))
)

Expand Down
22 changes: 10 additions & 12 deletions Lib/test/test_subclassinit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 4 additions & 2 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
28 changes: 3 additions & 25 deletions Python/codecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 <ctype.h>
Expand Down Expand Up @@ -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).

Expand All @@ -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;
}

Expand Down Expand Up @@ -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) ||
Expand Down
27 changes: 27 additions & 0 deletions Python/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down