python / cpython

The Python programming language
https://www.python.org
Other
63.49k stars 30.41k forks source link

Use forward compatible macro in example code for creating new type #73351

Closed methane closed 7 years ago

methane commented 7 years ago
BPO 29165
Nosy @methane, @serhiy-storchaka
PRs
  • python/cpython#211
  • 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: ```python assignee = None closed_at = created_at = labels = ['type-feature', 'docs'] title = 'Use forward compatible macro in example code for creating new type' updated_at = user = 'https://github.com/methane' ``` bugs.python.org fields: ```python activity = actor = 'methane' assignee = 'docs@python' closed = True closed_date = closer = 'methane' components = ['Documentation'] creation = creator = 'methane' dependencies = [] files = [] hgrepos = [] issue_num = 29165 keywords = [] message_count = 6.0 messages = ['284709', '288281', '288283', '288286', '288289', '288290'] nosy_count = 3.0 nosy_names = ['methane', 'docs@python', 'serhiy.storchaka'] pr_nums = ['211'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue29165' versions = ['Python 2.7'] ```

    methane commented 7 years ago

    https://docs.python.org/2.7/extending/newtypes.html#the-basics uses PyObject_HEAD_INIT for type object header.

    static PyTypeObject noddy_NoddyType = { PyObject_HEAD_INIT(NULL) 0, /ob_size/

    This code isn't compatible with Python 3. In Python 3, PyVarObject_HEAD_INIT is used instead. https://docs.python.org/3.6/extending/newtypes.html#the-basics

    static PyTypeObject noddy_NoddyType = { PyVarObject_HEAD_INIT(NULL, 0)

    This code is compatible with Python 2.

    This example code can be copy and pasted when creating new extension. If people start writing Python 2 extension, and forward port it to Python 3, this small incompatibility cause compile error.

    Let's use more forward compatible and short code for example.

    serhiy-storchaka commented 7 years ago

    PyVarObject_HEAD_INIT() is more used in Python 3 code, but the code with PyObject_HEAD_INIT() doesn't look incompatible with Python 3. I don't see a need of this change.

    PyObject_HEAD_INIT() also is used in .c files in Doc/includes/.

    methane commented 7 years ago

    but the code with PyObject_HEAD_INIT() doesn't look incompatible with Python 3.

    It's incompatible actually.

    https://github.com/python/cpython/blob/2.7/Include/object.h

    / PyObject_HEAD defines the initial segment of every PyObject. \/

    #define PyObject_HEAD_INIT(type)        \
        _PyObject_EXTRA_INIT                \
        1, type,

    https://github.com/python/cpython/blob/3.5/Include/object.h

    #define PyObject_HEAD_INIT(type)        \
        { _PyObject_EXTRA_INIT              \
        1, type },

    I noticed PyVarObject_HEAD_INIT is compatible, and simplified an extension I maintain. https://github.com/PyMySQL/mysqlclient-python/commit/2feb5ed6850a3905edf0333e0cd11ea6218f0f4f

    This small doc change helps people who writing Python 2's extension module now, and port it to Python 3 later.

    serhiy-storchaka commented 7 years ago

    Ah, now I see.

    I think sample C files in Doc/includes/ should be updated too. And it seems to me that Doc/includes/shoddy.c in Python 3 is not correct.

    methane commented 7 years ago

    New changeset 9436bbd87b7eed18dec4c32f25b88452fe282e1c by GitHub in branch '2.7': bpo-29165: doc: make extending/newtypes more Python 3 friendly (GH-211) https://github.com/python/cpython/commit/9436bbd87b7eed18dec4c32f25b88452fe282e1c

    methane commented 7 years ago

    And it seems to me that Doc/includes/shoddy.c in Python 3 is not correct.

    I created another pull request PR 215.