Skip to content

Use reinterpret_cast and nullptr in pythoncapi_compat.h#3237

Merged
oleksiyskononenko merged 1 commit intoh2oai:mainfrom
vstinner:cpp_cast
Feb 10, 2022
Merged

Use reinterpret_cast and nullptr in pythoncapi_compat.h#3237
oleksiyskononenko merged 1 commit intoh2oai:mainfrom
vstinner:cpp_cast

Conversation

@vstinner
Copy link
Copy Markdown
Contributor

@vstinner vstinner commented Feb 9, 2022

Adjust pythoncapi_compat.h to produce no warnings in C++ compilers.

@oleksiyskononenko
Copy link
Copy Markdown
Contributor

Thanks, now I don't see any warnings produced by pythoncapi_compat.h. Is this PR still WIP or you're ok it to be merged?

@vstinner vstinner marked this pull request as ready for review February 10, 2022 00:07
@vstinner
Copy link
Copy Markdown
Contributor Author

I was awaiting until the CIs completed for checking for warnings. Thanks for checking for me! The PR is now ready for review ;-)

I updated datatable copy of pythoncapi_compat.h. I already merged the C++ compatibility in pythoncapi_compat ;-)

@oleksiyskononenko oleksiyskononenko changed the title WIP: pythoncapi_compat.h uses reinterpret_cast<> and nullptr Use reinterpret_cast and nullptr in pythoncapi_compat.h Feb 10, 2022
@oleksiyskononenko oleksiyskononenko merged commit f6a0082 into h2oai:main Feb 10, 2022
@vstinner vstinner deleted the cpp_cast branch February 10, 2022 11:29
@vstinner
Copy link
Copy Markdown
Contributor Author

Thanks, now I don't see any warnings produced by pythoncapi_compat.h.

That's great :-) I tried many options to avoid emitting warnings when including this file in C++.

I still don't get why some pythoncapi_compat.h functions emit warnings, whereas Python.h doesn't.

It seems like "old-style cast" is triggered when a static function implements a cast inside the static inline function used in C++. I don't understand if it only emits a warning if the function is used or not.

@st-pasha st-pasha added this to the Release 1.1.0 milestone Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants