Skip to content

Commit

Permalink
[3.12] gh-98963: Restore the ability to have a dict-less property. (G…
Browse files Browse the repository at this point in the history
…H-105262) (#105297)

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.  (the docstring is discarded)
(cherry picked from commit 418befd)

This fixes a behavior regression in 3.12beta1 where an AttributeError was being raised in a situation it has never been in the past. It keeps the existing unusual single situation where AttributeError does get raised.

Existing widely deployed projects depend on this not raising an exception.

Co-authored-by: Gregory P. Smith <greg@krypto.org>
  • Loading branch information
miss-islington and gpshead authored Jun 5, 2023
1 parent 9ce3312 commit 3e7ddc2
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 13 deletions.
61 changes: 56 additions & 5 deletions Lib/test/test_property.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
45 changes: 37 additions & 8 deletions Objects/descrobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down

0 comments on commit 3e7ddc2

Please sign in to comment.