gh-93274: Make vectorcall safe on mutable classes & inherit it by default (#95437)

Doc/c-api/call.rst

@@ -57,6 +57,15 @@ This bears repeating:
    A class supporting vectorcall **must** also implement
    :c:member:`~PyTypeObject.tp_call` with the same semantics.
 
+.. versionchanged:: 3.12
+
+   The :const:`Py_TPFLAGS_HAVE_VECTORCALL` flag is now removed from a class
+   when the class's :py:meth:`~object.__call__` method is reassigned.
+   (This internally sets :c:member:`~PyTypeObject.tp_call` only, and thus
+   may make it behave differently than the vectorcall function.)
+   In earlier Python versions, vectorcall should only be used with
+   :const:`immutable <Py_TPFLAGS_IMMUTABLETYPE>` or static types.
+
 A class should not implement vectorcall if that would be slower
 than *tp_call*. For example, if the callee needs to convert
 the arguments to an args tuple and kwargs dict anyway, then there is no point

Doc/c-api/typeobj.rst

@@ -720,29 +720,29 @@ and :c:type:`PyType_Type` effectively act as defaults.)
    with the *vectorcallfunc* function.
    This can be done by setting *tp_call* to :c:func:`PyVectorcall_Call`.
 
-   .. warning::
-
-      It is not recommended for :ref:`mutable heap types <heap-types>` to implement
-      the vectorcall protocol.
-      When a user sets :attr:`__call__` in Python code, only *tp_call* is updated,
-      likely making it inconsistent with the vectorcall function.
-
    .. versionchanged:: 3.8
 
       Before version 3.8, this slot was named ``tp_print``.
       In Python 2.x, it was used for printing to a file.
       In Python 3.0 to 3.7, it was unused.
 
+   .. versionchanged:: 3.12
+
+      Before version 3.12, it was not recommended for
+      :ref:`mutable heap types <heap-types>` to implement the vectorcall
+      protocol.
+      When a user sets :attr:`~type.__call__` in Python code, only *tp_call* is
+      updated, likely making it inconsistent with the vectorcall function.
+      Since 3.12, setting ``__call__`` will disable vectorcall optimization
+      by clearing the :const:`Py_TPFLAGS_HAVE_VECTORCALL` flag.
+
    **Inheritance:**
 
    This field is always inherited.
    However, the :const:`Py_TPFLAGS_HAVE_VECTORCALL` flag is not
-   always inherited. If it's not, then the subclass won't use
+   always inherited. If it's not set, then the subclass won't use
    :ref:`vectorcall <vectorcall>`, except when
    :c:func:`PyVectorcall_Call` is explicitly called.
-   This is in particular the case for types without the
-   :const:`Py_TPFLAGS_IMMUTABLETYPE` flag set (including subclasses defined in
-   Python).
 
 
 .. c:member:: getattrfunc PyTypeObject.tp_getattr
@@ -1178,12 +1178,18 @@ and :c:type:`PyType_Type` effectively act as defaults.)
 
       **Inheritance:**
 
-      This bit is inherited for types with the
+      This bit is inherited if :c:member:`~PyTypeObject.tp_call` is also
-      :const:`Py_TPFLAGS_IMMUTABLETYPE` flag set, if
+      inherited.
-      :c:member:`~PyTypeObject.tp_call` is also inherited.
 
       .. versionadded:: 3.9
 
+      .. versionchanged:: 3.12
+
+         This flag is now removed from a class when the class's
+         :py:meth:`~object.__call__` method is reassigned.
+
+         This flag can now be inherited by mutable classes.
+
    .. data:: Py_TPFLAGS_IMMUTABLETYPE
 
       This bit is set for type objects that are immutable: type attributes cannot be set nor deleted.

Doc/whatsnew/3.12.rst

@@ -414,6 +414,15 @@ New Features
   an additional metaclass argument.
   (Contributed by Wenzel Jakob in :gh:`93012`.)
 
+* (XXX: this should be combined with :gh:`93274` when that is done)
+  The :const:`Py_TPFLAGS_HAVE_VECTORCALL` flag is now removed from a class
+  when the class's :py:meth:`~object.__call__` method is reassigned.
+  This makes vectorcall safe to use with mutable types (i.e. heap types
+  without the :const:`immutable <Py_TPFLAGS_IMMUTABLETYPE>` flag).
+  Mutable types that do not override :c:member:`~PyTypeObject.tp_call` now
+  inherit the :const:`Py_TPFLAGS_HAVE_VECTORCALL` flag.
+  (Contributed by Petr Viktorin in :gh:`93012`.)
+
 Porting to Python 3.12
 ----------------------
 

Lib/test/test_call.py

@@ -606,9 +606,19 @@ class TestPEP590(unittest.TestCase):
         self.assertFalse(_testcapi.MethodDescriptorNopGet.__flags__ & Py_TPFLAGS_HAVE_VECTORCALL)
         self.assertTrue(_testcapi.MethodDescriptor2.__flags__ & Py_TPFLAGS_HAVE_VECTORCALL)
 
-        # Mutable heap types should not inherit Py_TPFLAGS_HAVE_VECTORCALL
+        # Mutable heap types should inherit Py_TPFLAGS_HAVE_VECTORCALL,
+        # but should lose it when __call__ is overridden
         class MethodDescriptorHeap(_testcapi.MethodDescriptorBase):
             pass
+        self.assertTrue(MethodDescriptorHeap.__flags__ & Py_TPFLAGS_HAVE_VECTORCALL)
+        MethodDescriptorHeap.__call__ = print
+        self.assertFalse(MethodDescriptorHeap.__flags__ & Py_TPFLAGS_HAVE_VECTORCALL)
+
+        # Mutable heap types should not inherit Py_TPFLAGS_HAVE_VECTORCALL if
+        # they define __call__ directly
+        class MethodDescriptorHeap(_testcapi.MethodDescriptorBase):
+            def __call__(self):
+                pass
         self.assertFalse(MethodDescriptorHeap.__flags__ & Py_TPFLAGS_HAVE_VECTORCALL)
 
     def test_vectorcall_override(self):
@@ -621,6 +631,58 @@ class TestPEP590(unittest.TestCase):
         f = _testcapi.MethodDescriptorNopGet()
         self.assertIs(f(*args), args)
 
+    def test_vectorcall_override_on_mutable_class(self):
+        """Setting __call__ should disable vectorcall"""
+        TestType = _testcapi.make_vectorcall_class()
+        instance = TestType()
+        self.assertEqual(instance(), "tp_call")
+        instance.set_vectorcall(TestType)
+        self.assertEqual(instance(), "vectorcall")  # assume vectorcall is used
+        TestType.__call__ = lambda self: "custom"
+        self.assertEqual(instance(), "custom")
+
+    def test_vectorcall_override_with_subclass(self):
+        """Setting __call__ on a superclass should disable vectorcall"""
+        SuperType = _testcapi.make_vectorcall_class()
+        class DerivedType(SuperType):
+            pass
+
+        instance = DerivedType()
+
+        # Derived types with its own vectorcall should be unaffected
+        UnaffectedType1 = _testcapi.make_vectorcall_class(DerivedType)
+        UnaffectedType2 = _testcapi.make_vectorcall_class(SuperType)
+
+        # Aside: Quickly check that the C helper actually made derived types
+        self.assertTrue(issubclass(UnaffectedType1, DerivedType))
+        self.assertTrue(issubclass(UnaffectedType2, SuperType))
+
+        # Initial state: tp_call
+        self.assertEqual(instance(), "tp_call")
+        self.assertEqual(_testcapi.has_vectorcall_flag(SuperType), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(DerivedType), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(UnaffectedType1), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(UnaffectedType2), True)
+
+        # Setting the vectorcall function
+        instance.set_vectorcall(SuperType)
+
+        self.assertEqual(instance(), "vectorcall")
+        self.assertEqual(_testcapi.has_vectorcall_flag(SuperType), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(DerivedType), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(UnaffectedType1), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(UnaffectedType2), True)
+
+        # Setting __call__ should remove vectorcall from all subclasses
+        SuperType.__call__ = lambda self: "custom"
+
+        self.assertEqual(instance(), "custom")
+        self.assertEqual(_testcapi.has_vectorcall_flag(SuperType), False)
+        self.assertEqual(_testcapi.has_vectorcall_flag(DerivedType), False)
+        self.assertEqual(_testcapi.has_vectorcall_flag(UnaffectedType1), True)
+        self.assertEqual(_testcapi.has_vectorcall_flag(UnaffectedType2), True)
+
+
     def test_vectorcall(self):
         # Test a bunch of different ways to call objects:
         # 1. vectorcall using PyVectorcall_Call()

Misc/NEWS.d/next/C API/2022-07-29-15-24-45.gh-issue-93012.-DdGEy.rst

@@ -0,0 +1,6 @@
+The :const:`Py_TPFLAGS_HAVE_VECTORCALL` flag is now removed from a class
+when the class's :py:meth:`~object.__call__` method is reassigned. This
+makes vectorcall safe to use with mutable types (i.e. heap types without the
+:const:`immutable <Py_TPFLAGS_IMMUTABLETYPE>` flag). Mutable types that do
+not override :c:member:`~PyTypeObject.tp_call` now inherit the
+:const:`Py_TPFLAGS_HAVE_VECTORCALL` flag.

Modules/_testcapi/clinic/vectorcall.c.h

@@ -0,0 +1,107 @@
+/*[clinic input]
+preserve
+[clinic start generated code]*/
+
+PyDoc_STRVAR(_testcapi_VectorCallClass_set_vectorcall__doc__,
+"set_vectorcall($self, type, /)\n"
+"--\n"
+"\n"
+"Set self\'s vectorcall function for `type` to one that returns \"vectorcall\"");
+
+#define _TESTCAPI_VECTORCALLCLASS_SET_VECTORCALL_METHODDEF    \
+    {"set_vectorcall", (PyCFunction)_testcapi_VectorCallClass_set_vectorcall, METH_O, _testcapi_VectorCallClass_set_vectorcall__doc__},
+
+static PyObject *
+_testcapi_VectorCallClass_set_vectorcall_impl(PyObject *self,
+                                              PyTypeObject *type);
+
+static PyObject *
+_testcapi_VectorCallClass_set_vectorcall(PyObject *self, PyObject *arg)
+{
+    PyObject *return_value = NULL;
+    PyTypeObject *type;
+
+    if (!PyObject_TypeCheck(arg, &PyType_Type)) {
+        _PyArg_BadArgument("set_vectorcall", "argument", (&PyType_Type)->tp_name, arg);
+        goto exit;
+    }
+    type = (PyTypeObject *)arg;
+    return_value = _testcapi_VectorCallClass_set_vectorcall_impl(self, type);
+
+exit:
+    return return_value;
+}
+
+PyDoc_STRVAR(_testcapi_make_vectorcall_class__doc__,
+"make_vectorcall_class($module, base=<unrepresentable>, /)\n"
+"--\n"
+"\n"
+"Create a class whose instances return \"tpcall\" when called.\n"
+"\n"
+"When the \"set_vectorcall\" method is called on an instance, a vectorcall\n"
+"function that returns \"vectorcall\" will be installed.");
+
+#define _TESTCAPI_MAKE_VECTORCALL_CLASS_METHODDEF    \
+    {"make_vectorcall_class", _PyCFunction_CAST(_testcapi_make_vectorcall_class), METH_FASTCALL, _testcapi_make_vectorcall_class__doc__},
+
+static PyObject *
+_testcapi_make_vectorcall_class_impl(PyObject *module, PyTypeObject *base);
+
+static PyObject *
+_testcapi_make_vectorcall_class(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
+{
+    PyObject *return_value = NULL;
+    PyTypeObject *base = NULL;
+
+    if (!_PyArg_CheckPositional("make_vectorcall_class", nargs, 0, 1)) {
+        goto exit;
+    }
+    if (nargs < 1) {
+        goto skip_optional;
+    }
+    if (!PyObject_TypeCheck(args[0], &PyType_Type)) {
+        _PyArg_BadArgument("make_vectorcall_class", "argument 1", (&PyType_Type)->tp_name, args[0]);
+        goto exit;
+    }
+    base = (PyTypeObject *)args[0];
+skip_optional:
+    return_value = _testcapi_make_vectorcall_class_impl(module, base);
+
+exit:
+    return return_value;
+}
+
+PyDoc_STRVAR(_testcapi_has_vectorcall_flag__doc__,
+"has_vectorcall_flag($module, type, /)\n"
+"--\n"
+"\n"
+"Return true iff Py_TPFLAGS_HAVE_VECTORCALL is set on the class.");
+
+#define _TESTCAPI_HAS_VECTORCALL_FLAG_METHODDEF    \
+    {"has_vectorcall_flag", (PyCFunction)_testcapi_has_vectorcall_flag, METH_O, _testcapi_has_vectorcall_flag__doc__},
+
+static int
+_testcapi_has_vectorcall_flag_impl(PyObject *module, PyTypeObject *type);
+
+static PyObject *
+_testcapi_has_vectorcall_flag(PyObject *module, PyObject *arg)
+{
+    PyObject *return_value = NULL;
+    PyTypeObject *type;
+    int _return_value;
+
+    if (!PyObject_TypeCheck(arg, &PyType_Type)) {
+        _PyArg_BadArgument("has_vectorcall_flag", "argument", (&PyType_Type)->tp_name, arg);
+        goto exit;
+    }
+    type = (PyTypeObject *)arg;
+    _return_value = _testcapi_has_vectorcall_flag_impl(module, type);
+    if ((_return_value == -1) && PyErr_Occurred()) {
+        goto exit;
+    }
+    return_value = PyBool_FromLong((long)_return_value);
+
+exit:
+    return return_value;
+}
+/*[clinic end generated code: output=cf39927be151aebd input=a9049054013a1b77]*/

Modules/_testcapi/vectorcall.c

@@ -1,5 +1,8 @@
 #include "parts.h"
-#include <stddef.h>  // offsetof
+#include "clinic/vectorcall.c.h"
+
+#include "structmember.h"           // PyMemberDef
+#include <stddef.h>                 // offsetof
 
 
 /* Test PEP 590 - Vectorcall */
@@ -122,11 +125,128 @@ test_pyvectorcall_call(PyObject *self, PyObject *args)
     return PyVectorcall_Call(func, argstuple, kwargs);
 }
 
+PyObject *
+VectorCallClass_tpcall(PyObject *self, PyObject *args, PyObject *kwargs) {
+    return PyUnicode_FromString("tp_call");
+}
+
+PyObject *
+VectorCallClass_vectorcall(PyObject *callable,
+                            PyObject *const *args,
+                            size_t nargsf,
+                            PyObject *kwnames) {
+    return PyUnicode_FromString("vectorcall");
+}
+
+/*[clinic input]
+module _testcapi
+class _testcapi.VectorCallClass "PyObject *" "&PyType_Type"
+[clinic start generated code]*/
+/*[clinic end generated code: output=da39a3ee5e6b4b0d input=8423a8e919f2f0df]*/
+
+/*[clinic input]
+_testcapi.VectorCallClass.set_vectorcall
+
+    type: object(subclass_of="&PyType_Type", type="PyTypeObject *")
+    /
+
+Set self's vectorcall function for `type` to one that returns "vectorcall"
+[clinic start generated code]*/
+
+static PyObject *
+_testcapi_VectorCallClass_set_vectorcall_impl(PyObject *self,
+                                              PyTypeObject *type)
+/*[clinic end generated code: output=b37f0466f15da903 input=840de66182c7d71a]*/
+{
+    if (!PyObject_TypeCheck(self, type)) {
+        return PyErr_Format(
+            PyExc_TypeError,
+            "expected %s instance",
+            PyType_GetName(type));
+    }
+    if (!type->tp_vectorcall_offset) {
+        return PyErr_Format(
+            PyExc_TypeError,
+            "type %s has no vectorcall offset",
+            PyType_GetName(type));
+    }
+    *(vectorcallfunc*)((char*)self + type->tp_vectorcall_offset) = (
+        VectorCallClass_vectorcall);
+    Py_RETURN_NONE;
+}
+
+PyMethodDef VectorCallClass_methods[] = {
+    _TESTCAPI_VECTORCALLCLASS_SET_VECTORCALL_METHODDEF
+    {NULL, NULL}
+};
+
+PyMemberDef VectorCallClass_members[] = {
+    {"__vectorcalloffset__", T_PYSSIZET, 0/* set later */, READONLY},
+    {NULL}
+};
+
+PyType_Slot VectorCallClass_slots[] = {
+    {Py_tp_call, VectorCallClass_tpcall},
+    {Py_tp_members, VectorCallClass_members},
+    {Py_tp_methods, VectorCallClass_methods},
+    {0},
+};
+
+/*[clinic input]
+_testcapi.make_vectorcall_class
+
+    base: object(subclass_of="&PyType_Type", type="PyTypeObject *") = NULL
+    /
+
+Create a class whose instances return "tpcall" when called.
+
+When the "set_vectorcall" method is called on an instance, a vectorcall
+function that returns "vectorcall" will be installed.
+[clinic start generated code]*/
+
+static PyObject *
+_testcapi_make_vectorcall_class_impl(PyObject *module, PyTypeObject *base)
+/*[clinic end generated code: output=16dcfc3062ddf968 input=f72e01ccf52de2b4]*/
+{
+    if (!base) {
+        base = (PyTypeObject *)&PyBaseObject_Type;
+    }
+    VectorCallClass_members[0].offset = base->tp_basicsize;
+    PyType_Spec spec = {
+        .name = "_testcapi.VectorcallClass",
+        .basicsize = base->tp_basicsize + (int)sizeof(vectorcallfunc),
+        .flags = Py_TPFLAGS_DEFAULT
+            | Py_TPFLAGS_HAVE_VECTORCALL
+            | Py_TPFLAGS_BASETYPE,
+        .slots = VectorCallClass_slots,
+    };
+
+    return PyType_FromSpecWithBases(&spec, (PyObject *)base);
+}
+
+/*[clinic input]
+_testcapi.has_vectorcall_flag -> bool
+
+    type: object(subclass_of="&PyType_Type", type="PyTypeObject *")
+    /
+
+Return true iff Py_TPFLAGS_HAVE_VECTORCALL is set on the class.
+[clinic start generated code]*/
+
+static int
+_testcapi_has_vectorcall_flag_impl(PyObject *module, PyTypeObject *type)
+/*[clinic end generated code: output=3ae8d1374388c671 input=8eee492ac548749e]*/
+{
+    return PyType_HasFeature(type, Py_TPFLAGS_HAVE_VECTORCALL);
+}
+
 static PyMethodDef TestMethods[] = {
     {"pyobject_fastcall", test_pyobject_fastcall, METH_VARARGS},
     {"pyobject_fastcalldict", test_pyobject_fastcalldict, METH_VARARGS},
     {"pyobject_vectorcall", test_pyobject_vectorcall, METH_VARARGS},
     {"pyvectorcall_call", test_pyvectorcall_call, METH_VARARGS},
+    _TESTCAPI_MAKE_VECTORCALL_CLASS_METHODDEF
+    _TESTCAPI_HAS_VECTORCALL_FLAG_METHODDEF
     {NULL},
 };
 

Objects/typeobject.c

@@ -6290,11 +6290,9 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
          * won't be used automatically. */
         COPYSLOT(tp_vectorcall_offset);
 
-        /* Inherit Py_TPFLAGS_HAVE_VECTORCALL for non-heap types
+        /* Inherit Py_TPFLAGS_HAVE_VECTORCALL if tp_call is not overridden */
-        * if tp_call is not overridden */
         if (!type->tp_call &&
-            _PyType_HasFeature(base, Py_TPFLAGS_HAVE_VECTORCALL) &&
+            _PyType_HasFeature(base, Py_TPFLAGS_HAVE_VECTORCALL))
-            _PyType_HasFeature(type, Py_TPFLAGS_IMMUTABLETYPE))
         {
             type->tp_flags |= Py_TPFLAGS_HAVE_VECTORCALL;
         }
@@ -8713,8 +8711,17 @@ update_one_slot(PyTypeObject *type, slotdef *p)
 {
     PyObject *descr;
     PyWrapperDescrObject *d;
-    void *generic = NULL, *specific = NULL;
+
+    // The correct specialized C function, like "tp_repr of str" in the
+    // example above
+    void *specific = NULL;
+
+    // A generic wrapper that uses method lookup (safe but slow)
+    void *generic = NULL;
+
+    // Set to 1 if the generic wrapper is necessary
     int use_generic = 0;
+
     int offset = p->offset;
     int error;
     void **ptr = slotptr(type, offset);
@@ -8797,6 +8804,10 @@ update_one_slot(PyTypeObject *type, slotdef *p)
         else {
             use_generic = 1;
             generic = p->function;
+            if (p->function == slot_tp_call) {
+                /* A generic __call__ is incompatible with vectorcall */
+                type->tp_flags &= ~Py_TPFLAGS_HAVE_VECTORCALL;
+            }
         }
     } while ((++p)->offset == offset);
     if (specific && !use_generic)