GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-91320: Add _Py_reinterpret_cast() macro by vstinner · Pull Request #91959 · python/cpython

Conversation

vstinner commented Apr 26, 2022

Copy link

Copy Markdown

Member

Fix C++ compiler warnings about "old-style cast"
(g++ -Wold-style-cast) in the Python C API. Use C++
reinterpret_cast<> and static_cast<> casts when the Python C API is
used in C++.

Example of fixed warning:

Include/object.h:107:43: error: use of old-style cast to
‘PyObject*’ {aka ‘struct _object*’} [-Werror=old-style-cast]
#define _PyObject_CAST(op) ((PyObject*)(op))

Add _Py_reinterpret_cast() and _Py_static_cast() macros.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          gh-91320: Add _Py_reinterpret_cast() macro

06aeb11

Fix C++ compiler warnings about "old-style cast"
(g++ -Wold-style-cast) in the Python C API.  Use C++
reinterpret_cast<> and static_cast<> casts when the Python C API is
used in C++.

Example of fixed warning:

    Include/object.h:107:43: error: use of old-style cast to
    ‘PyObject*’ {aka ‘struct _object*’} [-Werror=old-style-cast]
    #define _PyObject_CAST(op) ((PyObject*)(op))

Add _Py_reinterpret_cast() and _Py_static_cast() macros.

bedevere-bot added the awaiting core review label

Apr 26, 2022

vstinner commented Apr 26, 2022

Copy link

Copy Markdown

Member Author

cc @serge-sans-paille

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Fix typo in _PyCMethodObject_CAST()

01b1bae

vstinner mentioned this pull request

Apr 26, 2022

gh-91321: Add _testcppext C++ extension #32175

Merged

serge-sans-paille commented Apr 26, 2022

Copy link

Copy Markdown

Contributor

conceptually looks good to me :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Apr 26, 2022

Copy link

Copy Markdown

Member Author

cc @erlend-aasland @corona10

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented

Apr 26, 2022

View reviewed changes

Comment thread

Include/cpython/abstract.h


		#define PY_VECTORCALL_ARGUMENTS_OFFSET ((size_t)1 << (8 * sizeof(size_t) - 1))
		#define PY_VECTORCALL_ARGUMENTS_OFFSET \
		(_Py_static_cast(size_t, 1) << (8 * sizeof(size_t) - 1))

vstinner Apr 26, 2022

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro should be updated since it's used in a static inline function: PyVectorcall_NARGS().

It seems like the warning is emitted if an old-style cast is done in a static inline function, but it seems to be fine if it's only done in macros.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner commented

Apr 26, 2022

View reviewed changes

vstinner left a comment

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative is to don't add two macros, _Py_static_cast() and _Py_reinterpret_cast(), but only one _Py_CAST() macro which always uses reinterpret_cast<type>(expr) on C++.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner commented Apr 26, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member Author

An alternative is to don't add two macros, _Py_static_cast() and _Py_reinterpret_cast(), but only one _Py_CAST() macro which always uses reinterpret_cast(expr) on C++.

Sadly, size_t var = reinterpret_cast<size_t>(1); fails on C++, whereas size_t var = static_cast<int>(1); is fine. C++ requires to use right cast.

x.cpp:8:18: error: invalid cast from type 'int' to type 'size_t' {aka 'long unsigned int'}
    8 |     size_t var = reinterpret_cast<size_t>(1);
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~

I used static_cast<Py_ssize_t>(...) to fix C++ warnings in datatable: h2oai/datatable@753197c

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

corona10 approved these changes

Apr 27, 2022

View reviewed changes

corona10 left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-bot added awaiting merge and removed awaiting core review labels

Apr 27, 2022

vstinner merged commit 29e2245 into python:main

Apr 27, 2022

bedevere-bot removed the awaiting merge label

Apr 27, 2022

vstinner deleted the cpp_cast branch

April 27, 2022 08:41

vstinner commented Apr 27, 2022

Copy link

Copy Markdown

Member Author

Thanks for the reviews @serge-sans-paille and @corona10.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner mentioned this pull request

Apr 27, 2022

[C API] Add private "CAST" macros to clean up casts in C code #91320

Closed

erlend-aasland reviewed

Apr 27, 2022

View reviewed changes

erlend-aasland left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense; looks good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

tacaswell commented Apr 29, 2022

Copy link

Copy Markdown

Contributor

This suspect that this does not appear to preserve const correctness, I am seeing errors like:

$ 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::CreatedModule::PyAddObject(const char*, const PyObject*)’:
/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:20:44: error: ‘reinterpret_cast’ from type ‘const PyObject*’ {aka ‘const _object*’} to type ‘PyObject*’ {aka ‘_object*’} casts away qualifiers
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:505:35: note: in expansion of macro ‘_PyObject_CAST’
  505 | #  define Py_INCREF(op) Py_INCREF(_PyObject_CAST(op))
      |                                   ^~~~~~~~~~~~~~
src/greenlet/greenlet_refs.hpp:854:13: note: in expansion of macro ‘Py_INCREF’
  854 |             Py_INCREF(new_object);
      |             ^~~~~~~~~
src/greenlet/greenlet_refs.hpp: In member function ‘void greenlet::refs::PyErrPieces::normalize()’:
/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:20:44: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:20:44: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:20:44: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:20:44: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:20:44: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:20:44: error: invalid cast from type ‘greenlet::refs::BorrowedGreenlet’ {aka ‘greenlet::refs::_BorrowedGreenlet<_greenlet, greenlet::refs::GreenletChecker>’} to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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;
      |                                 ^~~~~~~
src/greenlet/greenlet_refs.hpp: In instantiation of ‘greenlet::refs::OwnedReference<T, <anonymous> >& greenlet::refs::OwnedReference<T, <anonymous> >::operator=(const greenlet::refs::OwnedReference<T, <anonymous> >&) [with T = _object; void (* TC)(void*) = greenlet::refs::NoOpChecker]’:
src/greenlet/greenlet_refs.hpp:908:11:   required from here
/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:20:44: error: ‘reinterpret_cast’ from type ‘const _object*’ to type ‘PyObject*’ {aka ‘_object*’} casts away qualifiers
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:605:37: note: in expansion of macro ‘_PyObject_CAST’
  605 | #  define Py_XDECREF(op) Py_XDECREF(_PyObject_CAST(op))
      |                                     ^~~~~~~~~~~~~~
src/greenlet/greenlet_refs.hpp:342:13: note: in expansion of macro ‘Py_XDECREF’
  342 |             Py_XDECREF(tmp);
      |             ^~~~~~~~~~
src/greenlet/greenlet_refs.hpp: In instantiation of ‘greenlet::refs::OwnedReference<T, <anonymous> >& greenlet::refs::OwnedReference<T, <anonymous> >::operator=(const greenlet::refs::OwnedReference<T, <anonymous> >&) [with T = _object; void (* TC)(void*) = greenlet::refs::ContextExactChecker]’:
src/greenlet/greenlet.cpp:2337:40:   required from here
/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:20:44: error: ‘reinterpret_cast’ from type ‘const _object*’ to type ‘PyObject*’ {aka ‘_object*’} casts away qualifiers
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:605:37: note: in expansion of macro ‘_PyObject_CAST’
  605 | #  define Py_XDECREF(op) Py_XDECREF(_PyObject_CAST(op))
      |                                     ^~~~~~~~~~~~~~
src/greenlet/greenlet_refs.hpp:342:13: note: in expansion of macro ‘Py_XDECREF’
  342 |             Py_XDECREF(tmp);
      |             ^~~~~~~~~~

when trying to build greenlets, however I have not bisected back to be sure this PR is at fault.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Apr 30, 2022

Copy link

Copy Markdown

Member Author

This suspect that this does not appear to preserve const correctness, I am seeing errors like:

The C API requires non-const PyObject*. Do you have an idea on how to fix the C++ cast to fix these errors?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tacaswell commented Apr 30, 2022

Copy link

Copy Markdown

Contributor

The C API requires non-const PyObject*. Do you have an idea on how to fix the C++ cast to fix these errors

My c++ is very rusty, but if the C API requires non-const there failures are correct. The casting to non-const should be done by greenlets (or what ever the calling library is as they should be aware that as escape hatch is being used). I will open an issue with them.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Apr 30, 2022

py311 issues with const correctness python-greenlet/greenlet#301

Closed

Clarify that const is being cast away python-greenlet/greenlet#302

Closed

vstinner commented May 1, 2022

Copy link

Copy Markdown

Member Author

Sadly, it's a Python regression compared to Python 3.10. It should be fixed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serge-sans-paille commented May 1, 2022 via email

Copy link

Copy Markdown

Contributor

You can perform a `const_cast` to by pass the constness check :-/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tacaswell commented May 1, 2022

Copy link

Copy Markdown

Contributor

Sadly, it's a Python regression compared to Python 3.10. It should be fixed.

Take my view with a grain of 🧂 , but I think you could argue that this was a bug in py310 and below where constness was incorrectly disregarded? Looking at the changes I had to make to greenlet to get it to compile again, I think they are all more "correct" than the code was before.

The const_cast changes were in code that already had a similar cast for dealing with the c++ side of their code, it only had to be added to the python macro calls.

This is fixing the errors to looked like:

Details

``` 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::CreatedModule::PyAddObject(const char*, const PyObject*)’: /home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:20:44: error: ‘reinterpret_cast’ from type ‘const PyObject*’ {aka ‘const _object*’} to type ‘PyObject*’ {aka ‘_object*’} casts away qualifiers 20 | # define _Py_reinterpret_cast(type, expr) reinterpret_cast(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:505:35: note: in expansion of macro ‘_PyObject_CAST’ 505 | # define Py_INCREF(op) Py_INCREF(_PyObject_CAST(op)) | ^~~~~~~~~~~~~~ src/greenlet/greenlet_refs.hpp:854:13: note: in expansion of macro ‘Py_INCREF’ 854 | Py_INCREF(new_object); | ^~~~~~~~~ ```

There is a another class of changes I had to make was to access the wrapped PyObject rather than blindly casting the wrapper object to pyobject (which I think used to work because in memory layout the PyObject was first so if you looked at that point in memory up to the length of PyObject you did indeed see a PyObject). It is not clear to me that you can move to the "new-style" casts and not have this issue. As with the const_cast, the resulting code reads as "more correct" to me than it did before.

These are the errors that look like:

Details

/home/tcaswell/.pybuild/bleeding/include/python3.11/pyport.h:20:44: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘PyObject*’ {aka ‘_object*’}
   20 | #  define _Py_reinterpret_cast(type, expr) reinterpret_cast<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:
```
</details>

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented May 2, 2022

Copy link

Copy Markdown

Member Author

PEP 670 was accepted with the condition: conversions must not introduce new compiler warnings (so no new compile errors neither). https://peps.python.org/pep-0670/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner mentioned this pull request

May 2, 2022

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

Closed

vstinner commented May 2, 2022

Copy link

Copy Markdown

Member Author

We are commenting a closed PR. I created a new issue to discuss the new C++ warning: #92135

erlend-aasland and tacaswell reacted with thumbs up emoji

All reactions

👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced May 14, 2022

PyUnicode_READ() fails to build on C++: _Py_CAST() used with a constant type (const void*) #92800

Closed

changes to PyObject casting macros causing build failures with python-greenlet #92898

Closed

nascheme mentioned this pull request

Jun 3, 2022

PyTuple_SET_ITEM fails to compile in C++ source #93442

Closed

Reviewers

erlend-aasland

erlend-aasland left review comments

corona10

corona10 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

vstinner commented Apr 26, 2022

Uh oh!

vstinner commented Apr 26, 2022

Uh oh!

serge-sans-paille commented Apr 26, 2022

Uh oh!

vstinner commented Apr 26, 2022

Uh oh!

vstinner Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corona10 left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Apr 27, 2022

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Apr 29, 2022

Uh oh!

vstinner commented Apr 30, 2022

Uh oh!

tacaswell commented Apr 30, 2022

Uh oh!

vstinner commented May 1, 2022

Uh oh!

serge-sans-paille commented May 1, 2022 via email

Uh oh!

tacaswell commented May 1, 2022

Uh oh!

vstinner commented May 2, 2022

Uh oh!

vstinner commented May 2, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vstinner commented Apr 26, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.