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

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

Closed
da-woods opened this issue May 14, 2022 · 5 comments
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@da-woods
Copy link
Contributor

da-woods commented May 14, 2022

Bug report

The new _Py_CAST macro (https://github.com/python/cpython/pull/91959/files, and other commits) designed to reduce C++ warnings adds a fairly significant restriction that wasn't there before

cpython/Include/pyport.h

Lines 25 to 26 in 9f68dab

// The type argument must not be constant. For example, in C++,
// _Py_CAST(const PyObject*, expr) fails with a compiler error.

The PyUnicode_READ macro casts to a const pointer

#define PyUnicode_READ(kind, data, index) \
PyUnicode_READ(_Py_STATIC_CAST(int, kind), _Py_CAST(const void*, data), \
(index))

This is currently reported to be breaking Cython (cython/cython#4790).

I'd propose a fix which removes the const requirement:

template <typename T>
struct _Py_add_const {
    typedef const T type;
};
#  define _Py_CAST(tp, expr) \
       const_cast<tp>(reinterpret_cast<_Py_add_const<tp>::type>(expr))

on C++11 you could use the standard library std::add_cast instead, at the cost of an extra include. However it isn't available on c++03.

I realise this adds an extra struct _Py_add_const to the namespace. I'm not sure if that's desired, or how it should be named. If that's acceptable I'm happy to submit a PR for this.

@vstinner

Your environment

Current 3.11 branch

@da-woods da-woods added the type-bug An unexpected behavior, bug, or error label May 14, 2022
@da-woods da-woods changed the title C++-friendly _Py_CAST macro breaks unicodeobject.h on C++ C++-warningless _Py_CAST macro breaks unicodeobject.h on C++ May 14, 2022
@tacaswell
Copy link
Contributor

This issue is scattered across a couple of places (which may be my fault) here is an index of the relevant links:

@vstinner vstinner changed the title C++-warningless _Py_CAST macro breaks unicodeobject.h on C++ PyUnicode_READ() fails to build on C++: _Py_CAST() used with a constant type (const void*) May 17, 2022
@vstinner
Copy link
Member

This is currently reported to be breaking Cython (cython/cython#4790).

The C++ project scipy fails to build with:

/home/tcaswell/.pybuild/bleeding/include/python3.11/cpython/unicodeobject.h:409:57: error: duplicate ‘const’
  409 |     PyUnicode_READ(_Py_STATIC_CAST(int, kind), _Py_CAST(const void*, data), \
      |                                                         ^~~~~

Oh right, that's my fault, _Py_CAST() cannot be used with a const type. _Py_CAST() comment:

// The type argument must not be constant. For example, in C++,
// _Py_CAST(const PyObject*, expr) fails with a compiler error.

@vstinner
Copy link
Member

Would you be able to test if #92872 fix scipy?

@tacaswell
Copy link
Contributor

I can confirm that scipy (and a large fraction of the pydata stack) build with #92872 .

@vstinner
Copy link
Member

Fixed by #92872 : thanks for the bug report!

jessecomeau87 pushed a commit to jessecomeau87/Python that referenced this issue May 20, 2024
Thus fixing PyUnicode_READ in C++.
Fixes python/cpython#92800

I've created a C++ helper struct definition to work out the
type with constant added (it's OK to add const to a constant
type in typedefs in C++).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants