classification
Title: PyStructSequence_NewType enhancement
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Robin.Schreiber, elemental, eric.snow, loewis, nanjekyejoannah, serhiy.storchaka
Priority: normal Keywords: pep3121

Created on 2012-08-19 20:15 by Robin.Schreiber, last changed 2019-09-16 00:55 by nanjekyejoannah.

Files
File name Uploaded Description Edit
structseq_newtype_fix.patch Robin.Schreiber, 2012-08-19 20:15 review
Messages (4)
msg168595 - (view) Author: Robin Schreiber (Robin.Schreiber) * (Python triager) Date: 2012-08-19 20:15
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.
msg228597 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-10-05 17:41
I'm uncertain as to whether or not the patch can be considered stand alone, or whether the pep3121 keyword is also relevant here.
msg231332 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-11-18 17:40
See also issue20066.
msg352500 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python committer) Date: 2019-09-16 00:55
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 issue2006 should also be closed as the Py_TPFLAGS_HEAPTYPE flag is set.
History
Date User Action Args
2019-09-16 00:55:26nanjekyejoannahsetnosy: + nanjekyejoannah
messages: + msg352500
2016-12-20 05:48:18BreamoreBoysetnosy: - BreamoreBoy
2016-12-20 05:46:17elementalsetnosy: + elemental
2014-11-18 17:40:21serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg231332
2014-10-05 17:41:08BreamoreBoysetnosy: + BreamoreBoy

messages: + msg228597
versions: + Python 3.5, - Python 3.4
2013-06-25 16:41:31eric.snowsetnosy: + eric.snow
2012-11-08 13:27:40Robin.Schreibersetkeywords: + pep3121, - patch
2012-08-19 20:15:57Robin.Schreibercreate