-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
bpo-38530: Offer suggestions on AttributeError #16856
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
Changes from 3 commits
c9c85b5
61d510b
dbd88f0
a3dd16a
c760303
71e8766
45061e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,12 @@ typedef struct { | |
PyObject *value; | ||
} PyStopIterationObject; | ||
|
||
typedef struct { | ||
PyException_HEAD | ||
PyObject *obj; | ||
PyObject *name; | ||
} PyAttributeErrorObject; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://www.python.org/dev/peps/pep-0473/ was rejected because it was too broad, but this PR adds a single exception, which sounds ok according to the resolution: https://mail.python.org/pipermail/python-dev/2019-March/156692.html My main worry is the risk of creating "more" and "worse" exception cycles, but Pablo says that it sounds unlikely: https://bugs.python.org/issue38530#msg354975 |
||
|
||
/* Compatibility typedefs */ | ||
typedef PyOSErrorObject PyEnvironmentErrorObject; | ||
#ifdef MS_WINDOWS | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
#include "Python.h" | ||
|
||
#ifndef Py_INTERNAL_SUGGESTIONS_H | ||
#define Py_INTERNAL_SUGGESTIONS_H | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to keep the header file, you should add extern "C". Third party C++ code is allowed to use it. |
||
|
||
#ifndef Py_BUILD_CORE | ||
# error "this header requires Py_BUILD_CORE define" | ||
#endif | ||
|
||
int _Py_Offer_Suggestions(PyObject* exception, PyObject* value); | ||
|
||
#endif /* !Py_INTERNAL_SUGGESTIONS_H */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
When printing :exc:`AttributeError`, :c:func:`PyErr_Display` will offer | ||
suggestions of simmilar attribute names in the object that the exception was | ||
raised from. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
#include "pycore_pystate.h" // _PyThreadState_GET() | ||
#include "pycore_symtable.h" // PySTEntry_Type | ||
#include "pycore_unionobject.h" // _Py_UnionType | ||
#include "pycore_suggestions.h" | ||
#include "frameobject.h" | ||
#include "interpreteridobject.h" | ||
|
||
|
@@ -884,29 +885,60 @@ _PyObject_SetAttrId(PyObject *v, _Py_Identifier *name, PyObject *w) | |
return result; | ||
} | ||
|
||
static inline int | ||
set_attribute_error_context(PyObject* v, PyObject* name) | ||
{ | ||
assert(PyErr_Occurred()); | ||
_Py_IDENTIFIER(name); | ||
_Py_IDENTIFIER(obj); | ||
// Intercept AttributeError exceptions and augment them to offer | ||
// suggestions later. | ||
if (PyErr_ExceptionMatches(PyExc_AttributeError)){ | ||
PyObject *type, *value, *traceback; | ||
PyErr_Fetch(&type, &value, &traceback); | ||
PyErr_NormalizeException(&type, &value, &traceback); | ||
if (PyErr_GivenExceptionMatches(value, PyExc_AttributeError) && | ||
(_PyObject_SetAttrId(value, &PyId_name, name) || | ||
_PyObject_SetAttrId(value, &PyId_obj, v))) { | ||
return 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really useful to report setattr() failed? the callers don't seem to care. |
||
} | ||
PyErr_Restore(type, value, traceback); | ||
} | ||
return 0; | ||
} | ||
|
||
PyObject * | ||
PyObject_GetAttr(PyObject *v, PyObject *name) | ||
{ | ||
PyTypeObject *tp = Py_TYPE(v); | ||
PyObject* result = NULL; | ||
|
||
if (!PyUnicode_Check(name)) { | ||
PyErr_Format(PyExc_TypeError, | ||
"attribute name must be string, not '%.200s'", | ||
Py_TYPE(name)->tp_name); | ||
return NULL; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: I like to only declare variables just before they are initialized (here) thanks to C99.
|
||
if (tp->tp_getattro != NULL) | ||
return (*tp->tp_getattro)(v, name); | ||
if (tp->tp_getattr != NULL) { | ||
if (tp->tp_getattro != NULL) { | ||
result = (*tp->tp_getattro)(v, name); | ||
} | ||
else if (tp->tp_getattr != NULL) { | ||
const char *name_str = PyUnicode_AsUTF8(name); | ||
if (name_str == NULL) | ||
if (name_str == NULL) { | ||
return NULL; | ||
return (*tp->tp_getattr)(v, (char *)name_str); | ||
} | ||
result = (*tp->tp_getattr)(v, (char *)name_str); | ||
} | ||
PyErr_Format(PyExc_AttributeError, | ||
"'%.50s' object has no attribute '%U'", | ||
tp->tp_name, name); | ||
return NULL; | ||
else { | ||
PyErr_Format(PyExc_AttributeError, | ||
"'%.50s' object has no attribute '%U'", | ||
tp->tp_name, name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe only set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is code from before, I prefer to not change it in case we introduce some unwanted change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok. It was just a suggestion, you can ignore it. it's just a coding style preference ;-) |
||
} | ||
|
||
if (result == NULL) { | ||
set_attribute_error_context(v, name); | ||
} | ||
return result; | ||
} | ||
|
||
int | ||
|
@@ -1165,6 +1197,8 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) | |
PyErr_Format(PyExc_AttributeError, | ||
"'%.50s' object has no attribute '%U'", | ||
tp->tp_name, name); | ||
|
||
set_attribute_error_context(obj, name); | ||
return 0; | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.