Skip to content

3.12.0b1: static declaration follows non-static when importing internal/pycore_dict.h #105268

Closed
Closed
3.12.0b1: static declaration follows non-static when importing internal/pycore_dict.h#105268
Labels
3.12only security fixes3.13only security fixestype-bugAn unexpected behavior, bug, or error

Description

@P403n1x87
P403n1x87
opened on Jun 3, 2023

Bug report

I have a change within a project that includes a native extension which fails to compile with 3.12.0b1:

      In file included from echion/coremodule.cc:22:
      In file included from ./echion/dict.h:7:
      In file included from /Users/gabriele/.pyenv/versions/3.12.0b1/include/python3.12/internal/pycore_dict.h:13:
      In file included from /Users/gabriele/.pyenv/versions/3.12.0b1/include/python3.12/internal/pycore_runtime.h:16:
      In file included from /Users/gabriele/.pyenv/versions/3.12.0b1/include/python3.12/internal/pycore_global_objects.h:11:
      /Users/gabriele/.pyenv/versions/3.12.0b1/include/python3.12/internal/pycore_gc.h:84:19: error: static declaration of 'PyObject_GC_IsFinalized' follows non-static declaration
      static inline int _PyGC_FINALIZED(PyObject *op) {
                        ^
      /Users/gabriele/.pyenv/versions/3.12.0b1/include/python3.12/cpython/objimpl.h:85:30: note: expanded from macro '_PyGC_FINALIZED'
      #  define _PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o)
                                   ^
      /Users/gabriele/.pyenv/versions/3.12.0b1/include/python3.12/objimpl.h:209:17: note: previous declaration is here
      PyAPI_FUNC(int) PyObject_GC_IsFinalized(PyObject *);

I didn't look too deep into what changed here, but my guess would be that the macro should drop the _ prefix, i.e.

#  define PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o)

since it seems that PyObject_GC_IsFinalized is part of the API. The same code compiles fine with older versions of Python.

Your environment

MacOS Ventura 13.3.1
Python 3.12.0b1

Linked PRs

Activity

  1. P403n1x87
    added
    type-bugAn unexpected behavior, bug, or error
    on Jun 3, 2023
  2. AlexWaygood
    added
    3.12only security fixes
    3.13only security fixes
    on Jun 3, 2023
  3. terryjreedy
    added
    OS-mac
    on Jun 3, 2023

  4. sunmy2019 commented on Jun 3, 2023

    @sunmy2019
    sunmy2019
    on Jun 3, 2023
    Member

    You are including internal headers which is not allowed. You should never define Py_BUILD_CORE.

    /* Defines to build Python and its standard library:
 *
 * - Py_BUILD_CORE: Build Python core. Give access to Python internals, but
 *   should not be used by third-party modules.
 * - Py_BUILD_CORE_BUILTIN: Build a Python stdlib module as a built-in module.
 * - Py_BUILD_CORE_MODULE: Build a Python stdlib module as a dynamic library.
 *
 * Py_BUILD_CORE_BUILTIN and Py_BUILD_CORE_MODULE imply Py_BUILD_CORE.
 *
 * On Windows, Py_BUILD_CORE_MODULE exports "PyInit_xxx" symbol, whereas
 * Py_BUILD_CORE_BUILTIN does not.
 */

    Thus, you will never see this function:

          static inline int _PyGC_FINALIZED(PyObject *op) {

    Say if you are really relying on some internal things, make sure you have defined Py_BUILD_CORE for all headers.

    Then you will never see this definition:

          #  define _PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o)

    P.S. I have found no dict.h in your repo.

  5. P403n1x87 commented on Jun 3, 2023

    @P403n1x87
    P403n1x87
    on Jun 3, 2023
    ContributorAuthor

    Say if you are really relying on some internal things, make sure you have defined Py_BUILD_CORE for all headers.

    I really do need these internal structures. So this means that the fact it used to work for older versions of Python is just a mere accident. Correct? I'll try defining Py_BUILD_CORE in all my headers and see what happens 🤞 . Just wanted to call this out in case it was the symptom of an actual issue, since this worked with previous minor releases.

    P.S. I have found no dict.h in your repo.

    The change with the #include <internal/pycore_dict.h> lives in a local branch that I haven't pushed yet.

  6. ronaldoussoren
    removed
    OS-mac
    on Jun 4, 2023

  7. iritkatriel commented on Jun 4, 2023

    @iritkatriel
    iritkatriel
    on Jun 4, 2023
    Member

    CC @vstinner .

  8. vstinner
    added a commit that references this issue on Jun 6, 2023

    pythongh-105268: Remove _PyGC_FINALIZED() macro

    724ed0e

  9. vstinner commented on Jun 6, 2023

    @vstinner
    vstinner
    on Jun 6, 2023
    Member
      include/python3.12/cpython/objimpl.h:85:30: note: expanded from macro '_PyGC_FINALIZED'
      #  define _PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o)

    It seems like the Python.h header was included before the Py_BUILD_CORE macro was defined, and the internal/pycore_gc.h header was included after the Py_BUILD_CORE macro was defined.

    Well, this old macro is causing the trouble here. It should just be removed: I wrote PR #105350 for that.

    You should be able to work around the issue by defining the Py_BUILD_CORE macro earlier.

  10. vstinner
    added a commit that references this issue on Jun 6, 2023

    gh-105268: Remove _PyGC_FINALIZED() macro (#105350)

    8ddf0dd

  11. vstinner commented on Jun 6, 2023

    @vstinner
    vstinner
    on Jun 6, 2023
    Member

    I removed the macro. Thanks for your reminder :-) I close the issue.

  12. vstinner
    closed this as completedon Jun 6, 2023
  13. vstinner
    added 3 commits that reference this issue on Jun 6, 2023

    pythongh-105268: Add _Py_FROM_GC() function to pycore_gc.h

    74ceaf8

    pythongh-105268: Add _Py_FROM_GC() function to pycore_gc.h

    961468e

    gh-105268: Add _Py_FROM_GC() function to pycore_gc.h (#105362)

    c7bf74b

  14. albanD commented on Jul 26, 2023

    @albanD
    albanD
    on Jul 26, 2023
    Contributor

    I would add to this issue that the PyTorch project is also heavily using partial Py_BUILD_CORE import.
    It is actually required to have access to the _PyInterpreterFrame object required for the frame hook API https://docs.python.org/3/c-api/init.html#c._PyFrameEvalFunction from https://peps.python.org/pep-0523/

    The removal of the macro fixes our build as well but wanted to add one more data point.

  15. vstinner commented on Jul 26, 2023

    @vstinner
    vstinner
    on Jul 26, 2023
    Member

    I would add to this issue that the PyTorch project is also heavily using partial Py_BUILD_CORE import.
    It is actually required to have access to the _PyInterpreterFrame object required for the frame hook API

    Maybe you should open an issue to ask for a way to avoid Py_BUILD_CORE in PyTorch :-)

    The removal of the macro fixes our build as well but wanted to add one more data point.

    Hum, I didn't backport this change to Python 3.12. Maybe I should? Are you using Python 3.12 or the future 3.13? Can you patch Python?

  16. albanD commented on Jul 27, 2023

    @albanD
    albanD
    on Jul 27, 2023
    Contributor

    Maybe I should? Are you using Python 3.12 or the future 3.13? Can you patch Python?

    Ho I assumed it would make it to the rc even though it wasn't on b4. Backporting it would be amazing as other we will have to build the whole of PyTorch with Py_BUILD_CORE for 3.12 :/

    Maybe you should open an issue to ask for a way to avoid Py_BUILD_CORE in PyTorch

    Very happy to do that! Is the best place here on github or on the discuss forum?

  17. vstinner
    added 2 commits that reference this issue on Jul 27, 2023

    pythongh-105268: Remove _PyGC_FINALIZED() macro (python#105350)

    a31167b

    pythongh-105268: _PyGC_FINALIZED() removal is already documented in 3.12

    efcf7d7

  18. vstinner commented on Jul 27, 2023

    @vstinner
    vstinner
    on Jul 27, 2023
    Member

    I created PR #107348 and PR #107350 for the backport to 3.12.

    Very happy to do that! Is the best place here on github or on the discuss forum?

    An issue on GitHub sounds like a good idea.

  19. vstinner
    added a commit that references this issue on Jul 27, 2023

    gh-105268: _PyGC_FINALIZED() removal is already documented in 3.12 (#…

    9a7204b
  20. vstinner
    added a commit that references this issue on Aug 14, 2023

    pythongh-105268: Remove _PyGC_FINALIZED() macro (python#105350)

    149ad93

  21. vstinner commented on Aug 15, 2023

    @vstinner
    vstinner
    on Aug 15, 2023
    Member

    The change is not going to be backported to Python 3.12: #107348

    The workaround is to first include <Python.h>, undefine _PyGC_FINALIZED macro (#undef _PyGC_FINALIZED), and only then include the internal pycore header files.

    Thanks for the bug report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.12only security fixes3.13only security fixestype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions