Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyMemberDef missing in limited API / Deprecate structmember.h #47146

Closed
benjaminp opened this issue May 16, 2008 · 22 comments
Closed

PyMemberDef missing in limited API / Deprecate structmember.h #47146

benjaminp opened this issue May 16, 2008 · 22 comments
Assignees
Labels
3.10 only security fixes docs Documentation in the Doc dir interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API type-feature A feature request or enhancement

Comments

@benjaminp
Copy link
Contributor

benjaminp commented May 16, 2008

BPO 2897
Nosy @loewis, @smontanaro, @birkenfeld, @rhettinger, @abalkin, @vstinner, @benjaminp, @berkerpeksag, @serhiy-storchaka, @matrixise, @erlend-aasland, @MatzeB
PRs
  • bpo-2897: Make PyMemberDef part of stable ABI; deprecate structmember.h #20462
  • bpo-41861: Clean up sqlite3 header files wrt. PEP 384 #22419
  • Dependencies
  • bpo-24065: Outdated *_RESTRICTED flags in structmember.h
  • bpo-28349: Issues with PyMemberDef flags
  • Files
  • issue2897.diff
  • issue2897-docs-3x.diff
  • 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 = 'https://github.com/abalkin'
    closed_at = None
    created_at = <Date 2008-05-16.22:45:11.685>
    labels = ['interpreter-core', 'expert-C-API', 'type-feature', '3.10', 'docs']
    title = 'PyMemberDef missing in limited API / Deprecate structmember.h'
    updated_at = <Date 2020-09-26.20:51:31.703>
    user = 'https://github.com/benjaminp'

    bugs.python.org fields:

    activity = <Date 2020-09-26.20:51:31.703>
    actor = 'erlendaasland'
    assignee = 'belopolsky'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation', 'Interpreter Core', 'C API']
    creation = <Date 2008-05-16.22:45:11.685>
    creator = 'benjamin.peterson'
    dependencies = ['24065', '28349']
    files = ['44943', '44976']
    hgrepos = []
    issue_num = 2897
    keywords = ['patch']
    message_count = 21.0
    messages = ['66972', '67028', '67062', '79242', '79244', '277971', '277973', '277976', '277982', '277985', '277986', '277988', '278136', '342468', '370098', '370100', '370101', '370104', '370106', '370107', '370138']
    nosy_count = 15.0
    nosy_names = ['loewis', 'skip.montanaro', 'georg.brandl', 'rhettinger', 'belopolsky', 'vstinner', 'benjamin.peterson', 'Arfrever', 'herzbube', 'docs@python', 'berker.peksag', 'serhiy.storchaka', 'matrixise', 'erlendaasland', 'Matthias Braun']
    pr_nums = ['20462', '22419']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue2897'
    versions = ['Python 3.10']

    @benjaminp
    Copy link
    Contributor Author

    As the comment in descrobject.c says:

    /* Why is this not included in Python.h? */

    @benjaminp benjaminp added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) labels May 16, 2008
    @birkenfeld
    Copy link
    Member

    We could include it in Py3k.

    @abalkin
    Copy link
    Member

    abalkin commented May 19, 2008

    Note that structmember.h pollutes global namespace with macros that do not
    have conventional Py_ or PY_ prefix. READONLY and RESTRICTED macros seem
    to be most likely to conflict with other code. I would be -0 on including tructmember.h in Python.h if flags macros are not properly renamed. +0
    otherwise. T_* macros are probably OK, but T prefix is reminiscent of
    popular (in some circles) Taligent naming conventions:

    http://pcroot.cern.ch/TaligentDocs/TaligentOnline/DocumentRoot/1.0/Docs/bo
    oks/WM/WM_63.html#HEADING77

    @gvanrossum gvanrossum removed their assignment Jan 6, 2009
    @rhettinger
    Copy link
    Contributor

    Martin, do you want to make the call on this one?

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jan 6, 2009

    I agree with Alexander; the header shouldn't be included into Python.h
    as-is.

    I would propose to eliminate it eventually, with the following steps:

    1. move PyMemberDef and the function declarations into object.h
    2. (simultaneously) introduce properly-prefixed macros in object.h
    3. deprecate structmember.h
    4. remove it

    @loewis loewis mannequin removed their assignment Feb 2, 2009
    @abalkin abalkin added the 3.7 (EOL) end of life label Oct 3, 2016
    @abalkin
    Copy link
    Member

    abalkin commented Oct 3, 2016

    I am attaching a patch that implements steps 1 and 2 of Martin's plan. There are over 50 files that include structmember.h. I am not sure it is worth the trouble to update all those files before structmember.h is actually removed. If we agree that this is the right way forward, I'll make the necessary changes to the docs.

    @abalkin
    Copy link
    Member

    abalkin commented Oct 3, 2016

    I would also like this opportunity to rename T_PYSSIZET to something more readable: maybe PY_T_PY_SSIZE_T or PY_T_SSIZE_T.

    @serhiy-storchaka
    Copy link
    Member

    Please don't forget to use "hg copy" for creating object.h from structmember.h. This preserves the history.

    structmember.h should be implemented using object.h. Include object.h and add aliases.

    Only READONLY flag is used in 3.x (bpo-28349). Other flags can be removed.

    @abalkin
    Copy link
    Member

    abalkin commented Oct 3, 2016

    As explained in bpo-24065, READ_RESTRICTED, PY_WRITE_RESTRICTED and RESTRICTED flags were used for "restricted mode" in Python 2. I don't think we would want to preserve these as we move the rest to object.h.

    @serhiy-storchaka
    Copy link
    Member

    PY_T_PY_SSIZE_T is not much readable than PY_T_PYSSIZET. I think it can be just PY_T_SSIZE.

    @serhiy-storchaka
    Copy link
    Member

    And please don't miss to fix the documentation in 2.7 and 3.5-3.6.

    @abalkin
    Copy link
    Member

    abalkin commented Oct 3, 2016

    Changed the title to reflect the way forward and added affected versions to remember to update the documentation. See bpo-28349 and bpo-24065 for details.

    @abalkin abalkin added the docs Documentation in the Doc dir label Oct 3, 2016
    @abalkin abalkin changed the title include structmember.h in Python.h Deprecate structmember.h Oct 3, 2016
    @abalkin
    Copy link
    Member

    abalkin commented Oct 5, 2016

    I am attaching a proposed doc patch for Python 3.5+. For 2.7 we should probably just add a versionchanged note explaining "restricted mode" has been deprecated in Python 2.3.

    @abalkin abalkin assigned abalkin and unassigned docspython Oct 10, 2016
    @matrixise
    Copy link
    Member

    I move this issue to master (3.8)

    @matrixise matrixise added 3.8 only security fixes and removed 3.7 (EOL) end of life labels May 14, 2019
    @MatzeB
    Copy link
    Mannequin

    MatzeB mannequin commented May 27, 2020

    This wasn't mentioned before: Having PyMemberDef part of the structmember.h is a big problem for users of PEP384/limited API, because structmember.h is not part of it.

    Which results in the odd situation that Py_tp_members or PyDescr_NewMember() are part of the limited API but technically you cannot use it because you are not supposed to include headers that are not part of Python.h.

    The proposed patch here, would fix this!

    @MatzeB MatzeB mannequin added the topic-C-API label May 27, 2020
    @MatzeB MatzeB mannequin added 3.10 only security fixes and removed 3.8 only security fixes labels May 27, 2020
    @vstinner
    Copy link
    Member

    The proposed patch here, would fix this!

    The issue title is misleading, it says "Deprecate structmember.h". Is the plan still to deprecate it? Or to make it usable in the limited C API? Please update the title.

    @vstinner
    Copy link
    Member

    Note that structmember.h pollutes global namespace with macros that do not have conventional Py_ or PY_ prefix. READONLY and RESTRICTED macros seem to be most likely to conflict with other code.

    One small enhance would be to add such prefix when Py_LIMITED_API is defined.

    @MatzeB
    Copy link
    Mannequin

    MatzeB mannequin commented May 27, 2020

    The issue title is misleading, it says "Deprecate structmember.h". Is the plan still to deprecate it? Or to make it usable in the limited C API? Please update the title.

    As far as I understand it: The attached diff, moves the interesting declaration to object.h solving the limited API problem. And only leaves structmember.h around for backward compatibility for people using the "old" names READONLY or RESTRICTED. So in that sense it does deprecate structmember.h

    But indeed I hijacked this issue with my complaints about the limited API which may not have been the original intention here, but they get solved nonetheless.

    @vstinner
    Copy link
    Member

    Also, the bare minimum enhancement would be add rename READONLY to PY_READONLY, but keep a deprecated alias READONLY to PY_READONLY, and update CPython code base to use PY_READONLY. (Same for other similar flags.)

    @MatzeB MatzeB mannequin changed the title Deprecate structmember.h PyMemberDef missing in limited API / Deprecate structmember.h May 27, 2020
    @MatzeB
    Copy link
    Mannequin

    MatzeB mannequin commented May 27, 2020

    Happy to take the proposed diff here (assuming @belopolsky wont mind) and include it into a pull request that also renames the uses of the READONLY flags (and maybe removes the RESTRICTED flags) within cpython source itself.

    @MatzeB
    Copy link
    Mannequin

    MatzeB mannequin commented May 27, 2020

    While working on the pull request I felt that the type and constants better fit descrobject.h rather than object.h.

    @encukou
    Copy link
    Member

    encukou commented Nov 2, 2022

    I ran into this issue, so I made a PR: #99014
    The twist is that old code doesn't need to change. That would be unnecessary churn. Keeping a few weirdly named aliases around is not a maintenance burden, and since they need the extra header they won't pollute new code.

    encukou added a commit that referenced this issue Nov 22, 2022
    …on.h (GH-99014)
    
    The ``structmember.h`` header is deprecated, though it continues to be available
    and there are no plans to remove it. There are no deprecation warnings. Old code
    can stay unchanged (unless the extra include and non-namespaced macros bother
    you greatly). Specifically, no uses in CPython are updated -- that would just be
    unnecessary churn.
    The ``structmember.h`` header is deprecated, though it continues to be
    available and there are no plans to remove it.
    
    Its contents are now available just by including ``Python.h``,
    with a ``Py`` prefix added if it was missing:
    
    - `PyMemberDef`, `PyMember_GetOne` and`PyMember_SetOne`
    - Type macros like `Py_T_INT`, `Py_T_DOUBLE`, etc.
      (previously ``T_INT``, ``T_DOUBLE``, etc.)
    - The flags `Py_READONLY` (previously ``READONLY``) and
      `Py_AUDIT_READ` (previously all uppercase)
    
    Several items are not exposed from ``Python.h``:
    
    - `T_OBJECT` (use `Py_T_OBJECT_EX`)
    - `T_NONE` (previously undocumented, and pretty quirky)
    - The macro ``WRITE_RESTRICTED`` which does nothing.
    - The macros ``RESTRICTED`` and ``READ_RESTRICTED``, equivalents of
      `Py_AUDIT_READ`.
    - In some configurations, ``<stddef.h>`` is not included from ``Python.h``.
      It should be included manually when using ``offsetof()``.
    
    The deprecated header continues to provide its original
    contents under the original names.
    Your old code can stay unchanged, unless the extra include and non-namespaced
    macros bother you greatly.
    
    There is discussion on the issue to rename `T_PYSSIZET` to `PY_T_SSIZE` or
    similar. I chose not to do that -- users will probably copy/paste that with any
    spelling, and not renaming it makes migration docs simpler.
    
    
    Co-Authored-By: Alexander Belopolsky <abalkin@users.noreply.github.com>
    Co-Authored-By: Matthias Braun <MatzeB@users.noreply.github.com>
    @encukou encukou closed this as completed Nov 22, 2022
    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jul 18, 2023
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 21, 2023
    …ializer (pythonGH-106862)
    
    (cherry picked from commit 8d397ee)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    serhiy-storchaka added a commit that referenced this issue Jul 21, 2023
    …tializer (GH-106862) (GH-106953)
    
    (cherry picked from commit 8d397ee)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes docs Documentation in the Doc dir interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    9 participants