Description

vstinner

opened on Mar 30, 2022

Issue body actions

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

   on Mar 30, 2022

   MemberAuthor

   More actions

   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 fixesonly security fixes

   topic-C-API

   type-featureA feature request or enhancementA feature request or enhancement

   on Mar 30, 2022
3. gpshead commented on Apr 5, 2022

   

   gpshead

   on Apr 5, 2022

   Member

   More actions

   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 dirTests 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

   on Apr 26, 2022

   MemberAuthor

   More actions

   @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

   on Apr 26, 2022

   Contributor

   More actions

   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

   on Apr 26, 2022

   MemberAuthor

   More actions

   An example of (fixed) C++ issue with the Python C API: #87347
9. vstinner commented on Apr 27, 2022

   

   vstinner

   on Apr 27, 2022

   MemberAuthor

   More actions

   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

    on May 2, 2022

    Contributor

    More actions

    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

    on May 2, 2022

    MemberAuthor

    More actions

    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

    on May 2, 2022

    Contributor

    More actions

    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

    on May 3, 2022

    MemberAuthor

    More actions

    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

Issue actions