NotificationsYou must be signed in to change notification settings
Fork33.7k
Star70.4k

gh-92135: Fix _Py_reinterpret_cast() for const#92138

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

vstinner merged 1 commit intopython:mainfromvstinner:const_cast

May 2, 2022

Merged

gh-92135: Fix _Py_reinterpret_cast() for const#92138

vstinner merged 1 commit intopython:mainfromvstinner:const_cast

May 2, 2022

Conversation

Copy link

Member

vstinner commentedMay 2, 2022•
edited
Loading

Fix C++ compiler warnings on cast macros, like _PyObject_CAST(), when
casting a constant expression to a non constant type: use
const_cast<> in C++.

In C++, Py_SAFE_DOWNCAST() now uses static_cast<> rather than
reinterpret_cast<>.
Add tests to the _testcppext C++ extension.
test_cppext no longer captures stdout in verbose mode.

bedevere-bot added the awaiting core review label

May 2, 2022

Copy link

MemberAuthor

vstinner commentedMay 2, 2022

@tacaswell @serge-sans-paille: Does my_Py_reinterpret_const_cast() macro looks like a monster, or does it look correct?

vstinner added the skip news label

May 2, 2022

Copy link

MemberAuthor

vstinner commentedMay 2, 2022

I merged my PR#32175 adding C++ tests to Python stdlib. So I rebased this PR on top of it and I added tests for this PR.

Currently, tests are notexecuted: only compiler warnings are checked by test_cppext. It is enough for checking that issue#92135 is fixed.

Copy link

Contributor

erlend-aasland commentedMay 2, 2022

FTR, in another project, I recently had to use two casts in order to avoid G++ warnings:

voidinner_set(void *);// original code (generated warnings)voidset_func(constchar *str) {inner_set(str);}// new set func, still generated warningsvoidimproved_set_func(constchar *str) {inner_set(const_cast<void *>(str));}// final set func, no warningsvoidset_func_without_warning(constchar *str) {constvoid *vptr =static_cast<constvoid *>(str);inner_set(const_cast<void *>(vptr));}

gh-92135: Fix _Py_reinterpret_cast() for const

31826b0

Fix C++ compiler warnings on cast macros, like _PyObject_CAST(), whencasting a constant expression to a non constant type: useconst_cast<> in C++.* In C++, Py_SAFE_DOWNCAST() now uses static_cast<> rather than  reinterpret_cast<>.* Add tests to the _testcppext C++ extension.* test_cppext no longer captures stdout in verbose mode.

vstinner changed the title~~gh-92135: Add _Py_reinterpret_const_cast() macro~~gh-92135: Fix _Py_reinterpret_cast() for const

May 2, 2022

Copy link

MemberAuthor

vstinner commentedMay 2, 2022

FTR, in another project, I recently had to use two casts in order to avoid G++ warnings

I understand that this approach (updated_Py_reinterpret_cast()) is correct ;-)

First, I renamed the macro _Py_reinterpret_const_cast(), but@serge-sans-paille didn't like this name, and me neither (it's too long!). So I kept the name_Py_reinterpret_cast().

erlend-aasland approved these changes

May 2, 2022

View reviewed changes

vstinner merged commit0313970 intopython:main

May 2, 2022

vstinner deleted the const_cast branch

May 2, 2022 15:07

bedevere-bot removed the awaiting core review label

May 2, 2022

vstinner mentioned this pull request

May 2, 2022

In C++, Python 3.11 C API introduces new compiler warnings/errors#92135

Closed

Copy link

MemberAuthor

vstinner commentedMay 2, 2022

@tacaswell: This issue should now be fixed. Please try greenlet with on the main branch of Python.

Copy link

Contributor

tacaswell commentedMay 2, 2022

The const issues is fixed the other casting issues still remains

✔ 13:12:04 $ gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/tcaswell/.virtualenvs/bleeding/include -I/home/tcaswell/.pybuild/bleeding/include/python3.11 -c src/greenlet/greenlet.cpp -o build/temp.linux-x86_64-3.11/src/greenlet/greenlet.o In file included from /home/tcaswell/.pybuild/bleeding/include/python3.11/Python.h:38,                 from src/greenlet/greenlet.cpp:14:src/greenlet/greenlet_refs.hpp: In member function ‘void greenlet::refs::PyErrPieces::normalize()’:/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}   30 |        const_cast<type>(reinterpret_cast<const type>(expr))      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:107:28: note: in expansion of macro ‘_Py_reinterpret_cast’  107 | #define _PyObject_CAST(op) _Py_reinterpret_cast(PyObject*, (op))      |                            ^~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:780:41: note: in expansion of macro ‘_PyObject_CAST’  780 | #  define PyType_Check(op) PyType_Check(_PyObject_CAST(op))      |                                         ^~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/pyerrors.h:57:6: note: in expansion of macro ‘PyType_Check’   57 |     (PyType_Check((x)) &&                                               \      |      ^~~~~~~~~~~~src/greenlet/greenlet_refs.hpp:993:17: note: in expansion of macro ‘PyExceptionClass_Check’  993 |             if (PyExceptionClass_Check(type)) {      |                 ^~~~~~~~~~~~~~~~~~~~~~In file included from /home/tcaswell/.pybuild/bleeding/include/python3.11/Python.h:44,                 from src/greenlet/greenlet.cpp:14:/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}   30 |        const_cast<type>(reinterpret_cast<const type>(expr))      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:774:59: note: in definition of macro ‘PyType_FastSubclass’  774 | #define PyType_FastSubclass(type, flag) PyType_HasFeature(type, flag)      |                                                           ^~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:107:28: note: in expansion of macro ‘_Py_reinterpret_cast’  107 | #define _PyObject_CAST(op) _Py_reinterpret_cast(PyObject*, (op))      |                            ^~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:136:31: note: in expansion of macro ‘_PyObject_CAST’  136 | #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))      |                               ^~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/pyerrors.h:61:25: note: in expansion of macro ‘Py_TYPE’   61 |     PyType_FastSubclass(Py_TYPE(x), Py_TPFLAGS_BASE_EXC_SUBCLASS)      |                         ^~~~~~~src/greenlet/greenlet_refs.hpp:1003:22: note: in expansion of macro ‘PyExceptionInstance_Check’ 1003 |             else if (PyExceptionInstance_Check(type)) {      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~In file included from /home/tcaswell/.pybuild/bleeding/include/python3.11/Python.h:38,                 from src/greenlet/greenlet.cpp:14:src/greenlet/greenlet_thread_state.hpp: In destructor ‘greenlet::ThreadState::~ThreadState()’:/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}   30 |        const_cast<type>(reinterpret_cast<const type>(expr))      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:107:28: note: in expansion of macro ‘_Py_reinterpret_cast’  107 | #define _PyObject_CAST(op) _Py_reinterpret_cast(PyObject*, (op))      |                            ^~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:265:59: note: in expansion of macro ‘_PyObject_CAST’  265 | #  define PyObject_TypeCheck(ob, type) PyObject_TypeCheck(_PyObject_CAST(ob), type)      |                                                           ^~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/methodobject.h:17:31: note: in expansion of macro ‘PyObject_TypeCheck’   17 | #define PyCFunction_Check(op) PyObject_TypeCheck(op, &PyCFunction_Type)      |                               ^~~~~~~~~~~~~~~~~~src/greenlet/greenlet_thread_state.hpp:400:33: note: in expansion of macro ‘PyCFunction_Check’  400 |                              && PyCFunction_Check(refs.at(0))      |                                 ^~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}   30 |        const_cast<type>(reinterpret_cast<const type>(expr))      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:107:28: note: in expansion of macro ‘_Py_reinterpret_cast’  107 | #define _PyObject_CAST(op) _Py_reinterpret_cast(PyObject*, (op))      |                            ^~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:127:35: note: in expansion of macro ‘_PyObject_CAST’  127 | #  define Py_REFCNT(ob) Py_REFCNT(_PyObject_CAST(ob))      |                                   ^~~~~~~~~~~~~~src/greenlet/greenlet_thread_state.hpp:401:33: note: in expansion of macro ‘Py_REFCNT’  401 |                              && Py_REFCNT(refs.at(0)) == 2) {      |                                 ^~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}   30 |        const_cast<type>(reinterpret_cast<const type>(expr))      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:107:28: note: in expansion of macro ‘_Py_reinterpret_cast’  107 | #define _PyObject_CAST(op) _Py_reinterpret_cast(PyObject*, (op))      |                            ^~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:580:29: note: in expansion of macro ‘_PyObject_CAST’  580 |         PyObject *_py_tmp = _PyObject_CAST(op); \      |                             ^~~~~~~~~~~~~~src/greenlet/greenlet_thread_state.hpp:422:33: note: in expansion of macro ‘Py_CLEAR’  422 |                                 Py_CLEAR(function_w);      |                                 ^~~~~~~~src/greenlet/greenlet.cpp: In function ‘PyObject* green_repr(greenlet::refs::BorrowedGreenlet)’:/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedGreenlet’ {aka ‘greenlet::refs::_BorrowedGreenlet<_greenlet, greenlet::refs::GreenletChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}   30 |        const_cast<type>(reinterpret_cast<const type>(expr))      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:107:28: note: in expansion of macro ‘_Py_reinterpret_cast’  107 | #define _PyObject_CAST(op) _Py_reinterpret_cast(PyObject*, (op))      |                            ^~~~~~~~~~~~~~~~~~~~/home/tcaswell/.pybuild/bleeding/include/python3.11/object.h:136:31: note: in expansion of macro ‘_PyObject_CAST’  136 | #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))      |                               ^~~~~~~~~~~~~~src/greenlet/greenlet.cpp:2395:33: note: in expansion of macro ‘Py_TYPE’ 2395 |     const char* const tp_name = Py_TYPE(self)->tp_name;      |                                 ^~~~~~~

tacaswell mentioned this pull request

May 2, 2022

Clarify that const is being cast awaypython-greenlet/greenlet#302

Closed

Copy link

MemberAuthor

vstinner commentedMay 3, 2022

/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}

I have no idea how to fix this cast. Do you have an idea? Doesgreenlet::refs::OwnedErrPiece inherit somehow from PyObject? Is it a pointer?

Maybe greenlet should be modified to cast toPyObject*.

Copy link

Contributor

tacaswell commentedMay 3, 2022

I have no idea how to fix this cast. Do you have an idea?

I think we need an actual C++ expert to tell us if what greenlets is doing is fully kosher or undefined behavior that just happened to work before.

Maybe greenlet should be modified to cast to PyObject*.

python-greenlet/greenlet@50f8786 is one way that works.

Copy link

Contributor

serge-sans-paille commentedMay 4, 2022

That's an interesting situation: greenlets are written in C++, and the object that raises issues is of typegreenlet::refs::OwnedErrPiece that has a conversion operator toPyObject* (namely:https://github.com/python-greenlet/greenlet/blob/be41e1a24925326b72a02ef5cb6d1ed9643eb062/src/greenlet/greenlet_refs.hpp#L924 ) Using C-style cast or static_cast directly uses that operator so that's fine. But a reinterpret_cast won't work.

According to me, this means we need something along these lines:

#  define _Py_reinterpret_cast(type, expr) _Py_reinterpret_cast_impl<type>(expr)template<typename type, typename expr_type>type _Py_reinterpret_cast_impl(expr_type* expr)  { return reinterpret_cast<type>(expr);}template<typename type, typename expr_type>type _Py_reinterpret_cast_impl(expr_type const * expr)  { return reinterpret_cast<type>(const_cast<expr_type*>(expr));}template<typename type, typename expr_type>type _Py_reinterpret_cast_impl(expr_type & expr)  { return static_cast<type>(expr);}template<typename type, typename expr_type>type _Py_reinterpret_cast_impl(expr_type const & expr)  { return static_cast<type>(const_cast<expr_type &>(expr));}

(seehttps://godbolt.org/z/9vdzj4fcn)