This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author tim.peters
Recipients mark.dickinson, serhiy.storchaka, sir-sigurd, tim.peters
Date 2018-08-14.21:27:40
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1534282060.97.0.56676864532.issue34397@psf.upfronthosting.co.za>
In-reply-to
Content
I agree there's pointless code now, but don't understand why the patch replaces it with mysterious asserts.  For example, what's the point of this?

assert(Py_SIZE(a) <= PY_SSIZE_T_MAX / sizeof(PyObject*));
assert(Py_SIZE(b) <= PY_SSIZE_T_MAX / sizeof(PyObject*));

The invariant that needs to be maintained is that the number of elements is no larger than PY_SSIZE_T_MAX, so the _relevant_ thing to assert is:

assert(Py_SIZE(a) + Py_SIZE(b) <= PY_SSIZE_T_MAX);

That in turn cries out for an explanation of why it must be true.  The asserts actually in the patch are part of that explanation, but on their own appear to be coming from Mars.  They're missing the conclusion, and rely on further unstated subtleties.

Suggest adding a comment where the structs are defined, like:

"""
Note:  Python's memory allocators return at most PY_SSIZE_T_MAX bytes.  Therefore a vector of PyObject* can contain no more than PY_SSIZE_T_MAX / sizeof(PyObject*) elements.  In particular, a PyObject* consumes at least 2 bytes, so a vector can contain no more than PY_SSIZE_T_MAX / 2 elements.  Code freely relies on that.

#if SIZEOF_VOID_P < 2
#   error "size of pointer less than 2 bytes?!"
#endif
"""
History
Date User Action Args
2018-08-14 21:27:41tim.peterssetrecipients: + tim.peters, mark.dickinson, serhiy.storchaka, sir-sigurd
2018-08-14 21:27:40tim.peterssetmessageid: <1534282060.97.0.56676864532.issue34397@psf.upfronthosting.co.za>
2018-08-14 21:27:40tim.peterslinkissue34397 messages
2018-08-14 21:27:40tim.peterscreate