PyStructSequence_NewType enhancement

[RobinSchreiber]

BPO	15729
Nosy	@loewis, @ericsnowcurrently, @serhiy-storchaka, @markazmierczak, @nanjekyejoannah
Files	structseq_newtype_fix.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 = None
created_at = <Date 2012-08-19.20:15:56.996>
labels = ['interpreter-core', 'type-feature']
title = 'PyStructSequence_NewType enhancement'
updated_at = <Date 2019-09-16.00:55:26.833>
user = 'https://bugs.python.org/RobinSchreiber'

bugs.python.org fields:

activity = <Date 2019-09-16.00:55:26.833>
actor = 'nanjekyejoannah'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2012-08-19.20:15:56.996>
creator = 'Robin.Schreiber'
dependencies = []
files = ['26901']
hgrepos = []
issue_num = 15729
keywords = ['pep3121']
message_count = 4.0
messages = ['168595', '228597', '231332', '352500']
nosy_count = 6.0
nosy_names = ['loewis', 'eric.snow', 'serhiy.storchaka', 'Robin.Schreiber', 'elemental', 'nanjekyejoannah']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue15729'
versions = ['Python 3.5']

[RobinSchreiber]

To create a HeapType from Structseq description, there is the helpful, yet undocumented PyStructSequence_NewType Method, which can do just that.
Until now, this method solely allocates some generic TypeObject on which it then performs PyStructSequence_InitType().

I have found that this is far from enough, as for one the flags of the type are not set appropriately and neither ht_name nor ht_qualname are set (which is as far as I know required of a proper Heaptype).
I have now added this missing initialization of the fields to PyStructSequence_NewType().

I have also changed the previous definition of PyStructSequence_InitType, by extracting a new Method _PyStructSequence_InitTypeWithFlags. This initializes the given type with the flags variable passed to the function.
This method extraction is needed, as we can not alter the semantics of InitType itself, yet need some way to initialize a SequenceType with Heaptype-flags, without having too much duplicate code.

[BreamoreBoy]

I'm uncertain as to whether or not the patch can be considered stand alone, or whether the PEP-3121 keyword is also relevant here.

[serhiy-storchaka]

See also bpo-20066.

[nanjekyejoannah]

All enhancements specified in this issue have since been done. PyStructSequence_NewType() calls PyType_FromSpecWithBases() which sets the needed flags and even the qualname:

PyObject *
PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
{
    PyHeapTypeObject *res;
    PyMemberDef *memb;
    PyObject *modname;
    PyTypeObject *type, *base;

    PyType_Slot *slot;
    Py_ssize_t nmembers;
    char *s, *res_start;

    nmembers = 0;
    for (slot = spec->slots; slot->slot; slot++) {
        if (slot->slot == Py_tp_members) {
            nmembers = 0;
            for (memb = slot->pfunc; memb->name != NULL; memb++) {
                nmembers++;
            }
        }
    }

    res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nmembers);
    if (res == NULL)
        return NULL;
    res_start = (char*)res;

    if (spec->name == NULL) {
        PyErr_SetString(PyExc_SystemError,
                        "Type spec does not define the name field.");
        goto fail;
    }

/* Set the type name and qualname */
s = strrchr(spec->name, '.');
if (s == NULL)
    s = (char*)spec->name;
else
    s++;

    type = &res->ht_type;
    /* The flags must be initialized early, before the GC traverses us */
    type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
    res->ht_name = PyUnicode_FromString(s);
    if (!res->ht_name)
        goto fail;
    res->ht_qualname = res->ht_name;
    Py_INCREF(res->ht_qualname);
    type->tp_name = spec->name;

    /* Adjust for empty tuple bases */
    if (!bases) {
        base = &PyBaseObject_Type;
        /* See whether Py_tp_base(s) was specified */
        for (slot = spec->slots; slot->slot; slot++) {
            if (slot->slot == Py_tp_base)
                base = slot->pfunc;
            else if (slot->slot == Py_tp_bases) {
                bases = slot->pfunc;
                Py_INCREF(bases);
            }
        }
        if (!bases)
            bases = PyTuple_Pack(1, base);
        if (!bases)
            goto fail;
    }
    else
        Py_INCREF(bases);

    /* Calculate best base, and check that all bases are type objects */
    base = best_base(bases);
    if (base == NULL) {
        goto fail;
    }
    if (!PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) {
        PyErr_Format(PyExc_TypeError,
                     "type '%.100s' is not an acceptable base type",
                     base->tp_name);
        goto fail;
    }

    /* Initialize essential fields */
    type->tp_as_async = &res->as_async;
    type->tp_as_number = &res->as_number;
    type->tp_as_sequence = &res->as_sequence;
    type->tp_as_mapping = &res->as_mapping;
    type->tp_as_buffer = &res->as_buffer;
    /* Set tp_base and tp_bases */
    type->tp_bases = bases;
    bases = NULL;
    Py_INCREF(base);
    type->tp_base = base;

type->tp_basicsize = spec->basicsize;
type->tp_itemsize = spec->itemsize;

    for (slot = spec->slots; slot->slot; slot++) {
        if (slot->slot < 0
            || (size_t)slot->slot >= Py_ARRAY_LENGTH(slotoffsets)) {
            PyErr_SetString(PyExc_RuntimeError, "invalid slot offset");
            goto fail;
        }
        else if (slot->slot == Py_tp_base || slot->slot == Py_tp_bases) {
            /* Processed above */
            continue;
        }
        else if (slot->slot == Py_tp_doc) {
            /* For the docstring slot, which usually points to a static string
               literal, we need to make a copy */
            const char *old_doc = _PyType_DocWithoutSignature(type->tp_name, slot->pfunc);
            size_t len = strlen(old_doc)+1;
            char *tp_doc = PyObject_MALLOC(len);
            if (tp_doc == NULL) {
                type->tp_doc = NULL;
                PyErr_NoMemory();
                goto fail;
            }
            memcpy(tp_doc, old_doc, len);
            type->tp_doc = tp_doc;
        }
        else if (slot->slot == Py_tp_members) {
            /* Move the slots to the heap type itself */
            size_t len = Py_TYPE(type)->tp_itemsize * nmembers;
            memcpy(PyHeapType_GET_MEMBERS(res), slot->pfunc, len);
            type->tp_members = PyHeapType_GET_MEMBERS(res);
        }
        else {
            /* Copy other slots directly */
            *(void**)(res_start + slotoffsets[slot->slot]) = slot->pfunc;
        }
    }
    if (type->tp_dealloc == NULL) {
        /* It's a heap type, so needs the heap types' dealloc.
           subtype_dealloc will call the base type's tp_dealloc, if
           necessary. */
        type->tp_dealloc = subtype_dealloc;
    }

if (PyType_Ready(type) < 0)
    goto fail;

    if (type->tp_dictoffset) {
        res->ht_cached_keys = _PyDict_NewKeysForClass();
    }

    /* Set type.__module__ */
    s = strrchr(spec->name, '.');
    if (s != NULL) {
        int err;
        modname = PyUnicode_FromStringAndSize(
                spec->name, (Py_ssize_t)(s - spec->name));
        if (modname == NULL) {
            goto fail;
        }
        err = _PyDict_SetItemId(type->tp_dict, &PyId___module__, modname);
        Py_DECREF(modname);
        if (err != 0)
            goto fail;
    } else {
        if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
                "builtin type %.200s has no __module__ attribute",
                spec->name))
            goto fail;
    }

return (PyObject*)res;

 fail:
    Py_DECREF(res);
    return NULL;
}

So I think this should be closed.

I think bpo-2006 should also be closed as the Py_TPFLAGS_HEAPTYPE flag is set.

[kumaraditya303]

Closing as this is fixed.