Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-98963: Restore the ability to have a dict-less property. #105262

Merged
merged 7 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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