gh-94731: Revert to C-style casts for _Py_CAST (GH-94782)

Co-authored-by: da-woods <[email protected]>

Author: encukou

Include/pyport.h

@@ -14,62 +14,14 @@
 #endif
 
 
-// Macro to use C++ static_cast<>, reinterpret_cast<> and const_cast<>
-// in the Python C API.
-//
-// In C++, _Py_CAST(type, expr) converts a constant expression to a
-// non constant type using const_cast<type>. For example,
-// _Py_CAST(PyObject*, op) can convert a "const PyObject*" to
-// "PyObject*".
-//
-// The type argument must not be a constant type.
+// Macro to use C++ static_cast<> in the Python C API.
 #ifdef __cplusplus
-#include <cstddef>
 #  define _Py_STATIC_CAST(type, expr) static_cast<type>(expr)
-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);
-        }
-#if __cplusplus >= 201103
-        template <typename type>
-        inline type _Py_CAST_impl(std::nullptr_t) {
-            return static_cast<type>(nullptr);
-        }
-#endif
-
-        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)
-
 #else
 #  define _Py_STATIC_CAST(type, expr) ((type)(expr))
-#  define _Py_CAST(type, expr) ((type)(expr))
 #endif
+// Macro to use the more powerful/dangerous C-style cast even in C++.
+#define _Py_CAST(type, expr) ((type)(expr))
 
 // Static inline functions should use _Py_NULL rather than using directly NULL
 // to prevent C++ compiler warnings. On C++11 and newer, _Py_NULL is defined as

Lib/test/_testcppext.cpp

@@ -12,6 +12,9 @@
 #  define NAME _testcpp03ext
 #endif
 
+#define _STR(NAME) #NAME
+#define STR(NAME) _STR(NAME)
+
 PyDoc_STRVAR(_testcppext_add_doc,
 "add(x, y)\n"
 "\n"
@@ -123,11 +126,77 @@ test_unicode(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
     Py_RETURN_NONE;
 }
 
+/* Test a `new`-allocated object with a virtual method.
+ * (https://github.com/python/cpython/issues/94731) */
+
+class VirtualPyObject : public PyObject {
+public:
+    VirtualPyObject();
+    virtual ~VirtualPyObject() {
+        delete [] internal_data;
+        --instance_count;
+    }
+    virtual void set_internal_data() {
+        internal_data[0] = 1;
+    }
+    static void dealloc(PyObject* o) {
+        delete static_cast<VirtualPyObject*>(o);
+    }
+
+    // Number of "living" instances
+    static int instance_count;
+private:
+    // buffer that can get corrupted
+    int* internal_data;
+};
+
+int VirtualPyObject::instance_count = 0;
+
+PyType_Slot VirtualPyObject_Slots[] = {
+    {Py_tp_free, (void*)VirtualPyObject::dealloc},
+    {0, _Py_NULL},
+};
+
+PyType_Spec VirtualPyObject_Spec = {
+    /* .name */ STR(NAME) ".VirtualPyObject",
+    /* .basicsize */ sizeof(VirtualPyObject),
+    /* .itemsize */ 0,
+    /* .flags */ Py_TPFLAGS_DEFAULT,
+    /* .slots */ VirtualPyObject_Slots,
+};
+
+VirtualPyObject::VirtualPyObject() {
+    // Create a temporary type (just so we don't need to store it)
+    PyObject *type = PyType_FromSpec(&VirtualPyObject_Spec);
+    // no good way to signal failure from a C++ constructor, so use assert
+    // for error handling
+    assert(type);
+    assert(PyObject_Init(this, (PyTypeObject *)type));
+    Py_DECREF(type);
+    internal_data = new int[50];
+    ++instance_count;
+}
+
+static PyObject *
+test_virtual_object(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
+{
+    VirtualPyObject* obj = new VirtualPyObject();
+    obj->set_internal_data();
+    Py_DECREF(obj);
+    if (VirtualPyObject::instance_count != 0) {
+        return PyErr_Format(
+            PyExc_AssertionError,
+            "instance_count should be 0, got %d",
+            VirtualPyObject::instance_count);
+    }
+    Py_RETURN_NONE;
+}
 
 static PyMethodDef _testcppext_methods[] = {
     {"add", _testcppext_add, METH_VARARGS, _testcppext_add_doc},
     {"test_api_casts", test_api_casts, METH_NOARGS, _Py_NULL},
     {"test_unicode", test_unicode, METH_NOARGS, _Py_NULL},
+    {"test_virtual_object", test_virtual_object, METH_NOARGS, _Py_NULL},
     // Note: _testcppext_exec currently runs all test functions directly.
     // When adding a new one, add a call there.
 
@@ -152,6 +221,10 @@ _testcppext_exec(PyObject *module)
     if (!result) return -1;
     Py_DECREF(result);
 
+    result = PyObject_CallMethod(module, "test_virtual_object", "");
+    if (!result) return -1;
+    Py_DECREF(result);
+
     return 0;
 }
 
@@ -163,9 +236,6 @@ static PyModuleDef_Slot _testcppext_slots[] = {
 
 PyDoc_STRVAR(_testcppext_doc, "C++ test extension.");
 
-#define _STR(NAME) #NAME
-#define STR(NAME) _STR(NAME)
-
 static struct PyModuleDef _testcppext_module = {
     PyModuleDef_HEAD_INIT,  // m_base
     STR(NAME),  // m_name

Lib/test/setup_testcppext.py

@@ -17,8 +17,6 @@
         # a C++ extension using the Python C API does not emit C++ compiler
         # warnings
         '-Werror',
-        # Warn on old-style cast (C cast) like: (PyObject*)op
-        '-Wold-style-cast',
     ]
 else:
     # Don't pass any compiler flag to MSVC
@@ -37,9 +35,6 @@ def main():
         name = '_testcpp11ext'
 
     cppflags = [*CPPFLAGS, f'-std={std}']
-    if std == 'c++11':
-        # Warn when using NULL rather than _Py_NULL in static inline functions
-        cppflags.append('-Wzero-as-null-pointer-constant')
 
     cpp_ext = Extension(
         name,

Lib/test/test_cppext.py

@@ -4,6 +4,7 @@
 import sys
 import unittest
 import subprocess
+import sysconfig
 from test import support
 from test.support import os_helper
 
@@ -25,6 +26,11 @@ def test_build_cpp03(self):
     # With MSVC, the linker fails with: cannot open file 'python311.lib'
     # https://github.com/python/cpython/pull/32175#issuecomment-1111175897
     @unittest.skipIf(MS_WINDOWS, 'test fails on Windows')
+    # Building and running an extension in clang sanitizing mode is not
+    # straightforward
+    @unittest.skipIf(
+        '-fsanitize' in (sysconfig.get_config_var('PY_CFLAGS') or ''),
+        'test does not work with analyzing builds')
     # the test uses venv+pip: skip if it's not available
     @support.requires_venv_with_pip()
     def check_build(self, std_cpp03, extension_name):

Misc/NEWS.d/next/C API/2022-07-12-17-39-32.gh-issue-94731.9CPJNU.rst

@@ -0,0 +1,3 @@
+Python again uses C-style casts for most casting operations when compiled
+with C++. This may trigger compiler warnings, if they are enabled with e.g.
+``-Wold-style-cast `` or ``-Wzero-as-null-pointer-constant`` options for ``g++``.