Document that tp_setattro and tp_setattr are used for deleting attributes

[serhiy-storchaka]

BPO	25701
Nosy	@vadmium, @serhiy-storchaka
Files	setattr.patch: For Python 3 setattr.2.patch: Add setitem slots setattr.3.patch setattr-2.7.patch

^{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:

assignee = None
closed_at = <Date 2016-11-30.11:11:14.283>
created_at = <Date 2015-11-23.06:57:21.485>
labels = ['type-crash', 'docs']
title = 'Document that tp_setattro and tp_setattr are used for deleting attributes'
updated_at = <Date 2016-11-30.11:11:14.282>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2016-11-30.11:11:14.282>
actor = 'martin.panter'
assignee = 'docs@python'
closed = True
closed_date = <Date 2016-11-30.11:11:14.283>
closer = 'martin.panter'
components = ['Documentation']
creation = <Date 2015-11-23.06:57:21.485>
creator = 'serhiy.storchaka'
dependencies = []
files = ['41143', '41194', '41259', '41370']
hgrepos = []
issue_num = 25701
keywords = ['patch']
message_count = 17.0
messages = ['255131', '255237', '255284', '255291', '255335', '255609', '256014', '256034', '256040', '256056', '256096', '256098', '256762', '281595', '281607', '282076', '282077']
nosy_count = 4.0
nosy_names = ['docs@python', 'python-dev', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue25701'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[serhiy-storchaka]

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 bpo-25691).

[vadmium]

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.

[serhiy-storchaka]

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.

[vadmium]

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.

[serhiy-storchaka]

Python-Dev discussion:

http://comments.gmane.org/gmane.comp.python.devel/155474

[vadmium]

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.

[serhiy-storchaka]

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.

[vadmium]

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.

[vadmium]

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.

[serhiy-storchaka]

The patch LGTM. Thanks Martin.

[python-dev]

New changeset 50711c80ff76 by Martin Panter in branch '3.5':
Issue bpo-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 bpo-25701: Merge set and delete documentation from 3.5
https://hg.python.org/cpython/rev/7bb7173cd97a

[vadmium]

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?).

[serhiy-storchaka]

In 2.7 all is the same except that there is the sq_ass_slice slot and the PySequence_SetSlice() function.

[vadmium]

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?

[serhiy-storchaka]

Please do this yourself. This is mainly your patch.

[python-dev]

New changeset 83e3c863594c by Martin Panter in branch '2.7':
Issue bpo-25701: Document that some C APIs can both set and delete items
https://hg.python.org/cpython/rev/83e3c863594c

[vadmium]

I made one minor change: element → slice