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

The Python library will not compile with a C++2020 compiler because the code uses the reserved “module” keyword #83536

Closed
aCuria mannequin opened this issue Jan 16, 2020 · 19 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes easy topic-C-API type-feature A feature request or enhancement

Comments

@aCuria
Copy link
Mannequin

aCuria mannequin commented Jan 16, 2020

BPO 39355
Nosy @vstinner, @encukou, @serhiy-storchaka, @corona10, @erlend-aasland, @aCuria, @AliyevH
PRs
  • bpo-39355: Renamed module argument to pyModule in header files #28359
  • bpo-39355: Renamed module argument to pyModule in header files #28373
  • bpo-39355: making Python.h compatible with C++20 compilers #31282
  • gh-91321: Add _testcppext C++ extension #32175
  • 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 = <Date 2022-03-29.13:39:30.927>
    created_at = <Date 2020-01-16.09:25:21.719>
    labels = ['easy', '3.9', '3.10', '3.11', 'expert-C-API', 'type-feature', 'invalid']
    title = 'The Python library will not compile with a C++2020 compiler because the code uses the reserved \xe2\x80\x9cmodule\xe2\x80\x9d keyword'
    updated_at = <Date 2022-03-30.14:14:56.748>
    user = 'https://github.com/aCuria'

    bugs.python.org fields:

    activity = <Date 2022-03-30.14:14:56.748>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-03-29.13:39:30.927>
    closer = 'vstinner'
    components = ['C API']
    creation = <Date 2020-01-16.09:25:21.719>
    creator = 'aCuria'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39355
    keywords = ['patch', 'easy (C)']
    message_count = 19.0
    messages = ['360103', '360106', '360110', '360115', '398915', '399073', '399975', '401109', '401778', '413056', '414646', '414654', '416253', '416254', '416255', '416256', '416257', '416259', '416360']
    nosy_count = 7.0
    nosy_names = ['vstinner', 'petr.viktorin', 'serhiy.storchaka', 'corona10', 'erlendaasland', 'aCuria', 'AliyevH']
    pr_nums = ['28359', '28373', '31282', '32175']
    priority = 'normal'
    resolution = 'not a bug'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue39355'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @aCuria
    Copy link
    Mannequin Author

    aCuria mannequin commented Jan 16, 2020

    The Python library will not compile with a C++2020 compiler because the code uses the reservedmodulekeyword
     
    For example, in warnings.h, we have the following code:
     
    #ifndef Py_LIMITED_API
    PyAPI_FUNC(int) PyErr_WarnExplicitObject(
        PyObject *category,
        PyObject *message,
        PyObject *filename,
        int lineno,
        PyObject *module,
    PyObject *registry);
     
     
    In modsupport.h we have the following code:
    PyAPI_FUNC(int) PyModule_ExecDef(PyObject *module, PyModuleDef *def);
     
    We can fix this by using a different identifier, for examplepyModuleinstead ofmodule

    @aCuria aCuria mannequin added 3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes topic-C-API labels Jan 16, 2020
    @serhiy-storchaka
    Copy link
    Member

    Names of arguments can be just removed from function declarations in header files.

    @serhiy-storchaka serhiy-storchaka added easy type-feature A feature request or enhancement labels Jan 16, 2020
    @vstinner
    Copy link
    Member

    Qt has a similar issue with "slots": bpo-1086854 and bpo-38007.

    @serhiy-storchaka
    Copy link
    Member

    Qt has different issue. "slots" is not a keyword, and the issue can be avoided by including Python.h before Qt.h or undefining the "slots" macro.

    It could be a harder issue if "module" would be a field name of a public structure. But names of arguments are not part of the API.

    @AliyevH
    Copy link
    Mannequin

    AliyevH mannequin commented Aug 4, 2021

    If nobody works, i would like to solve this

    @vstinner
    Copy link
    Member

    vstinner commented Aug 6, 2021

    Please go ahead. Anyone is free to propose a fix.

    @AliyevH
    Copy link
    Mannequin

    AliyevH mannequin commented Aug 20, 2021

    We have tested with cxx-modules that issue.
    module is just a specifier for export (only export is a compiler-based keyword in >= C++20)
    That's why we can use module as argument name and there's no need to rename or delete *module arguments from header files.

    What do you recommend to do?

    http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1103r3.pdf

    @aCuria
    Copy link
    Mannequin Author

    aCuria mannequin commented Sep 6, 2021

    the word "module" should be treated as a reserved keyword.

    Any use of "module" as an argument name should be changed to something else
    throughout the code base.

    On Fri, Aug 20, 2021 at 11:28 PM Hasan <report@bugs.python.org> wrote:

    Hasan <hasan.aleeyev@gmail.com> added the comment:

    We have tested with cxx-modules that issue.
    module is just a specifier for export (only export is a compiler-based
    keyword in >= C++20)
    That's why we can use module as argument name and there's no need to
    rename or delete *module arguments from header files.

    What do you recommend to do?

    http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1103r3.pdf

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue39355\>


    @AliyevH
    Copy link
    Mannequin

    AliyevH mannequin commented Sep 14, 2021

    Okey. There will be huge changes for this issue as this keyword has been used in a lot of places.

    That's why i will try to send pull requests module by module to keep it clean and simple for review.

    @erlend-aasland
    Copy link
    Contributor

    There will be huge changes for this issue as this keyword has been used in a lot of places.

    For external extension modules, it should be sufficient to change only the exposed headers; that is Include/.h and Include/cpython/.h. We should also change the AC tool, since that may be used by external projects.

    @vstinner
    Copy link
    Member

    vstinner commented Mar 7, 2022

    The Python library will not compile with a C++2020 compiler because the code uses the reserved “module” keyword

    What is the error message? How can the error be reproduced?

    @aCuria
    Copy link
    Mannequin Author

    aCuria mannequin commented Mar 7, 2022

    Compile with a compiler supporting the C++20 core feature (Modules)

    https://en.cppreference.com/w/cpp/compiler_support

    In visual studio, use C/C++ > Language > CPP Language Standard > C++20 or
    higher

    On Mon, Mar 7, 2022 at 5:32 PM STINNER Victor <report@bugs.python.org>
    wrote:

    STINNER Victor <vstinner@python.org> added the comment:

    > The Python library will not compile with a C++2020 compiler because the
    code uses the reserved “module” keyword

    What is the error message? How can the error be reproduced?

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue39355\>


    @vstinner
    Copy link
    Member

    If I build a C++ extension with -std=c++20, I get a compiler error on PyModuleDef_HEAD_INIT:

    Modules/_testcppext.cpp:419:5: error: either all initializer clauses should be designated or none of them should be
    419 | .m_name = "_testcppext",
    | ^

    Code:
    ---

    static struct PyModuleDef module = {
        PyModuleDef_HEAD_INIT,
        .m_name = "_testcppext",
        .m_doc = module_doc,
        ...
    };

    Macro defined as (simplified code):
    ---

    #define PyObject_HEAD_INIT(type) \
        { 1, type },
    
    #define PyModuleDef_HEAD_INIT { \
        PyObject_HEAD_INIT(NULL)    \
        NULL, /* m_init */          \
        0,    /* m_index */         \
        NULL, /* m_copy */          \
      }

    @vstinner
    Copy link
    Member

    When I wrote a PR to use the C header file pythoncapi_compat.h in the datatable C++ project, I got multiple C++ compiler warnings in static inline functions and in some macros:
    h2oai/datatable#3231 (comment)

    • Usage of NULL
    • Usage of "old-style cast" like (ssize_t)1 or (PyObject*)obj

    I solved this issue in pythoncapi_compat.h by using reinterpret_cast and nullptr:

    By the way, pythoncapi_compat.h no longer uses "module":
    python/pythoncapi-compat#22

    The Python C API has similar issues, but warnings about NULL and old-style cast depend on the C++ compiler flags:

    • -Wzero-as-null-pointer-constant
    • -Wold-style-cast

    @vstinner
    Copy link
    Member

    STINNER Victor:

    What is the error message? How can the error be reproduced?

    Keith (aCuria):

    Compile with a compiler supporting the C++20 core feature (Modules)
    https://en.cppreference.com/w/cpp/compiler_support

    I built a C++ extension which calls PyModule_AddType(): I get no warning. I tested GCC and LLVM clang.

    Commands:

    clang -Wno-unused-result -g -Og -Wall -O0 -fPIC -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c Modules/_testcppext.cpp -o build/temp.linux-x86_64-3.11-pydebug/Modules/_testcppext.o -I/home/vstinner/python -Werror -Wall -Wextra -Wconversion -Wno-typedef-redefinition -std=c++20 -Wzero-as-null-pointer-constant -Wold-style-cast

    gcc -Wno-unused-result -g -Og -Wall -O0 -fPIC -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c Modules/_testcppext.cpp -o build/temp.linux-x86_64-3.11-pydebug/Modules/_testcppext.o -I/home/vstinner/python -Werror -Wall -Wextra -Wconversion -Wno-typedef-redefinition -std=c++20 -Wzero-as-null-pointer-constant -Wold-style-cast

    Reformatted commands:

    ['clang',
    '-Wno-unused-result',
    '-g',
    '-Og',
    '-Wall',
    '-O0',
    '-fPIC',
    '-I/home/vstinner/python/main/Include',
    '-I/home/vstinner/python/main',
    '-c', 'Modules/_testcppext.cpp',
    '-o', 'build/temp.linux-x86_64-3.11-pydebug/Modules/_testcppext.o',
    '-I/home/vstinner/python',
    '-Werror',
    '-Wall',
    '-Wextra',
    '-Wconversion',
    '-Wno-typedef-redefinition',
    '-std=c++20',
    '-Wzero-as-null-pointer-constant',
    '-Wold-style-cast']

    ['gcc',
    '-Wno-unused-result',
    '-g',
    '-Og',
    '-Wall',
    '-O0',
    '-fPIC',
    '-I/home/vstinner/python/main/Include',
    '-I/home/vstinner/python/main',
    '-c', 'Modules/_testcppext.cpp',
    '-o', 'build/temp.linux-x86_64-3.11-pydebug/Modules/_testcppext.o',
    '-I/home/vstinner/python',
    '-Werror',
    '-Wall',
    '-Wextra',
    '-Wconversion',
    '-Wno-typedef-redefinition',
    '-std=c++20',
    '-Wzero-as-null-pointer-constant',
    '-Wold-style-cast']

    @vstinner vstinner added 3.10 only security fixes 3.11 only security fixes and removed 3.7 (EOL) end of life labels Mar 29, 2022
    @vstinner vstinner added 3.10 only security fixes 3.11 only security fixes and removed 3.8 (EOL) end of life 3.7 (EOL) end of life labels Mar 29, 2022
    @vstinner
    Copy link
    Member

    I wrote the draft PR #76356 to test https://bugs.python.org/issue39355 and #75463. Problem: I don't get any compiler warning or error about the "module" C++20 keyword. I tested GCC 11.2.1 and clang 13.0.0 of Fedora 35.

    @vstinner
    Copy link
    Member

    The C++20 "module" keyword is "contextual keyword". It's only a keyword if the first line if a file contains "module".

    It's not the case in any .h file of the Python C API, so Python doesn't need to be changed. I close the issue.

    @erlend-aasland
    Copy link
    Contributor

    The C++20 "module" keyword is "contextual keyword". It's only a keyword if the first line if a file contains "module".

    Great! No changes needed. Thanks for investigating.

    @vstinner
    Copy link
    Member

    Follow-up issue: bpo-47165 "[C API] Test that the Python C API is compatible with C++".

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes easy topic-C-API type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants