Issue python#23722: improve __classcell__ compatibility

ncoghlan · ncoghlan · commit 19d246745d9d · 2016-12-05T16:47:55.000+10:00
Handling zero-argument super() in __init_subclass__ and
__set_name__ involved moving __class__ initialisation to
type.__new__. This requires cooperation from custom
metaclasses to ensure that the new __classcell__ entry
is passed along appropriately.

The initial implementation of that change resulted in abruptly
broken zero-argument super() support in metaclasses that didn't
adhere to the new requirements (such as Django's metaclass for
Model definitions).

The updated approach adopted here instead emits a deprecation
warning for those cases, and makes them work the same way they
did in Python 3.5.

This patch also improves the related class machinery documentation
to cover these details and to include more reader-friendly
cross-references and index entries.
diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst
@@ -1700,6 +1700,10 @@ class defining the method.
 Metaclasses
 ^^^^^^^^^^^
 
+.. index::
+    single: metaclass
+    builtin: type
+
 By default, classes are constructed using :func:`type`. The class body is
 executed in a new namespace and the class name is bound locally to the
 result of ``type(name, bases, namespace)``.
@@ -1730,6 +1734,8 @@ When a class definition is executed, the following steps occur:
 
 Determining the appropriate metaclass
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+.. index::
+    single: metaclass hint
 
 The appropriate metaclass for a class definition is determined as follows:
 
@@ -1751,6 +1757,9 @@ that criterion, then the class definition will fail with ``TypeError``.
 Preparing the class namespace
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+.. index::
+    single: __prepare__ (metaclass method)
+
 Once the appropriate metaclass has been identified, then the class namespace
 is prepared. If the metaclass has a ``__prepare__`` attribute, it is called
 as ``namespace = metaclass.__prepare__(name, bases, **kwds)`` (where the
@@ -1768,6 +1777,9 @@ is initialised as an empty ordered mapping.
 Executing the class body
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
+.. index::
+    single: class; body
+
 The class body is executed (approximately) as
 ``exec(body, globals(), namespace)``. The key difference from a normal
 call to :func:`exec` is that lexical scoping allows the class body (including
@@ -1777,12 +1789,19 @@ class definition occurs inside a function.
 However, even when the class definition occurs inside the function, methods
 defined inside the class still cannot see names defined at the class scope.
 Class variables must be accessed through the first parameter of instance or
-class methods, and cannot be accessed at all from static methods.
+class methods, or through the implicit lexically scoped ``__class__`` reference
+described in the next section.
 
+.. _class-object-creation:
 
 Creating the class object
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 
+.. index::
+    single: __class__ (method cell)
+    single: __classcell__ (class namespace entry)
+
+
 Once the class namespace has been populated by executing the class body,
 the class object is created by calling
 ``metaclass(name, bases, namespace, **kwds)`` (the additional keywords
@@ -1796,6 +1815,26 @@ created by the compiler if any methods in a class body refer to either
 lexical scoping, while the class or instance that was used to make the
 current call is identified based on the first argument passed to the method.
 
+.. impl-detail::
+
+   In CPython 3.6 and later, the ``__class__`` cell is passed to the metaclass
+   as a ``__classcell__`` entry in the class namespace. If present, this must
+   be propagated up to the ``type.__new__`` call in order for the class to be
+   initialised correctly.
+   Failing to do so will result in a :exc:`DeprecationWarning` in Python 3.6,
+   and a :exc:`RuntimeWarning` in the future.
+
+When using the default metaclass :class:`type`, or any metaclass that ultimately
+calls ``type.__new__``, the following additional customisation steps are
+invoked after creating the class object:
+
+* first, ``type.__new__`` collects all of the descriptors in the class
+  namespace that define a :meth:`~object.__set_name__` method;
+* second, all of these ``__set_name__`` methods are called with the class
+  being defined and the assigned name of that particular descriptor; and
+* finally, the :meth:`~object.__init_subclass__` hook is called on the
+  immediate parent of the new class in its method resolution order.
+
 After the class object is created, it is passed to the class decorators
 included in the class definition (if any) and the resulting object is bound
 in the local namespace as the defined class.
diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst
@@ -351,6 +351,11 @@ whenever a new subclass is created::
     class Plugin2(PluginBase):
         pass
 
+In order to allow zero-argument :func:`super` calls to work correctly from
+:meth:`~object.__init_subclass__` implementations, custom metaclasses must
+ensure that the new ``__classcell__`` namespace entry is propagated to
+``type.__new__`` (as described in :ref:`class-object-creation`).
+
 .. seealso::
 
  :pep:`487` -- Simpler customization of class creation
@@ -2235,6 +2240,11 @@ Changes in the Python API
   on a ZipFile created with mode ``'r'`` will raise a :exc:`ValueError`.
   Previously, a :exc:`RuntimeError` was raised in those scenarios.
 
+* when custom metaclasses are combined with zero-argument :func:`super` or
+  direct references from methods to the implicit ``__class__`` closure
+  variable, the implicit ``__classcell__`` namespace entry must now be passed
+  up to ``type.__new__`` for initialisation. Failing to do so will result in
+  a :exc:`DeprecationWarning` in 3.6 and a :exc:`RuntimeWarning` in the future.
 
 Changes in the C API
 --------------------
diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
@@ -239,6 +239,7 @@ def _write_atomic(path, data, mode=0o666):
 #     Python 3.6b1  3376 (simplify CALL_FUNCTIONs & BUILD_MAP_UNPACK_WITH_CALL)
 #     Python 3.6b1  3377 (set __class__ cell from type.__new__ #23722)
 #     Python 3.6b2  3378 (add BUILD_TUPLE_UNPACK_WITH_CALL #28257)
+#     Python 3.6rc1 3379 (more thorough __class__ validation #23722)
 #
 # MAGIC must change whenever the bytecode emitted by the compiler may no
 # longer be understood by older implementations of the eval loop (usually
@@ -247,7 +248,7 @@ def _write_atomic(path, data, mode=0o666):
 # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
 # in PC/launcher.c must also be updated.
 
-MAGIC_NUMBER = (3378).to_bytes(2, 'little') + b'\r\n'
+MAGIC_NUMBER = (3379).to_bytes(2, 'little') + b'\r\n'
 _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little')  # For import.c
 
 _PYCACHE = '__pycache__'
diff --git a/Lib/test/test_super.py b/Lib/test/test_super.py
@@ -1,7 +1,9 @@
-"""Unit tests for new super() implementation."""
+"""Unit tests for zero-argument super() & related machinery."""
 
 import sys
 import unittest
+import warnings
+from test.support import check_warnings
 
 
 class A:
@@ -144,6 +146,8 @@ def f():
         self.assertIs(X.f(), X)
 
     def test___class___new(self):
+        # See issue #23722
+        # Ensure zero-arg super() works as soon as type.__new__() is completed
         test_class = None
 
         class Meta(type):
@@ -161,6 +165,7 @@ def f():
         self.assertIs(test_class, A)
 
     def test___class___delayed(self):
+        # See issue #23722
         test_namespace = None
 
         class Meta(type):
@@ -169,17 +174,22 @@ def __new__(cls, name, bases, namespace):
                 test_namespace = namespace
                 return None
 
-        class A(metaclass=Meta):
-            @staticmethod
-            def f():
-                return __class__
+        # This case shouldn't trigger the __classcell__ deprecation warning
+        with check_warnings() as w:
+            warnings.simplefilter("always", DeprecationWarning)
+            class A(metaclass=Meta):
+                @staticmethod
+                def f():
+                    return __class__
+        self.assertEqual(w.warnings, [])
 
         self.assertIs(A, None)
 
         B = type("B", (), test_namespace)
         self.assertIs(B.f(), B)
 
     def test___class___mro(self):
+        # See issue #23722
         test_class = None
 
         class Meta(type):
@@ -195,34 +205,105 @@ def f():
 
         self.assertIs(test_class, A)
 
-    def test___classcell___deleted(self):
+    def test___classcell___expected_behaviour(self):
+        # See issue #23722
         class Meta(type):
             def __new__(cls, name, bases, namespace):
-                del namespace['__classcell__']
+                nonlocal namespace_snapshot
+                namespace_snapshot = namespace.copy()
                 return super().__new__(cls, name, bases, namespace)
 
-        class A(metaclass=Meta):
-            @staticmethod
-            def f():
-                __class__
-
-        with self.assertRaises(NameError):
-            A.f()
+        # __classcell__ is injected into the class namespace by the compiler
+        # when at least one method needs it, and should be omitted otherwise
+        namespace_snapshot = None
+        class WithoutClassRef(metaclass=Meta):
+            pass
+        self.assertNotIn("__classcell__", namespace_snapshot)
+
+        # With zero-arg super() or an explicit __class__ reference,
+        # __classcell__ is the exact cell reference to be populated by
+        # type.__new__
+        namespace_snapshot = None
+        class WithClassRef(metaclass=Meta):
+            def f(self):
+                return __class__
 
-    def test___classcell___reset(self):
+        class_cell = namespace_snapshot["__classcell__"]
+        method_closure = WithClassRef.f.__closure__
+        self.assertEqual(len(method_closure), 1)
+        self.assertIs(class_cell, method_closure[0])
+        # Ensure the cell reference *doesn't* get turned into an attribute
+        with self.assertRaises(AttributeError):
+            WithClassRef.__classcell__
+
+    def test___classcell___missing(self):
+        # See issue #23722
+        # Some metaclasses may not pass the original namespace to type.__new__
+        # We test that case here by forcibly deleting __classcell__
         class Meta(type):
             def __new__(cls, name, bases, namespace):
-                namespace['__classcell__'] = 0
+                namespace.pop('__classcell__', None)
                 return super().__new__(cls, name, bases, namespace)
 
-        class A(metaclass=Meta):
-            @staticmethod
-            def f():
-                __class__
+        # The default case should continue to work without any warnings
+        with check_warnings() as w:
+            warnings.simplefilter("always", DeprecationWarning)
+            class WithoutClassRef(metaclass=Meta):
+                pass
+        self.assertEqual(w.warnings, [])
+
+        # With zero-arg super() or an explicit __class__ reference, we expect
+        # __build_class__ to emit a DeprecationWarning complaining that
+        # __class__ was not set, and asking if __classcell__ was propagated
+        # to type.__new__.
+        # In Python 3.7, that warning will become a RuntimeError.
+        expected_warning = (
+            '__class__ not set.*__classcell__ propagated',
+            DeprecationWarning
+        )
+        with check_warnings(expected_warning):
+            warnings.simplefilter("always", DeprecationWarning)
+            class WithClassRef(metaclass=Meta):
+                def f(self):
+                    return __class__
+        # Check __class__ still gets set despite the warning
+        self.assertIs(WithClassRef().f(), WithClassRef)
+
+        # Check the warning is turned into an error as expected
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", DeprecationWarning)
+            with self.assertRaises(DeprecationWarning):
+                class WithClassRef(metaclass=Meta):
+                    def f(self):
+                        return __class__
+
+    def test___classcell___overwrite(self):
+        # See issue #23722
+        # Overwriting __classcell__ with nonsense is explicitly prohibited
+        class Meta(type):
+            def __new__(cls, name, bases, namespace, cell):
+                namespace['__classcell__'] = cell
+                return super().__new__(cls, name, bases, namespace)
+
+        for bad_cell in (None, 0, "", object()):
+            with self.subTest(bad_cell=bad_cell):
+                with self.assertRaises(TypeError):
+                    class A(metaclass=Meta, cell=bad_cell):
+                        pass
 
-        with self.assertRaises(NameError):
-            A.f()
-        self.assertEqual(A.__classcell__, 0)
+    def test___classcell___wrong_cell(self):
+        # See issue #23722
+        # Pointing the cell reference at the wrong class is also prohibited
+        class Meta(type):
+            def __new__(cls, name, bases, namespace):
+                cls = super().__new__(cls, name, bases, namespace)
+                B = type("B", (), namespace)
+                return cls
+
+        with self.assertRaises(TypeError):
+            class A(metaclass=Meta):
+                def f(self):
+                    return __class__
 
     def test_obscure_super_errors(self):
         def f():
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -10,6 +10,11 @@ What's New in Python 3.6.0 release candidate 1
 Core and Builtins
 -----------------
 
+- Issue #23722: Rather than silently producing a class that doesn't support
+  zero-argument ``super()`` in methods, failing to pass the new
+  ``__classcell__`` namespace entry up to ``type.__new__`` now results in a
+  ``DeprecationWarning`` and a class that supports zero-argument ``super()``.
+
 - Issue #28797: Modifying the class __dict__ inside the __set_name__ method of
   a descriptor that is used inside that class no longer prevents calling the
   __set_name__ method of other descriptors.
@@ -31,6 +36,13 @@ Library
 
 - Issue #28843: Fix asyncio C Task to handle exceptions __traceback__.
 
+Documentation
+-------------
+
+- Issue #23722: The data model reference and the porting section in the What's
+  New guide now cover the additional ``__classcell__`` handling needed for
+  custom metaclasses to fully support PEP 487 and zero-argument ``super()``.
+
 Tools/Demos
 -----------
 
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -2687,9 +2687,16 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds)
     else
         type->tp_free = PyObject_Del;
 
-    /* store type in class' cell */
+    /* store type in class' cell if one is supplied */
     cell = _PyDict_GetItemId(dict, &PyId___classcell__);
-    if (cell != NULL && PyCell_Check(cell)) {
+    if (cell != NULL) {
+        /* At least one method requires a reference to its defining class */
+        if (!PyCell_Check(cell)) {
+            PyErr_Format(PyExc_TypeError,
+                         "__classcell__ must be a nonlocal cell, not %.200R",
+                         Py_TYPE(cell));
+            goto error;
+        }
         PyCell_Set(cell, (PyObject *) type);
         _PyDict_DelItemId(dict, &PyId___classcell__);
         PyErr_Clear();
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
diff --git a/Python/compile.c b/Python/compile.c
diff --git a/Python/importlib_external.h b/Python/importlib_external.h
