Sitelet https://github.com/python/cpython/issues/91321
Skip to content

[C API] Test that the Python C API is compatible with C++ #91321

Closed
Closed
[C API] Test that the Python C API is compatible with C++#91321
Labels
3.11only security fixestestsTests in the Lib/test dirtopic-C-APItype-featureA feature request or enhancement

Description

@vstinner
vstinner
opened on Mar 30, 2022
BPO 47165
Nosy @gpshead, @vstinner

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 2022-03-30.14:14:35.817>
labels = ['expert-C-API', 'type-feature', 'tests', '3.11']
title = '[C API] Test that the Python C API is compatible with C++'
updated_at = <Date 2022-04-05.06:49:28.827>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-04-05.06:49:28.827>
actor = 'gregory.p.smith'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Tests', 'C API']
creation = <Date 2022-03-30.14:14:35.817>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 47165
keywords = []
message_count = 2.0
messages = ['416359', '416756']
nosy_count = 2.0
nosy_names = ['gregory.p.smith', 'vstinner']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'test needed'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue47165'
versions = ['Python 3.11']

Activity

  1. vstinner commented on Mar 30, 2022

    @vstinner
    vstinner
    on Mar 30, 2022
    MemberAuthor

    There are more and more popular projects using the Python C API. The first big player is pybind11:
    "Seamless operability between C++11 and Python"
    https://pybind11.readthedocs.io/

    Recently, I proposed a PR to add Python 3.11 support to the datatable project:
    h2oai/datatable#3231

    My PR uses pythoncapi_compat.h header file which provides recent C API functions to old Python functions. The header file implements these functions as static inline function.

    Problem: a static inline function implemented in a C header file used in a C++ code file can emit C++ compiler warnings.

    In datatable, I got two kinds of C++ compiler warnings:

    • Usage of the C NULL constant: C++ prefers nullptr
    • "Old-style" cast like (PyObject*)obj: C++ prefers static_cast, reinterpret_cast, etc.

    It seems like these compiler warnings are not enabled by default. The datatable project seems enabling them in its CI and I was asked to fix these warnings.

    In the pythoncapi-compat project (*), I chose to use nullptr and reinterpret_cast if the "__cplusplus" macro is defined. Example:

    // C++ compatibility
#ifdef __cplusplus
#  define PYCAPI_COMPAT_CAST(TYPE, EXPR) reinterpret_cast<TYPE>(EXPR)
#  define PYCAPI_COMPAT_NULL nullptr
#else
#  define PYCAPI_COMPAT_CAST(TYPE, EXPR) ((TYPE)(EXPR))
#  define PYCAPI_COMPAT_NULL NULL
#endif

// Cast argument to PyObject* type.
#ifndef _PyObject_CAST
#  define _PyObject_CAST(op) PYCAPI_COMPAT_CAST(PyObject*, op)
#endif

    (*) https://github.com/python/pythoncapi_compat

    It's unclear to me if the Python C API has or has not the same issue than pythoncapi_compat.h.

    Last years, some old macros of the Python C API have been converted to static inline functions, like Py_INCREF(). It's unclear to me if these compiler warnings happen on Py_INCREF(). I don't understand why, but static inline macros from Python.h didn't emit compiler warnings in datatable, whereas similar static inline functions of pythoncapi_compat.h emitted compiler warnings.

    Maybe there is a difference between <Python.h> and "Python.h". Or maybe it depends if the header file is a "local" file, or a "system" header file (ex: installed in /usr/include/ on Linux).

    A first step would be to build a C++ extension as part of the Python test suite and check that there is no compiler warning. My #76356 PR is a proof-of-concept of that.

    I don't know which C++ version we should target. pybind11 targets C++11. See bpo-39355 for a discussion about C++20: usage of the C++20 "module" keyword... which is a "contextual keyword" in practice.

  2. vstinner
    added
    3.11only security fixes
    topic-C-API
    type-featureA feature request or enhancement
    on Mar 30, 2022

  3. gpshead commented on Apr 5, 2022

    @gpshead
    gpshead
    on Apr 5, 2022
    Member

    If we can conditionally test new things based on C++XX version, accumulating modern issue regression tests seems useful. Otherwise 11 at minimum.

    As for why some things trigger this and others don't, my wild _guess_ would be whether the statements appear within an extern "C" block. Though that's not really what that is for so it isn't clear to me.

  4. gpshead
    added
    testsTests in the Lib/test dir
    on Apr 5, 2022
  5. ezio-melotti
    transferred this issue fromon Apr 10, 2022

  6. vstinner commented on Apr 26, 2022

    @vstinner
    vstinner
    on Apr 26, 2022
    MemberAuthor

    @erlend-aasland: it seems like PEP 670 made this issue worse. It seems like C and C++ compilers emit more warnings when code comes from a static inline function, rather than from a macro.

    cc @corona10

  7. erlend-aasland commented on Apr 26, 2022

    @erlend-aasland
    erlend-aasland
    on Apr 26, 2022
    Contributor

    That depends on how the macro was written in the first place (argument casts?, "return" cast?), so yeah, for some macros you'll get more warnings if you convert them to static inlined functions because of the strict typing.

  8. vstinner commented on Apr 26, 2022

    @vstinner
    vstinner
    on Apr 26, 2022
    MemberAuthor

    An example of (fixed) C++ issue with the Python C API: #87347

  9. vstinner commented on Apr 27, 2022

    @vstinner
    vstinner
    on Apr 27, 2022
    MemberAuthor

    I wrote #32175 to check that the Python C API is compatible with C++.

  10. 4 remaining items

  11. kumaraditya303 commented on May 2, 2022

    @kumaraditya303
    kumaraditya303
    on May 2, 2022
    Contributor

    C++ does not support designated initializers in array and is causing compilation errors in #92142. I reopen the issue.

  12. kumaraditya303
    reopened this on May 2, 2022

  13. vstinner commented on May 2, 2022

    @vstinner
    vstinner
    on May 2, 2022
    MemberAuthor

    C++ does not support designated initializers in array and is causing compilation errors in #92142. I reopen the issue.

    Which code fails to build? What is the error message?

    This issue is about testing that the current Python C API builds successfully in C++. It does: the test passed on all CIs.

    Is the issue related to your PR? If yes, please open a new issue.

  14. kumaraditya303 commented on May 2, 2022

    @kumaraditya303
    kumaraditya303
    on May 2, 2022
    Contributor

    Okay I'll create a separate issue for it.

  15. kumaraditya303
    closed this as completedon May 2, 2022
  16. vstinner
    added a commit that references this issue on May 3, 2022

    gh-91321: Add _Py_NULL macro (#92253)

    551d02b

  17. vstinner commented on May 3, 2022

    @vstinner
    vstinner
    on May 3, 2022
    MemberAuthor

    I was still able to reproduce the C++ warning about NULL, so I pushed: commit 551d02b "gh-91321: Add _Py_NULL macro (#92253)".

  18. vstinner
    added a commit that references this issue on May 3, 2022

    gh-91321: Fix PyModuleDef_HEAD_INIT on C++ (#92259)

    3a35b62
  19. pablogsal
    added a commit that references this issue on May 6, 2022

    gh-91321: Fix _PyObject_EXTRA_INIT for C++ (#92396)

    c33d67c
  20. vstinner
    added 2 commits that reference this issue on Jun 14, 2022

    gh-91321: Fix compatibility with C++ older than C++11 (#93784)

    4caf5c2

    gh-91321: Fix compatibility with C++ older than C++11 (#93784) (#93802)

    ef591cf
  21. miss-islington
    added a commit that references this issue on Jun 16, 2022

    pythongh-91321: Fix test_cppext for C++03 (pythonGH-93902)

    619ae36
  22. vstinner
    added 2 commits that reference this issue on Jun 16, 2022

    gh-91321: Fix test_cppext for C++03 (#93902)

    a38c2a6

    gh-91321: Fix test_cppext for C++03 (GH-93902) (#93904)

    62f72e4
  23. serhiy-storchaka
    added a commit that references this issue on Jun 26, 2022

    Revert 30935 defer bpo45162 to 312 (#94285)

    1ffde3e
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.11only security fixestestsTests in the Lib/test dirtopic-C-APItype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions