classification
Title: Document that tp_setattro and tp_setattr are used for deleting attributes
Type: crash Stage: resolved
Components: Documentation Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: docs@python, martin.panter, python-dev, serhiy.storchaka
Priority: high Keywords: patch

Created on 2015-11-23 06:57 by serhiy.storchaka, last changed 2016-11-30 11:11 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
setattr.patch martin.panter, 2015-11-23 23:06 For Python 3 review
setattr.2.patch martin.panter, 2015-11-30 05:43 Add setitem slots review
setattr.3.patch martin.panter, 2015-12-07 01:14 review
setattr-2.7.patch serhiy.storchaka, 2015-12-20 11:30 review
Messages (17)
msg255131 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-23 06:58
tp_setattro and tp_setattr fields of PyTypeObject are used for deleting attributes. In this case the third argument is NULL. This should be documented. Not awareness of this can cause a segmentation fail (for example see issue25691).
msg255237 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-23 23:06
Here is a patch. The same problem happens with getset descriptors:

>>> del sys.stdout._CHUNK_SIZE
SystemError: null argument to internal routine

So I also changed the documentation for “setter” descriptor functions and the tp_descr_set() method.

I only looked at the Python 3 implementation, and I understand Python 2 objects work differently in some cases, so I’m not sure if my changes are apropriate for Python 2.
msg255284 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-24 18:55
Thank you Martin. In general the patch LGTM, but I'm not expert in English writing.

There are other functions and slots with twofold purpose: PyObject_SetItem, PySequence_SetItem, PySequence_SetSlice, PyMapping_SetItemString, PySequenceMethods.sq_ass_item, PyMappingMethods.mp_ass_subscript. May be address them in this issue?

I'm not sure about documenting the deletion for PyObject_SetAttr and like. All these functions have Del-counterpart. Deleting by Set-function with NULL may be a side effect and be deprecated in future. May be worth to discuss this on Python-Dev.

For PySys_SetObject the deletion if value is NULL already is documented, but there is no PySys_DelObject.
msg255291 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-24 21:04
I agree it might be safer not to document that PyObject_SetAttr etc can delete (move that change to the tp_setattro etc method slot). I’ll also review the other functions you mentioned when I get a chance.
msg255335 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-25 11:14
Python-Dev discussion:

http://comments.gmane.org/gmane.comp.python.devel/155474
msg255609 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-30 05:43
In the python-dev thread, Nick Coghlan gave some arguments and examples where PyObject_SetAttr() is intended to be used for deletion. So I will keep my changes for PyObject_SetAttr(), and also _SetAttrString() for consistency.

My second patch documents deletion with sq_ass_item(), mp_ass_subscript(), and PySequence_SetItem().

PyObject_SetItem() does not look like it deletes items. It explicitly considers value == NULL to be an error. As a consequence, PyMapping_SetItemString() does not delete either.

PySequence_SetItem() does accept NULL to delete the item. I think this is reasonable, to keep it consistent with sq_ass_item(), so I documented it.

PySequence_SetSlice() also accepts NULL to delete the slice. But I am more reluctant to document this, because I don’t know of any slot or other code that would benefit. So I have left it as is for the time being.
msg256014 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-06 15:30
I think the conclusion of Python_Dev discussion is that it is not worth to deprecate this behavior in the code. Current behavior of  PyObject_SetAttr and PySequence_SetItem should be documented with a deprecation notice.
msg256034 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-06 21:51
Okay, I will look at making it say deleting via SetAttr etc is possible but is not the main purpose, and using DelAttr etc is recommended instead.
msg256040 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-07 01:14
setattr.3.patch changes to documenting setting as the main purpose, but mentions deleting also works but is deprecated in favour of the main deletion functions. I also clarified that the slots have to support deleting via NULL.
msg256056 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-07 10:02
The patch LGTM. Thanks Martin.
msg256096 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-08 00:07
New changeset 50711c80ff76 by Martin Panter in branch '3.5':
Issue #25701: Document C API functions that both set and delete objects
https://hg.python.org/cpython/rev/50711c80ff76

New changeset 7bb7173cd97a by Martin Panter in branch 'default':
Issue #25701: Merge set and delete documentation from 3.5
https://hg.python.org/cpython/rev/7bb7173cd97a
msg256098 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-08 00:12
I committed my patch to 3.5+. I will leave this open in case someone wants to look into how things work in 2.7. I presume things might be different in Python 2; all I know is there are two kinds of classes and objects, and the slots are a bit different (__setslice__?).
msg256762 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-20 11:30
In 2.7 all is the same except that there is the sq_ass_slice slot and the PySequence_SetSlice() function.
msg281595 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-24 01:06
The 2.7 patch looks okay to me. I confirmed that sq_ass_slice gets called to delete slices, and I presume the other stuff is similar to Python 3. Do you want to commit this to 2.7 Serhiy?
msg281607 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-24 05:58
Please do this yourself. This is mainly your patch.
msg282076 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-11-30 11:06
New changeset 83e3c863594c by Martin Panter in branch '2.7':
Issue #25701: Document that some C APIs can both set and delete items
https://hg.python.org/cpython/rev/83e3c863594c
msg282077 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-30 11:11
I made one minor change: element → slice
History
Date User Action Args
2016-11-30 11:11:14martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg282077

stage: commit review -> resolved
2016-11-30 11:06:32python-devsetmessages: + msg282076
2016-11-24 05:58:43serhiy.storchakasetmessages: + msg281607
2016-11-24 02:40:29martin.panterlinkissue28771 dependencies
2016-11-24 01:06:23martin.pantersetmessages: + msg281595
stage: patch review -> commit review
2015-12-20 11:30:33serhiy.storchakasetfiles: + setattr-2.7.patch

messages: + msg256762
2015-12-08 00:12:59martin.pantersetmessages: + msg256098
versions: - Python 3.4
2015-12-08 00:07:35python-devsetnosy: + python-dev
messages: + msg256096
2015-12-07 10:02:47serhiy.storchakasetmessages: + msg256056
2015-12-07 01:14:04martin.pantersetfiles: + setattr.3.patch

messages: + msg256040
2015-12-06 21:51:23martin.pantersetmessages: + msg256034
2015-12-06 15:30:17serhiy.storchakasetmessages: + msg256014
2015-11-30 05:43:30martin.pantersetfiles: + setattr.2.patch

messages: + msg255609
2015-11-25 11:14:36serhiy.storchakasetmessages: + msg255335
2015-11-24 21:04:06martin.pantersetmessages: + msg255291
2015-11-24 18:55:28serhiy.storchakasetmessages: + msg255284
2015-11-23 23:06:22martin.pantersetfiles: + setattr.patch

nosy: + martin.panter
messages: + msg255237

keywords: + patch
stage: needs patch -> patch review
2015-11-23 06:58:51serhiy.storchakasetstage: needs patch
2015-11-23 06:58:24serhiy.storchakasetmessages: + msg255131
2015-11-23 06:58:04serhiy.storchakaset -> (no value)
messages: - msg255130
2015-11-23 06:57:28serhiy.storchakasetpriority: normal -> high
2015-11-23 06:57:21serhiy.storchakacreate