FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-91321: Add _Py_NULL macro by vstinner · Pull Request #92253 · python/cpython · GitHub

/ cpython Public

gh-91321: Add _Py_NULL macro - #92253

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:py_null
May 3, 2022
ConversationCommits (1)ChecksFiles changed
Merged

gh-91321: Add _Py_NULL macro#92253
vstinner merged 1 commit into
python:mainfrom
vstinner:py_null

Conversation

vstinner commented May 3, 2022

Copy link
Copy Markdown
Member

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.

gh-91321: Add _Py_NULL macro
924d3bd 
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 mentioned this pull request May 3, 2022
Merged

vstinner commented May 3, 2022

Copy link
Copy Markdown
Member Author

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 commented May 3, 2022

Copy link
Copy Markdown
Member Author

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 commented May 3, 2022

Copy link
Copy Markdown
Member Author

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

vstinner merged commit 551d02b into python:main May 3, 2022
vstinner deleted the py_null branch May 3, 2022 19:38
vstinner mentioned this pull request May 3, 2022
Closed

neutrinoceros commented Jun 13, 2022

Copy link
Copy Markdown
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 ?

neutrinoceros mentioned this pull request Jun 13, 2022
Merged

vstinner commented Jun 13, 2022

Copy link
Copy Markdown
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 commented Jun 13, 2022

Copy link
Copy Markdown
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 commented Jun 13, 2022

Copy link
Copy Markdown
Member Author

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

vstinner commented Jun 13, 2022

Copy link
Copy Markdown
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 commented Jun 13, 2022

Copy link
Copy Markdown
Contributor

Awesome, thank you very much !

vstinner commented Jun 14, 2022

Copy link
Copy Markdown
Member Author

Python 3.11 is now fixed:

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL