gh-89519 & gh-113157 (Remove classmethod descriptor chaining, depreca…

…ted since 3.11) Summary: backport upstream PRs python/cpython#110163 and python/cpython#113233 upstream issues: python/cpython#89519 (Calling help executes classmethod property decorated methods) and python/cpython#113157 (Changed behavior of <instancemethod>.__get__ in Python 3.11) upstream commits: [`7f9a99e8549b792662f2cd28bf38a4d4625bd402`](python/cpython@7f9a99e) and [`d058eaeed44766a8291013b275ad22f153935d3b`](python/cpython@d058eae) Reviewed By: aleivag Differential Revision: D52014322 fbshipit-source-id: 87de6d9587bd9cc49f053ca340adfc469b041f91
facebookincubator · Dec 22, 2023 · f508768 · f508768
1 parent b1a1db5
commit f508768
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 193 deletions.
diff --git a/Doc/howto/descriptor.rst b/Doc/howto/descriptor.rst
@@ -1182,6 +1182,16 @@ roughly equivalent to:
             obj = self.__self__
             return func(obj, *args, **kwargs)
 
+        def __getattribute__(self, name):
+            "Emulate method_getset() in Objects/classobject.c"
+            if name == '__doc__':
+                return self.__func__.__doc__
+            return object.__getattribute__(self, name)
+
+        def __getattr__(self, name):
+            "Emulate method_getattro() in Objects/classobject.c"
+            return getattr(self.__func__, name)
+
 To support automatic creation of methods, functions include the
 :meth:`__get__` method for binding methods during attribute access.  This
 means that functions are non-data descriptors that return bound methods
@@ -1461,10 +1471,6 @@ Using the non-data descriptor protocol, a pure Python version of
         def __get__(self, obj, cls=None):
             if cls is None:
                 cls = type(obj)
-            if hasattr(type(self.f), '__get__'):
-                # This code path was added in Python 3.9
-                # and was deprecated in Python 3.11.
-                return self.f.__get__(cls, cls)
             return MethodType(self.f, cls)
 
 .. testcode::
@@ -1477,11 +1483,6 @@ Using the non-data descriptor protocol, a pure Python version of
             "Class method that returns a tuple"
             return (cls.__name__, x, y)
 
-        @ClassMethod
-        @property
-        def __doc__(cls):
-            return f'A doc for {cls.__name__!r}'
-
 
 .. doctest::
     :hide:
@@ -1494,10 +1495,6 @@ Using the non-data descriptor protocol, a pure Python version of
     >>> t.cm(11, 22)
     ('T', 11, 22)
 
-    # Check the alternate path for chained descriptors
-    >>> T.__doc__
-    "A doc for 'T'"
-
     # Verify that T uses our emulation
     >>> type(vars(T)['cm']).__name__
     'ClassMethod'
@@ -1522,24 +1519,6 @@ Using the non-data descriptor protocol, a pure Python version of
     ('T', 11, 22)
 
 
-The code path for ``hasattr(type(self.f), '__get__')`` was added in
-Python 3.9 and makes it possible for :func:`classmethod` to support
-chained decorators.  For example, a classmethod and property could be
-chained together.  In Python 3.11, this functionality was deprecated.
-
-.. testcode::
-
-    class G:
-        @classmethod
-        @property
-        def __doc__(cls):
-            return f'A doc for {cls.__name__!r}'
-
-.. doctest::
-
-    >>> G.__doc__
-    "A doc for 'G'"
-
 The :func:`functools.update_wrapper` call in ``ClassMethod`` adds a
 ``__wrapped__`` attribute that refers to the underlying function.  Also
 it carries forward the attributes necessary to make the wrapper look

diff --git a/Doc/library/functions.rst b/Doc/library/functions.rst
@@ -286,7 +286,7 @@ are always available.  They are listed here in alphabetical order.
       ``__name__``, ``__qualname__``, ``__doc__`` and ``__annotations__``) and
       have a new ``__wrapped__`` attribute.
 
-   .. versionchanged:: 3.11
+   .. deprecated-removed:: 3.11 3.13
       Class methods can no longer wrap other :term:`descriptors <descriptor>` such as
       :func:`property`.
 

diff --git a/Lib/test/test_decorators.py b/Lib/test/test_decorators.py
@@ -291,44 +291,6 @@ def bar(): return 42
         self.assertEqual(bar(), 42)
         self.assertEqual(actions, expected_actions)
 
-    def test_wrapped_descriptor_inside_classmethod(self):
-        class BoundWrapper:
-            def __init__(self, wrapped):
-                self.__wrapped__ = wrapped
-
-            def __call__(self, *args, **kwargs):
-                return self.__wrapped__(*args, **kwargs)
-
-        class Wrapper:
-            def __init__(self, wrapped):
-                self.__wrapped__ = wrapped
-
-            def __get__(self, instance, owner):
-                bound_function = self.__wrapped__.__get__(instance, owner)
-                return BoundWrapper(bound_function)
-
-        def decorator(wrapped):
-            return Wrapper(wrapped)
-
-        class Class:
-            @decorator
-            @classmethod
-            def inner(cls):
-                # This should already work.
-                return 'spam'
-
-            @classmethod
-            @decorator
-            def outer(cls):
-                # Raised TypeError with a message saying that the 'Wrapper'
-                # object is not callable.
-                return 'eggs'
-
-        self.assertEqual(Class.inner(), 'spam')
-        self.assertEqual(Class.outer(), 'eggs')
-        self.assertEqual(Class().inner(), 'spam')
-        self.assertEqual(Class().outer(), 'eggs')
-
     def test_bound_function_inside_classmethod(self):
         class A:
             def foo(self, cls):
@@ -339,91 +301,6 @@ class B:
 
         self.assertEqual(B.bar(), 'spam')
 
-    def test_wrapped_classmethod_inside_classmethod(self):
-        class MyClassMethod1:
-            def __init__(self, func):
-                self.func = func
-
-            def __call__(self, cls):
-                if hasattr(self.func, '__get__'):
-                    return self.func.__get__(cls, cls)()
-                return self.func(cls)
-
-            def __get__(self, instance, owner=None):
-                if owner is None:
-                    owner = type(instance)
-                return MethodType(self, owner)
-
-        class MyClassMethod2:
-            def __init__(self, func):
-                if isinstance(func, classmethod):
-                    func = func.__func__
-                self.func = func
-
-            def __call__(self, cls):
-                return self.func(cls)
-
-            def __get__(self, instance, owner=None):
-                if owner is None:
-                    owner = type(instance)
-                return MethodType(self, owner)
-
-        for myclassmethod in [MyClassMethod1, MyClassMethod2]:
-            class A:
-                @myclassmethod
-                def f1(cls):
-                    return cls
-
-                @classmethod
-                @myclassmethod
-                def f2(cls):
-                    return cls
-
-                @myclassmethod
-                @classmethod
-                def f3(cls):
-                    return cls
-
-                @classmethod
-                @classmethod
-                def f4(cls):
-                    return cls
-
-                @myclassmethod
-                @MyClassMethod1
-                def f5(cls):
-                    return cls
-
-                @myclassmethod
-                @MyClassMethod2
-                def f6(cls):
-                    return cls
-
-            self.assertIs(A.f1(), A)
-            self.assertIs(A.f2(), A)
-            self.assertIs(A.f3(), A)
-            self.assertIs(A.f4(), A)
-            self.assertIs(A.f5(), A)
-            self.assertIs(A.f6(), A)
-            a = A()
-            self.assertIs(a.f1(), A)
-            self.assertIs(a.f2(), A)
-            self.assertIs(a.f3(), A)
-            self.assertIs(a.f4(), A)
-            self.assertIs(a.f5(), A)
-            self.assertIs(a.f6(), A)
-
-            def f(cls):
-                return cls
-
-            self.assertIs(myclassmethod(f).__get__(a)(), A)
-            self.assertIs(myclassmethod(f).__get__(a, A)(), A)
-            self.assertIs(myclassmethod(f).__get__(A, A)(), A)
-            self.assertIs(myclassmethod(f).__get__(A)(), type(A))
-            self.assertIs(classmethod(f).__get__(a)(), A)
-            self.assertIs(classmethod(f).__get__(a, A)(), A)
-            self.assertIs(classmethod(f).__get__(A, A)(), A)
-            self.assertIs(classmethod(f).__get__(A)(), type(A))
 
 class TestClassDecorators(unittest.TestCase):
 

diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
@@ -5004,6 +5004,21 @@ class Child(Parent):
         gc.collect()
         self.assertEqual(Parent.__subclasses__(), [])
 
+    def test_instance_method_get_behavior(self):
+        # test case for gh-113157
+
+        class A:
+            def meth(self):
+                return self
+
+        class B:
+            pass
+
+        a = A()
+        b = B()
+        b.meth = a.meth.__get__(b, B)
+        self.assertEqual(b.meth(), a)
+
     def test_attr_raise_through_property(self):
         # test case for gh-103272
         class A:

diff --git a/Lib/test/test_doctest.py b/Lib/test/test_doctest.py
@@ -102,15 +102,6 @@ def a_classmethod(cls, v):
 
     a_class_attribute = 42
 
-    @classmethod
-    @property
-    def a_classmethod_property(cls):
-        """
-        >>> print(SampleClass.a_classmethod_property)
-        42
-        """
-        return cls.a_class_attribute
-
     class NestedClass:
         """
         >>> x = SampleClass.NestedClass(5)
@@ -533,7 +524,6 @@ def basics(): r"""
      1  SampleClass.NestedClass.__init__
      1  SampleClass.__init__
      2  SampleClass.a_classmethod
-     1  SampleClass.a_classmethod_property
      1  SampleClass.a_property
      1  SampleClass.a_staticmethod
      1  SampleClass.double
@@ -589,7 +579,6 @@ def basics(): r"""
      1  some_module.SampleClass.NestedClass.__init__
      1  some_module.SampleClass.__init__
      2  some_module.SampleClass.a_classmethod
-     1  some_module.SampleClass.a_classmethod_property
      1  some_module.SampleClass.a_property
      1  some_module.SampleClass.a_staticmethod
      1  some_module.SampleClass.double
@@ -631,7 +620,6 @@ def basics(): r"""
      1  SampleClass.NestedClass.__init__
      1  SampleClass.__init__
      2  SampleClass.a_classmethod
-     1  SampleClass.a_classmethod_property
      1  SampleClass.a_property
      1  SampleClass.a_staticmethod
      1  SampleClass.double
@@ -652,7 +640,6 @@ def basics(): r"""
      0  SampleClass.NestedClass.square
      1  SampleClass.__init__
      2  SampleClass.a_classmethod
-     1  SampleClass.a_classmethod_property
      1  SampleClass.a_property
      1  SampleClass.a_staticmethod
      1  SampleClass.double

diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py
@@ -183,27 +183,6 @@ def test_refleaks_in___init__(self):
             fake_prop.__init__('fget', 'fset', 'fdel', 'doc')
         self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10)
 
-    @unittest.skipIf(sys.flags.optimize >= 2,
-                     "Docstrings are omitted with -O2 and above")
-    def test_class_property(self):
-        class A:
-            @classmethod
-            @property
-            def __doc__(cls):
-                return 'A doc for %r' % cls.__name__
-        self.assertEqual(A.__doc__, "A doc for 'A'")
-
-    @unittest.skipIf(sys.flags.optimize >= 2,
-                     "Docstrings are omitted with -O2 and above")
-    def test_class_property_override(self):
-        class A:
-            """First"""
-            @classmethod
-            @property
-            def __doc__(cls):
-                return 'Second'
-        self.assertEqual(A.__doc__, 'Second')
-
     def test_property_set_name_incorrect_args(self):
         p = property()
 

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-30-17-30-11.gh-issue-89519.hz2pZf.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-30-17-30-11.gh-issue-89519.hz2pZf.rst
@@ -0,0 +1,6 @@
+Removed chained :class:`classmethod` descriptors (introduced in
+:issue:`19072`).  This can no longer be used to wrap other descriptors such
+as :class:`property`.  The core design of this feature was flawed and caused
+a number of downstream problems.  To "pass-through" a :class:`classmethod`,
+consider using the :attr:`!__wrapped__` attribute that was added in Python
+3.10.
diff --git a/Objects/classobject.c b/Objects/classobject.c
@@ -318,6 +318,13 @@ method_traverse(PyMethodObject *im, visitproc visit, void *arg)
     return 0;
 }
 
+static PyObject *
+method_descr_get(PyObject *meth, PyObject *obj, PyObject *cls)
+{
+    Py_INCREF(meth);
+    return meth;
+}
+
 PyTypeObject PyMethod_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
     .tp_name = "method",
@@ -338,6 +345,7 @@ PyTypeObject PyMethod_Type = {
     .tp_methods = method_methods,
     .tp_members = method_memberlist,
     .tp_getset = method_getset,
+    .tp_descr_get = method_descr_get,
     .tp_new = method_new,
 };
 

diff --git a/Objects/funcobject.c b/Objects/funcobject.c
@@ -1042,10 +1042,6 @@ cm_descr_get(PyObject *self, PyObject *obj, PyObject *type)
     }
     if (type == NULL)
         type = (PyObject *)(Py_TYPE(obj));
-    if (Py_TYPE(cm->cm_callable)->tp_descr_get != NULL) {
-        return Py_TYPE(cm->cm_callable)->tp_descr_get(cm->cm_callable, type,
-                                                      type);
-    }
     return PyMethod_New(cm->cm_callable, type);
 }