GeezerVison is showing github.com — PyTuple_SET_ITEM fails to compile in C++ source · Issue #93442 · python/cpython
PyTuple_SET_ITEM fails to compile in C++ source #93442Description
- CPython versions tested on: 3.11b3
- Operating system and architecture: Debian Linux, GCC 10.2.1-6
The kiwisolver extension doesn't compile with beta 3. The error is:
/usr/local/python3.11.0b3/include/python3.11/pyport.h:47:24: error: invalid ‘static_cast’ from type ‘int’ to type ‘_object*’
47 | return static_cast<type>(const_cast<expr_type &>(expr));
| ^~~~~
This seems to be an issue with the new inline functions for things that used to be macros. E.g. PyTuple_SET_ITEM. The C++ code triggering the issue is:
--- a/py/src/symbolics.h
+++ b/py/src/symbolics.h
@@ -123,7 +123,7 @@ PyObject* BinaryMul::operator()( Expression* first, double second )
return 0;
Py_ssize_t end = PyTuple_GET_SIZE( first->terms );
for( Py_ssize_t i = 0; i < end; ++i ) // memset 0 for safe error return
- PyTuple_SET_ITEM( terms.get(), i, 0 );
+ PyTuple_SetItem( terms.get(), i, 0 );
for( Py_ssize_t i = 0; i < end; ++i )
{
PyObject* item = PyTuple_GET_ITEM( first->terms, i );
Replacing the macro with the function version seems to allow the extension to compile.
This seems related to #92898 Activityvstinner commented on Jun 2, 2022 What is the result type of terms.get()? Can you write a short C++ code reproducing the issue?
I guess that the cast which fails is _PyObject_CAST(0): 3rd argument of PyTuple_SET_ITEM(). The function expects a PyObject* and you pass 0 as a pointer. nascheme commented on Jun 2, 2022 My understanding is that in C NULL and 0 can be used interchangeably. Maybe in modern C++ versions that's not the case?
https://stackoverflow.com/questions/176989/do-you-use-null-or-0-zero-for-pointers-in-c
For the kiwi source code, using NULL instead of 0 doesn't fix the error. Making a local variable for val does work:
diff --git a/py/src/util.h b/py/src/util.h
index 4b00fa7..6f51bca 100644
--- a/py/src/util.h
+++ b/py/src/util.h
@@ -116,8 +116,10 @@ make_terms( const std::map<PyObject*, double>& coeffs )
if( !terms )
return 0;
Py_ssize_t size = PyTuple_GET_SIZE( terms.get() );
- for( Py_ssize_t i = 0; i < size; ++i ) // zero tuple for safe early return
- PyTuple_SET_ITEM( terms.get(), i, 0 );
+ for( Py_ssize_t i = 0; i < size; ++i ) { // zero tuple for safe early return
+ PyObject *val = 0;
+ PyTuple_SET_ITEM( terms.get(), i, val );
+ }
Py_ssize_t i = 0;
iter_t it = coeffs.begin();
iter_t end = coeffs.end();
My guess is that the macro for PyTuple_GET_SIZE does not treat 0/NULL correctly but I don't know how you would fix it. vstinner commented on Jun 2, 2022
Maybe in modern C++ versions that's not the case?
In C++, nullptr should be used to pass a "null pointer" (0), no?
Well, Python should accept 0 as PyObject*. vstinner commented on Jun 2, 2022 The issue can be reproduced by running test_cppext with this change:
diff --git a/Lib/test/_testcppext.cpp b/Lib/test/_testcppext.cpp
index eade7ccdaa..70f434e678 100644
--- a/Lib/test/_testcppext.cpp
+++ b/Lib/test/_testcppext.cpp
@@ -74,6 +74,10 @@ test_api_casts(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
Py_INCREF(strong_ref);
Py_DECREF(strong_ref);
+ // gh-93442: Pass 0 as NULL for PyObject*
+ Py_XINCREF(0);
+ Py_XDECREF(0);
+
Py_DECREF(obj);
Py_RETURN_NONE;
}
nascheme commented on Jun 2, 2022 My guess is that the _Py_CAST_impl needs to have an overload to handle nullptr_t as the argument. Perhaps then _Py_NULL should not be needed. We would like that API users can still use 0 and NULL as args to these macros. nascheme commented on Jun 3, 2022 It looks like _Py_CAST_impl came about as a result of #91959. I wonder though if the cure is worse than the disease. I.e. previously we had warnings when you used -Wold-style-cast. Now you have C++ extensions having compile errors if you don't use nullptr instead of the old style NULL or 0. Can we just go back to:
#define _PyObject_CAST(op) ((PyObject *)(op))
and similar macros? Maybe the _Py_CAST_impl template magic can be opt-in in some way, for people who want to use -Wold-style-cast and not have errors. serge-sans-paille commented on Jun 3, 2022 The following overloads seem to be needed to cover all cases
#include <cstdlib>
typedef struct PyObject { int x ; } PyObject;
extern "C++" {
namespace {
template <typename type>
inline type _Py_CAST_impl(long int ptr) {
return reinterpret_cast<type>(ptr);
}
template <typename type>
inline type _Py_CAST_impl(int ptr) {
return reinterpret_cast<type>(ptr);
}
template <typename type>
inline type _Py_CAST_impl(std::nullptr_t) {
return static_cast<type>(nullptr);
}
template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type *expr) {
return reinterpret_cast<type>(expr);
}
template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type const *expr) {
return reinterpret_cast<type>(const_cast<expr_type *>(expr));
}
template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type &expr) {
return static_cast<type>(expr);
}
template <typename type, typename expr_type>
inline type _Py_CAST_impl(expr_type const &expr) {
return static_cast<type>(
const_cast<expr_type &>(expr)
);
}
}
}
# define _Py_CAST(type, expr) _Py_CAST_impl<type>(expr)
int main()
{
PyObject *obj = _Py_CAST(PyObject*, nullptr);
PyObject *obj2 = _Py_CAST(PyObject*, 0);
PyObject *obj3 = _Py_CAST(PyObject*, NULL);
return (obj == nullptr);
}
nascheme commented on Jun 3, 2022 I can confirm this change makes kiwi compile without having to make source code changes. 🎉 corona10 commented on Jun 4, 2022 added a commit that references this issue on Jun 5, 2022This issue will close once commit 5cf29ce is merged into the 'master' branch.5cf29ce
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|