Repository navigation
PyWeakref_NewRef docs are misleading #80384
Description
Activity
tekknolagi commented
on Mar 6, 2019 tekknolagimannequinMannequinAuthorThe docs for
PyWeakref_NewRefstate "if callback is not callable, None, or NULL, this will return NULL and raise TypeError". It does not appear as though there is a callable check for the callback.Windsooon commented
on Mar 6, 2019 WindsooonmannequinMannequinYes, Maxwell. I guess the docs are misleading, the code locate in https://github.com/python/cpython/blob/master/Objects/weakrefobject.c#L748
if (callback == Py_None) callback = NULL; if (callback == NULL) /* return existing weak reference if it exists */ result = ref; if (result != NULL) Py_INCREF(result); else { ... }
However, I'm not sure we should fix the docs or the code here.
tekknolagi commented
on Mar 6, 2019 tekknolagimannequinMannequinAuthorNewProxy checks if it's callable, so I suppose the code should be fixed.
On Wed, Mar 6, 2019, 03:32 Windson Yang <report@bugs.python.org> wrote:
Windson Yang <wiwindson@outlook.com> added the comment:
Yes, Maxwell. I guess the docs are misleading, the code locate in
https://github.com/python/cpython/blob/master/Objects/weakrefobject.c#L748if (callback == Py_None)
callback = NULL;
if (callback == NULL)
/* return existing weak reference if it exists */
result = ref;
if (result != NULL)
Py_INCREF(result);
else {
...
}However, I'm not sure we should fix the docs or the code here.
----------
nosy: +Windson Yang
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue36203\>
Windsooon commented
on Mar 7, 2019 WindsooonmannequinMannequinIt looks to me the fix is easy, we just will return NULL and raise TypeError when the callback is not callable, None, or NULL. I'm not an expert in C, but I would love to create a PR for it if you don't have time.
tekknolagi commented
on Mar 7, 2019 tekknolagimannequinMannequinAuthorI can likely do it tomorrow. If not I'll update this.
On Thu, Mar 7, 2019, 00:20 Windson Yang <report@bugs.python.org> wrote:
Windson Yang <wiwindson@outlook.com> added the comment:
It looks to me the fix is easy, we just will return NULL and raise
TypeError when the callback is not callable, None, or NULL. I'm not an
expert in C, but I would love to create a PR for it if you don't have time.----------
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue36203\>
Please add unit tests to the patch.
6 remaining items
#151130 added a base for new C API tests. Now it is trivial to add new tests.
#151145 adds checks for callback in
weakref.ref(),weakref.proxy(),PyWeakref_NewRef()andPyWeakref_NewProxy().
#151146 fixes docs forPyWeakref_NewRef()andPyWeakref_NewProxy()in Python 3.15 (will be backported to 3.14 and 3.13).
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs