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

gh-91321: Add _Py_NULL macro #92253

Merged
merged 1 commit into from
May 3, 2022
Merged

gh-91321: Add _Py_NULL macro #92253

merged 1 commit into from
May 3, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 3, 2022

Fix C++ compiler warnings: "zero as null pointer constant"
(clang -Wzero-as-null-pointer-constant).

  • Add the _Py_NULL macro used by static inline functions to use
    nullptr in C++.
  • Replace NULL with nullptr in _testcppext.cpp.

Fix C++ compiler warnings: "zero as null pointer constant"
(clang -Wzero-as-null-pointer-constant).

* Add the _Py_NULL macro used by static inline functions to use
  nullptr in C++.
* Replace NULL with nullptr in _testcppext.cpp.
@vstinner
Copy link
Member Author

vstinner commented May 3, 2022

I already had this issue and fixed it in pythoncapi-compat. I prepared a PR for pythoncapi-compat to reuse Python 3.11 _Py_NULL macro (this PR): python/pythoncapi-compat#36

@vstinner
Copy link
Member Author

vstinner commented May 3, 2022

Currently, with gcc -Wzero-as-null-pointer-constant, there is no C++ compiler warning when including <Python.h>. But using clang -Wzero-as-null-pointer-constant, I get many warnings (errors). A few examples:

$ CC=clang ./python -m test -v test_cppext
(...)
In file included from /home/vstinner/python/main/Lib/test/_testcppext.cpp:7:
In file included from /home/vstinner/python/main/Include/Python.h:44:
/home/vstinner/python/main/Include/object.h:590:15: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
    if (op != NULL) {
              ^~~~
              nullptr
/usr/lib64/clang/13.0.0/include/stddef.h:84:18: note: expanded from macro 'NULL'
#    define NULL __null
                 ^
In file included from /home/vstinner/python/main/Lib/test/_testcppext.cpp:7:
In file included from /home/vstinner/python/main/Include/Python.h:44:
/home/vstinner/python/main/Include/object.h:600:15: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
    if (op != NULL) {
              ^~~~
              nullptr
/usr/lib64/clang/13.0.0/include/stddef.h:84:18: note: expanded from macro 'NULL'
#    define NULL __null
                 ^
In file included from /home/vstinner/python/main/Lib/test/_testcppext.cpp:7:
In file included from /home/vstinner/python/main/Include/Python.h:51:
In file included from /home/vstinner/python/main/Include/unicodeobject.h:1042:
/home/vstinner/python/main/Include/cpython/unicodeobject.h:645:42: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
    if (_PyASCIIObject_CAST(op)->wstr == NULL) {
                                         ^~~~
                                         nullptr
/usr/lib64/clang/13.0.0/include/stddef.h:84:18: note: expanded from macro 'NULL'
#    define NULL __null
                 ^
(...)
Tests result: FAILURE

@vstinner
Copy link
Member Author

vstinner commented May 3, 2022

I also got these errors on the CI of the datatabl project: see h2oai/datatable#3231

@neutrinoceros
Copy link
Contributor

This breaks C-extensions builds with , e.g., -std=c++03, because nullptr is only defined since C++11
Is it desired that Python 3.11 de facto requires C++11 or above ?

@vstinner
Copy link
Member Author

In Python 3.11, I added test_cppext which checks that the Python C API is compatible with C++11. Do you care about older C++ versions? What is your OS (name and version) and C++ compiler (name and version)?

@neutrinoceros
Copy link
Contributor

Actually we're explicitly requiring -std=c++03 in a couple Cython files.
This is probably something we can (and should) update on our side, I just wanted to check if the breaking change was intentional :)

@vstinner
Copy link
Member Author

I wrote #93784 to fix compatibility with C++ older than C++11.

@vstinner
Copy link
Member Author

I just wanted to check if the breaking change was intentional :)

It's not intentional. My initial concern was to prevent C++ compiler warnings.

@neutrinoceros
Copy link
Contributor

Awesome, thank you very much !

@vstinner
Copy link
Member Author

Python 3.11 is now fixed:

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

Successfully merging this pull request may close these issues.

3 participants