There was a problem hiding this comment.
Looks great :)
Sorry, something went wrong.
There was a problem hiding this comment.
Looks really helpful, thanks a lot @pablogsal!
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit b1546e5 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 4925719 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
Sorry, something went wrong.
| if (!locals) { | ||
| goto error; | ||
| } | ||
| PyObject* self = PyDict_GetItem(locals, &_Py_ID(self)); /* borrowed */ |
There was a problem hiding this comment.
Do not use PyDict_GetItem. It is broken by design.
Sorry, something went wrong.
There was a problem hiding this comment.
What should we use instead for both?
Sorry, something went wrong.
There was a problem hiding this comment.
PyDict_GetItemWithError and PyObject_GetAttr if limited by public API. It is suggested in the documentation. But _PyObject_LookupAttr is more convenient in the latter case.
Sorry, something went wrong.
| goto error; | ||
| } | ||
|
|
||
| if (PyObject_HasAttr(self, name)) { |
There was a problem hiding this comment.
Do not use PyObject_HasAttr. It is broken by design.
Sorry, something went wrong.
|
I'm surprised this uses hasattr under the covers. A simple benign name error could cause side effects at run time. >>> class Test:
... def __getattr__(self, name):
... if name == "foo":
... raise SystemExit("The end.")
... def bar(self):
... foo
...
... Test().bar()
...
The end.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.