Skip to content

Commit

Permalink
gh-92135: Fix _Py_reinterpret_cast() for const (#92138)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vstinner authored May 2, 2022
1 parent b11243e commit 0313970
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 14 deletions.
3 changes: 1 addition & 2 deletions Include/methodobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ typedef PyObject *(*PyCMethod)(PyObject *, PyTypeObject *, PyObject *const *,
// it triggers an undefined behavior when Python calls it with 2 parameters
// (bpo-33012).
#define _PyCFunction_CAST(func) \
_Py_reinterpret_cast(PyCFunction, \
_Py_reinterpret_cast(void(*)(void), (func)))
_Py_reinterpret_cast(PyCFunction, _Py_reinterpret_cast(void(*)(void), (func)))

PyAPI_FUNC(PyCFunction) PyCFunction_GetFunction(PyObject *);
PyAPI_FUNC(PyObject *) PyCFunction_GetSelf(PyObject *);
Expand Down
4 changes: 2 additions & 2 deletions Include/objimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,9 @@ PyAPI_FUNC(void) PyObject_GC_UnTrack(void *);
PyAPI_FUNC(void) PyObject_GC_Del(void *);

#define PyObject_GC_New(type, typeobj) \
_Py_reinterpret_cast(type*, _PyObject_GC_New(typeobj))
_Py_reinterpret_cast(type*, _PyObject_GC_New(typeobj))
#define PyObject_GC_NewVar(type, typeobj, n) \
_Py_reinterpret_cast(type*, _PyObject_GC_NewVar((typeobj), (n)))
_Py_reinterpret_cast(type*, _PyObject_GC_NewVar((typeobj), (n)))

PyAPI_FUNC(int) PyObject_GC_IsTracked(PyObject *);
PyAPI_FUNC(int) PyObject_GC_IsFinalized(PyObject *);
Expand Down
20 changes: 15 additions & 5 deletions Include/pyport.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,20 @@
#endif


// Macro to use C++ static_cast<> and reinterpret_cast<> in the Python C API
// Macro to use C++ static_cast<>, reinterpret_cast<> and const_cast<>
// in the Python C API.
//
// In C++, _Py_reinterpret_cast(type, expr) converts a constant expression to a
// non constant type using const_cast<type>. For example,
// _Py_reinterpret_cast(PyObject*, op) can convert a "const PyObject*" to
// "PyObject*".
//
// The type argument must not be constant. For example, in C++,
// _Py_reinterpret_cast(const PyObject*, expr) fails with a compiler error.
#ifdef __cplusplus
# define _Py_static_cast(type, expr) static_cast<type>(expr)
# define _Py_reinterpret_cast(type, expr) reinterpret_cast<type>(expr)
# define _Py_reinterpret_cast(type, expr) \
const_cast<type>(reinterpret_cast<const type>(expr))
#else
# define _Py_static_cast(type, expr) ((type)(expr))
# define _Py_reinterpret_cast(type, expr) ((type)(expr))
Expand Down Expand Up @@ -307,10 +317,10 @@ extern "C" {
*/
#ifdef Py_DEBUG
# define Py_SAFE_DOWNCAST(VALUE, WIDE, NARROW) \
(assert((WIDE)(NARROW)(VALUE) == (VALUE)), (NARROW)(VALUE))
(assert(_Py_static_cast(WIDE, _Py_static_cast(NARROW, (VALUE))) == (VALUE)), \
_Py_static_cast(NARROW, (VALUE)))
#else
# define Py_SAFE_DOWNCAST(VALUE, WIDE, NARROW) \
_Py_reinterpret_cast(NARROW, (VALUE))
# define Py_SAFE_DOWNCAST(VALUE, WIDE, NARROW) _Py_static_cast(NARROW, (VALUE))
#endif


Expand Down
31 changes: 31 additions & 0 deletions Lib/test/_testcppext.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// gh-91321: Very basic C++ test extension to check that the Python C API is
// compatible with C++ and does not emit C++ compiler warnings.

// Always enable assertions
#undef NDEBUG

#include "Python.h"

PyDoc_STRVAR(_testcppext_add_doc,
Expand All @@ -20,8 +23,36 @@ _testcppext_add(PyObject *Py_UNUSED(module), PyObject *args)
}


static PyObject *
test_api_casts(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
{
PyObject *obj = Py_BuildValue("(ii)", 1, 2);
if (obj == nullptr) {
return nullptr;
}

// gh-92138: For backward compatibility, functions of Python C API accepts
// "const PyObject*". Check that using it does not emit C++ compiler
// warnings.
const PyObject *const_obj = obj;
Py_INCREF(const_obj);
Py_DECREF(const_obj);
PyTypeObject *type = Py_TYPE(const_obj);
assert(Py_REFCNT(const_obj) >= 1);

assert(type == &PyTuple_Type);
assert(PyTuple_GET_SIZE(const_obj) == 2);
PyObject *one = PyTuple_GET_ITEM(const_obj, 0);
assert(PyLong_AsLong(one) == 1);

Py_DECREF(obj);
Py_RETURN_NONE;
}


static PyMethodDef _testcppext_methods[] = {
{"add", _testcppext_add, METH_VARARGS, _testcppext_add_doc},
{"test_api_casts", test_api_casts, METH_NOARGS, NULL},
{nullptr, nullptr, 0, nullptr} /* sentinel */
};

Expand Down
18 changes: 13 additions & 5 deletions Lib/test/test_cppext.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# gh-91321: Build a basic C++ test extension to check that the Python C API is
# compatible with C++ and does not emit C++ compiler warnings.
import contextlib
import os
import sys
import unittest
Expand Down Expand Up @@ -39,17 +40,24 @@ def build(self):
sources=[SOURCE],
language='c++',
extra_compile_args=CPPFLAGS)
capture_stdout = (not support.verbose)

try:
try:
with (support.captured_stdout() as stdout,
support.swap_attr(sys, 'argv', ['setup.py', 'build_ext'])):
if capture_stdout:
stdout = support.captured_stdout()
else:
print()
stdout = contextlib.nullcontext()
with (stdout,
support.swap_attr(sys, 'argv', ['setup.py', 'build_ext', '--verbose'])):
setup(name="_testcppext", ext_modules=[cpp_ext])
return
except:
# Show output on error
print()
print(stdout.getvalue())
if capture_stdout:
# Show output on error
print()
print(stdout.getvalue())
raise
except SystemExit:
self.fail("Build failed")
Expand Down

0 comments on commit 0313970

Please sign in to comment.