Slots like PyTypeObject.tp_setattr, PySequenceMethods.sq_ass_item, PyMappingMethods.mp_ass_subscript are used not only for setting attribute/item value, but also for deleting attribute/item (if value is NULL). This is not documented and should be. [1] Correspondingly public API functions like PyObject_SetAttr, PyObject_SetItem, PySequence_SetItem, PySequence_SetSlice, PyMapping_SetItemString can be used for deleting. But all these functions have special counterparts for deleting: PyObject_DelAttr etc. The question is wherever deleting ability of Set-functions is intentional, should we document this or deprecate and then delete? [1] http://bugs.python.org/issue25701
Ooooh, that's probably really old code. I guess for the slots the reasoning is to save on slots. For the public functions, alas it will be hard to know if anyone is depending on it, even if it's undocumented. Perhaps add a deprecation warning to these if the value is NULL for one release cycle? On Tue, Nov 24, 2015 at 1:21 PM, Serhiy Storchaka <storchaka@gmail.com> wrote:
Slots like PyTypeObject.tp_setattr, PySequenceMethods.sq_ass_item, PyMappingMethods.mp_ass_subscript are used not only for setting attribute/item value, but also for deleting attribute/item (if value is NULL). This is not documented and should be. [1] Correspondingly public API functions like PyObject_SetAttr, PyObject_SetItem, PySequence_SetItem, PySequence_SetSlice, PyMapping_SetItemString can be used for deleting. But all these functions have special counterparts for deleting: PyObject_DelAttr etc.
The question is wherever deleting ability of Set-functions is intentional, should we document this or deprecate and then delete?
[1] http://bugs.python.org/issue25701
_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/guido%40python.org
-- --Guido van Rossum (python.org/~guido)
On 25 November 2015 at 07:33, Guido van Rossum <guido@python.org> wrote:
Ooooh, that's probably really old code. I guess for the slots the reasoning is to save on slots. For the public functions, alas it will be hard to know if anyone is depending on it, even if it's undocumented. Perhaps add a deprecation warning to these if the value is NULL for one release cycle?
I did a quick scan for "PyObject_SetAttr", and it turns out PyObject_DelAttr is only a convenience macro for calling PyObject_SetAttr with NULL as the value argument. bltinmodule.c and ceval.c also both include direct calls to PyObject_SetAttr with "(PyObject *)NULL" as the value argument. Investigating some of the uses that passed a variable as the value argument, one case is the weakref proxy implementation, which uses PyObject_SetAttr on the underlying object in its implementation of the setattr slot in the proxy. So it looks to me like replicating the NULL-handling behaviour of the slots in the public Set* APIs was intentional, and it's just the documentation of that detail that was missed (since most folks presumably use the Del* convenience APIs instead). Regards, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 25.11.15 08:39, Nick Coghlan wrote:
On 25 November 2015 at 07:33, Guido van Rossum <guido@python.org> wrote:
Ooooh, that's probably really old code. I guess for the slots the reasoning is to save on slots. For the public functions, alas it will be hard to know if anyone is depending on it, even if it's undocumented. Perhaps add a deprecation warning to these if the value is NULL for one release cycle?
I did a quick scan for "PyObject_SetAttr", and it turns out PyObject_DelAttr is only a convenience macro for calling PyObject_SetAttr with NULL as the value argument. bltinmodule.c and ceval.c also both include direct calls to PyObject_SetAttr with "(PyObject *)NULL" as the value argument.
Investigating some of the uses that passed a variable as the value argument, one case is the weakref proxy implementation, which uses PyObject_SetAttr on the underlying object in its implementation of the setattr slot in the proxy.
So it looks to me like replicating the NULL-handling behaviour of the slots in the public Set* APIs was intentional, and it's just the documentation of that detail that was missed (since most folks presumably use the Del* convenience APIs instead).
I'm not sure. This looks rather as implementation detail to me. There cases found by you are the only cases in the core/stdlib that call PyObject_SetAttr with third argument is NULL. Tests are passed after replacing Set* functions with Del* functions in these cases and making Set* functions to reject value is NULL. [1] Wouldn't be worth to deprecate deleting with Set* functions? Neither other abstract Set* APIs, not concrete Set* APIs don't support deleting. Deleting with Set* API can be unintentional and hide a bug. [1] http://bugs.python.org/issue25773
On 2 December 2015 at 01:50, Serhiy Storchaka <storchaka@gmail.com> wrote:
On 25.11.15 08:39, Nick Coghlan wrote:
So it looks to me like replicating the NULL-handling behaviour of the slots in the public Set* APIs was intentional, and it's just the documentation of that detail that was missed (since most folks presumably use the Del* convenience APIs instead).
I'm not sure. This looks rather as implementation detail to me. There cases found by you are the only cases in the core/stdlib that call PyObject_SetAttr with third argument is NULL. Tests are passed after replacing Set* functions with Del* functions in these cases and making Set* functions to reject value is NULL. [1]
Which means at the very least, folks relying on the current behaviour are relying on untested functionality, and would be better of switching to the tested APIs regardless of what happens on the deprecation front.
Wouldn't be worth to deprecate deleting with Set* functions? Neither other abstract Set* APIs, not concrete Set* APIs don't support deleting. Deleting with Set* API can be unintentional and hide a bug.
Since the behaviour is currently neither documented not tested, and it doesn't raise any new Python 2/3 migation issues, I don't personally mind deprecating the "delete via set" APIs for 3.6 - as you say, having "set this field/attribute to this value" occasionally mean "delete this field/attribute" if a pointer is NULL offers a surprising second way to do something that already has a more explicit spelling. Regards, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
participants (3)
-
Guido van Rossum -
Nick Coghlan -
Serhiy Storchaka