gh-114258: Use Argument Clinic for __len__ (GH-160012)

Author: corona10

Lib/test/test_clinic.py

@@ -23,8 +23,7 @@
     import libclinic
     from libclinic import ClinicError, unspecified, NULL, fail
     from libclinic.converters import (
-        int_converter, object_converter, str_converter, self_converter,
-        Py_ssize_t_converter)
+        int_converter, object_converter, str_converter, self_converter)
     from libclinic.function import (
         Module, Class, Function, FunctionKind, Parameter,
         permute_optional_groups, permute_right_option_groups,
@@ -35,7 +34,7 @@
     from libclinic.return_converters import return_converters, int_return_converter
     from libclinic.block_parser import Block, BlockParser
     from libclinic.codegen import BlockPrinter, Destination
-    from libclinic.dsl_parser import DSLParser
+    from libclinic.dsl_parser import DSLParser, unsupported_special_methods
     from libclinic.slots import SlotSpec, SLOTS
     from libclinic.cli import parse_file, Clinic
 
@@ -858,11 +857,10 @@ class Foo "FooObject *" "&Foo_Type"
 
     def test_slot_with_parameters_and_return_type(self):
         # Slots which take arguments or return something else than an
-        # object are not supported yet; check the machinery with fake ones.
+        # object are not supported yet; check the machinery with them.
         slots = {
-            '__contains__': SlotSpec('__contains__', 'int', 'int',
-                                     {(object_converter,): 'sq_contains'}),
-            '__bool__': SlotSpec('__bool__', 'int', 'int', {(): 'nb_bool'}),
+            '__contains__': SlotSpec(return_converter='int'),
+            '__bool__': SlotSpec(return_converter='int'),
         }
         block = """
             /*[clinic input]
@@ -3353,42 +3351,38 @@ class Foo "FooObject *" ""
             Foo.__next__
         """, signatures_in_block=3, function_index=2)
         self.assertIs(function.kind, FunctionKind.SLOT)
-        self.assertEqual(function.slot.name, "__next__")
+        self.assertIs(function.slot, SLOTS["__next__"])
         self.assertEqual(function.c_basename, "Foo_next")
         self.assertEqual(function.return_converter.type, "PyObject *")
         self.assertEqual(list(function.parameters), ["self"])
 
     def test_slot_parameters(self):
-        slot = SlotSpec('__contains__', 'int', 'int', {
-            (Py_ssize_t_converter,): 'sq_fake',
-            (object_converter,): 'sq_contains'})
+        # __getitem__ is not supported yet; its parameter gives the slot
+        # (Py_ssize_t for sq_item, object for mp_subscript) and is not parsed.
         block = """
             module foo
             class Foo "FooObject *" ""
-            Foo.__contains__
-                value: {converter}
+            Foo.__getitem__
+                key: {converter}
                 /
         """
         errors = [
-            ("Foo.__contains__",
-             "'__contains__' must declare (Py_ssize_t) for sq_fake or "
-             "(object) for sq_contains; got ()"),
-            ("Foo.__contains__\n    value: int\n    /",
-             "'__contains__' must declare"),
-            ("Foo.__contains__\n    value: object",
-             "The parameters of '__contains__' must be positional-only"),
-            ("Foo.__contains__\n    value: object = None\n    /",
-             "The parameters of '__contains__' cannot have a default value"),
+            ("Foo.__getitem__\n    key: object",
+             "The parameters of '__getitem__' must be positional-only"),
+            ("Foo.__getitem__\n    key: object = None\n    /",
+             "The parameters of '__getitem__' cannot have a default value"),
         ]
-        with mock.patch.dict(SLOTS, {'__contains__': slot}):
+        supported = unsupported_special_methods - {'__getitem__'}
+        with (mock.patch.dict(SLOTS, {'__getitem__': SlotSpec(return_converter='object')}),
+              mock.patch.object(libclinic.dsl_parser,
+                                'unsupported_special_methods', supported)):
             for converter in "Py_ssize_t", "object":
                 with self.subTest(converter=converter):
                     function = self.parse_function(
                         block.format(converter=converter),
                         signatures_in_block=3, function_index=2)
                     self.assertIs(function.kind, FunctionKind.SLOT)
-                    self.assertEqual(function.return_converter.type, "int")
-                    self.assertEqual(list(function.parameters), ["self", "value"])
+                    self.assertEqual(list(function.parameters), ["self", "key"])
             for body, err in errors:
                 with self.subTest(block=body):
                     body = 'class Foo "FooObject *" "&Foo_Type"\n' + body
@@ -3406,8 +3400,6 @@ def test_slot_errors(self):
              "@coexist cannot be applied to '__next__'"),
             ("@vectorcall\nFoo.__next__",
              "@vectorcall can only be used with __init__ and __new__"),
-            ("Foo.__next__\n    a: int",
-             "'__next__' methods cannot define parameters"),
             ("Foo.__next__ -> int",
              "'Foo.__next__' cannot define a return type"),
             ("Foo.__next__\n\nReturn the next item.",
@@ -3660,10 +3652,10 @@ def test_illegal_c_identifier(self):
         self.expect_failure(block, err, lineno=2)
 
     def test_cannot_convert_special_method(self):
-        err = "'__len__' is a special method and cannot be converted"
+        err = "'__iter__' is a special method and cannot be converted"
         block = """
             class T "" ""
-            T.__len__
+            T.__iter__
         """
         self.expect_failure(block, err, lineno=1)
 

Modules/_dbmmodule.c

@@ -116,10 +116,16 @@ dbm_dealloc(PyObject *self)
     Py_DECREF(tp);
 }
 
+/*[clinic input]
+@critical_section
+_dbm.dbm.__len__ as dbm_length
+[clinic start generated code]*/
+
 static Py_ssize_t
-dbm_length_lock_held(PyObject *self)
+dbm_length_impl(dbmobject *self)
+/*[clinic end generated code: output=d7ea9510562e5f86 input=f912d65e9b06fef5]*/
 {
-    dbmobject *dp = dbmobject_CAST(self);
+    dbmobject *dp = self;
     _dbm_state *state = PyType_GetModuleState(Py_TYPE(dp));
     assert(state != NULL);
     if (dp->di_dbm == NULL) {
@@ -139,16 +145,6 @@ dbm_length_lock_held(PyObject *self)
     return dp->di_size;
 }
 
-static Py_ssize_t
-dbm_length(PyObject *self)
-{
-    Py_ssize_t result;
-    Py_BEGIN_CRITICAL_SECTION(self);
-    result = dbm_length_lock_held(self);
-    Py_END_CRITICAL_SECTION();
-    return result;
-}
-
 static int
 dbm_bool_lock_held(PyObject *self)
 {

Modules/_gdbmmodule.c

@@ -136,10 +136,16 @@ gdbm_dealloc(PyObject *op)
     Py_DECREF(tp);
 }
 
+/*[clinic input]
+@critical_section
+_gdbm.gdbm.__len__ as gdbm_length
+[clinic start generated code]*/
+
 static Py_ssize_t
-gdbm_length_lock_held(PyObject *op)
+gdbm_length_impl(gdbmobject *self)
+/*[clinic end generated code: output=3ccb956974ed73b5 input=57878f60d264ed98]*/
 {
-    gdbmobject *dp = _gdbmobject_CAST(op);
+    gdbmobject *dp = self;
     _gdbm_state *state = PyType_GetModuleState(Py_TYPE(dp));
     if (dp->di_dbm == NULL) {
         PyErr_SetString(state->gdbm_error, "GDBM object has already been closed");
@@ -183,16 +189,6 @@ gdbm_length_lock_held(PyObject *op)
     return dp->di_size;
 }
 
-static Py_ssize_t
-gdbm_length(PyObject *op)
-{
-    Py_ssize_t result;
-    Py_BEGIN_CRITICAL_SECTION(op);
-    result = gdbm_length_lock_held(op);
-    Py_END_CRITICAL_SECTION();
-    return result;
-}
-
 static int
 gdbm_bool_lock_held(PyObject *op)
 {

Modules/clinic/_dbmmodule.c.h

@@ -1487,24 +1487,19 @@ mmap_buffer_releasebuf(PyObject *op, Py_buffer *Py_UNUSED(view))
     Py_END_CRITICAL_SECTION();
 }
 
+/*[clinic input]
+@critical_section
+mmap.mmap.__len__ as mmap_length
+[clinic start generated code]*/
+
 static Py_ssize_t
-mmap_length_lock_held(PyObject *op)
+mmap_length_impl(mmap_object *self)
+/*[clinic end generated code: output=a63e62615cdd9960 input=b7bbf695dc9b2b66]*/
 {
-    mmap_object *self = mmap_object_CAST(op);
     CHECK_VALID(-1);
     return self->size;
 }
 
-static Py_ssize_t
-mmap_length(PyObject *op)
-{
-    Py_ssize_t result;
-    Py_BEGIN_CRITICAL_SECTION(op);
-    result = mmap_length_lock_held(op);
-    Py_END_CRITICAL_SECTION();
-    return result;
-}
-
 static PyObject *
 mmap_item_lock_held(PyObject *op, Py_ssize_t i)
 {

Modules/clinic/_gdbmmodule.c.h

@@ -72,7 +72,6 @@
 __itruediv__
 __ixor__
 __le__
-__len__
 __lshift__
 __lt__
 __matmul__
@@ -695,7 +694,7 @@ def resolve_return_converter(
             return int_return_converter()
         if self.kind is SLOT:
             assert self.slot is not None
-            return return_converters[self.slot.return_converter]()
+            return self.slot.make_return_converter()
         return CReturnConverter()
 
     def parse_cloned_function(self, names: FunctionNames, existing: str) -> None:
@@ -920,11 +919,6 @@ def state_parameters_start(self, line: str) -> None:
         assert self.function is not None
         if self.function.kind is GETTER:
             fail("@getter methods cannot define parameters")
-        if self.function.kind is SLOT:
-            assert self.function.slot is not None
-            if not self.function.slot.takes_parameters:
-                fail(f"{self.function.name!r} methods cannot define parameters")
-
         self.parameter_continuation = ''
         return self.next(self.state_parameter, line)
 
@@ -1677,19 +1671,12 @@ def check_vectorcall_parameters(self, lineno: int) -> None:
                      line_number=lineno)
 
     def check_slot_parameters(self, func: Function) -> None:
-        """Check that the parameters of a slot method select a C slot."""
-        assert func.slot is not None
-        params = list(func.parameters.values())[1:]
-        for p in params:
+        """Check that the parameters of a slot method fit a C signature."""
+        for p in list(func.parameters.values())[1:]:
             if not p.is_positional_only():
                 fail(f"The parameters of {func.name!r} must be positional-only")
             if p.default is not unspecified:
                 fail(f"The parameters of {func.name!r} cannot have a default value")
-        converters = [p.converter for p in params]
-        if func.slot.c_slot(converters) is None:
-            got = ', '.join(c.dsl_name() for c in converters)
-            fail(f"{func.name!r} must declare "
-                 f"{func.slot.describe_signatures()}; got ({got})")
 
     def do_post_block_processing_cleanup(self, lineno: int) -> None:
         """