From 544efd062dc67addcd4eddb747f9f02d86abd775 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sun, 4 Jun 2023 20:18:15 -0700 Subject: [PATCH] gh-98963: Restore the ability to have a dict-less property. (GH-105262) Ignore doc string assignment failures in `property` as has been the behavior of all past Python releases. (cherry picked from commit 418befd75d4d0d1cba83d8b81e1a7bcc9a65be8e) Co-authored-by: Gregory P. Smith --- Lib/test/test_property.py | 61 +++++++++++++++++-- ...3-06-02-17-39-19.gh-issue-98963.J4wJgk.rst | 4 ++ Objects/descrobject.c | 45 +++++++++++--- 3 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-06-02-17-39-19.gh-issue-98963.J4wJgk.rst diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index d4bdf50c0192ae..45aa9e51c06de0 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -246,16 +246,67 @@ class PropertySubSlots(property): class PropertySubclassTests(unittest.TestCase): def test_slots_docstring_copy_exception(self): - try: + # A special case error that we preserve despite the GH-98963 behavior + # that would otherwise silently ignore this error. + # This came from commit b18500d39d791c879e9904ebac293402b4a7cd34 + # as part of https://bugs.python.org/issue5890 which allowed docs to + # be set via property subclasses in the first place. + with self.assertRaises(AttributeError): class Foo(object): @PropertySubSlots def spam(self): """Trying to copy this docstring will raise an exception""" return 1 - except AttributeError: - pass - else: - raise Exception("AttributeError not raised") + + def test_property_with_slots_no_docstring(self): + # https://github.com/python/cpython/issues/98963#issuecomment-1574413319 + class slotted_prop(property): + __slots__ = ("foo",) + + p = slotted_prop() # no AttributeError + self.assertIsNone(getattr(p, "__doc__", None)) + + def undocumented_getter(): + return 4 + + p = slotted_prop(undocumented_getter) # New in 3.12: no AttributeError + self.assertIsNone(getattr(p, "__doc__", None)) + + @unittest.skipIf(sys.flags.optimize >= 2, + "Docstrings are omitted with -O2 and above") + def test_property_with_slots_docstring_silently_dropped(self): + # https://github.com/python/cpython/issues/98963#issuecomment-1574413319 + class slotted_prop(property): + __slots__ = ("foo",) + + p = slotted_prop(doc="what's up") # no AttributeError + self.assertIsNone(p.__doc__) + + def documented_getter(): + """getter doc.""" + return 4 + + # Historical behavior: A docstring from a getter always raises. + # (matches test_slots_docstring_copy_exception above). + with self.assertRaises(AttributeError): + p = slotted_prop(documented_getter) + + @unittest.skipIf(sys.flags.optimize >= 2, + "Docstrings are omitted with -O2 and above") + def test_property_with_slots_and_doc_slot_docstring_present(self): + # https://github.com/python/cpython/issues/98963#issuecomment-1574413319 + class slotted_prop(property): + __slots__ = ("foo", "__doc__") + + p = slotted_prop(doc="what's up") + self.assertEqual("what's up", p.__doc__) # new in 3.12: This gets set. + + def documented_getter(): + """what's up getter doc?""" + return 4 + + p = slotted_prop(documented_getter) + self.assertEqual("what's up getter doc?", p.__doc__) @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-06-02-17-39-19.gh-issue-98963.J4wJgk.rst b/Misc/NEWS.d/next/Core and Builtins/2023-06-02-17-39-19.gh-issue-98963.J4wJgk.rst new file mode 100644 index 00000000000000..4caadb0875a188 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-06-02-17-39-19.gh-issue-98963.J4wJgk.rst @@ -0,0 +1,4 @@ +Restore the ability for a subclass of :class:`property` to define ``__slots__`` +or otherwise be dict-less by ignoring failures to set a docstring on such a +class. This behavior had regressed in 3.12beta1. An :exc:`AttributeError` +where there had not previously been one was disruptive to existing code. diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 17c0c85a06c4b8..72ac4703949262 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1485,7 +1485,10 @@ class property(object): self.__get = fget self.__set = fset self.__del = fdel - self.__doc__ = doc + try: + self.__doc__ = doc + except AttributeError: # read-only or dict-less class + pass def __get__(self, inst, type=None): if inst is None: @@ -1791,6 +1794,19 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, if (rc <= 0) { return rc; } + if (!Py_IS_TYPE(self, &PyProperty_Type) && + prop_doc != NULL && prop_doc != Py_None) { + // This oddity preserves the long existing behavior of surfacing + // an AttributeError when using a dict-less (__slots__) property + // subclass as a decorator on a getter method with a docstring. + // See PropertySubclassTest.test_slots_docstring_copy_exception. + int err = PyObject_SetAttr( + (PyObject *)self, &_Py_ID(__doc__), prop_doc); + if (err < 0) { + Py_DECREF(prop_doc); // release our new reference. + return -1; + } + } if (prop_doc == Py_None) { prop_doc = NULL; Py_DECREF(Py_None); @@ -1806,19 +1822,32 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, if (Py_IS_TYPE(self, &PyProperty_Type)) { Py_XSETREF(self->prop_doc, prop_doc); } else { - /* If this is a property subclass, put __doc__ - in dict of the subclass instance instead, - otherwise it gets shadowed by __doc__ in the - class's dict. */ + /* If this is a property subclass, put __doc__ in the dict + or designated slot of the subclass instance instead, otherwise + it gets shadowed by __doc__ in the class's dict. */ if (prop_doc == NULL) { prop_doc = Py_NewRef(Py_None); } int err = PyObject_SetAttr( (PyObject *)self, &_Py_ID(__doc__), prop_doc); - Py_XDECREF(prop_doc); - if (err < 0) - return -1; + Py_DECREF(prop_doc); + if (err < 0) { + assert(PyErr_Occurred()); + if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); + // https://github.com/python/cpython/issues/98963#issuecomment-1574413319 + // Python silently dropped this doc assignment through 3.11. + // We preserve that behavior for backwards compatibility. + // + // If we ever want to deprecate this behavior, only raise a + // warning or error when proc_doc is not None so that + // property without a specific doc= still works. + return 0; + } else { + return -1; + } + } } return 0;