Improving property to accept abstract methods

Objects/descrobject.c

 /* Descriptors -- a new, flexible way to describe attributes */
 
 #include "Python.h"
 #include "structmember.h" /* Why is this not included in Python.h? */
 
 static void
 descr_dealloc(PyDescrObject *descr)
 {
     _PyObject_GC_UNTRACK(descr);
     Py_XDECREF(descr->d_type);
@@ skipping 208 matching lines @@
     argc = PyTuple_GET_SIZE(args);
     if (argc < 1) {
         PyErr_Format(PyExc_TypeError,
                      "descriptor '%V' of '%.100s' "
                      "object needs an argument",
                      descr_name((PyDescrObject *)descr), "?",
                      PyDescr_TYPE(descr)->tp_name);
         return NULL;
     }
     self = PyTuple_GET_ITEM(args, 0);
-    if (!PyObject_IsInstance(self, (PyObject *)PyDescr_TYPE(descr))) {
+    if (!_PyObject_RealIsSubclass((PyObject *)Py_TYPE(self),
+                                  (PyObject *)PyDescr_TYPE(descr))) {
         PyErr_Format(PyExc_TypeError,
                      "descriptor '%V' "
                      "requires a '%.100s' object "
                      "but received a '%.100s'",
                      descr_name((PyDescrObject *)descr), "?",
                      PyDescr_TYPE(descr)->tp_name,
                      self->ob_type->tp_name);
         return NULL;
     }
 
@@ skipping 37 matching lines @@
     argc = PyTuple_GET_SIZE(args);
     if (argc < 1) {
         PyErr_Format(PyExc_TypeError,
                      "descriptor '%V' of '%.100s' "
                      "object needs an argument",
                      descr_name((PyDescrObject *)descr), "?",
                      PyDescr_TYPE(descr)->tp_name);
         return NULL;
     }
     self = PyTuple_GET_ITEM(args, 0);
-    if (!PyObject_IsInstance(self, (PyObject *)PyDescr_TYPE(descr))) {
+    if (!_PyObject_RealIsSubclass((PyObject *)Py_TYPE(self),
+                                  (PyObject *)PyDescr_TYPE(descr))) {
         PyErr_Format(PyExc_TypeError,
                      "descriptor '%V' "
                      "requires a '%.100s' object "
                      "but received a '%.100s'",
                      descr_name((PyDescrObject *)descr), "?",
                      PyDescr_TYPE(descr)->tp_name,
                      self->ob_type->tp_name);
         return NULL;
     }
 
@@ skipping 396 matching lines @@
     0,                                          /* sq_ass_slice */
     (objobjproc)proxy_contains,                 /* sq_contains */
     0,                                          /* sq_inplace_concat */
     0,                                          /* sq_inplace_repeat */
 };
 
 static PyObject *
 proxy_get(proxyobject *pp, PyObject *args)
 {
     PyObject *key, *def = Py_None;
+    _Py_IDENTIFIER(get);
 
     if (!PyArg_UnpackTuple(args, "get", 1, 2, &key, &def))
         return NULL;
-    return PyObject_CallMethod(pp->dict, "get", "(OO)", key, def);
+    return _PyObject_CallMethodId(pp->dict, &PyId_get, "(OO)", key, def);
 }
 
 static PyObject *
 proxy_keys(proxyobject *pp)
 {
-    return PyObject_CallMethod(pp->dict, "keys", NULL);
+    _Py_IDENTIFIER(keys);
+    return _PyObject_CallMethodId(pp->dict, &PyId_keys, NULL);
 }
 
 static PyObject *
 proxy_values(proxyobject *pp)
 {
-    return PyObject_CallMethod(pp->dict, "values", NULL);
+    _Py_IDENTIFIER(values);
+    return _PyObject_CallMethodId(pp->dict, &PyId_values, NULL);
 }
 
 static PyObject *
 proxy_items(proxyobject *pp)
 {
-    return PyObject_CallMethod(pp->dict, "items", NULL);
+    _Py_IDENTIFIER(items);
+    return _PyObject_CallMethodId(pp->dict, &PyId_items, NULL);
 }
 
 static PyObject *
 proxy_copy(proxyobject *pp)
 {
-    return PyObject_CallMethod(pp->dict, "copy", NULL);
+    _Py_IDENTIFIER(copy);
+    return _PyObject_CallMethodId(pp->dict, &PyId_copy, NULL);
 }
 
 static PyMethodDef proxy_methods[] = {
     {"get",       (PyCFunction)proxy_get,        METH_VARARGS,
      PyDoc_STR("D.get(k[,d]) -> D[k] if k in D, else d."
                                     "  d defaults to None.")},
     {"keys",      (PyCFunction)proxy_keys,       METH_NOARGS,
      PyDoc_STR("D.keys() -> list of D's keys")},
     {"values",    (PyCFunction)proxy_values,     METH_NOARGS,
      PyDoc_STR("D.values() -> list of D's values")},
@@ skipping 95 matching lines @@
     }
     return (PyObject *)pp;
 }
 
 
 /* --- Wrapper object for "slot" methods --- */
 
 /* This has no reason to be in this file except that adding new files is a
    bit of a pain */
 
-/* forward */
-static PyTypeObject wrappertype;
-
 typedef struct {
     PyObject_HEAD
     PyWrapperDescrObject *descr;
     PyObject *self;
 } wrapperobject;
 
-#define Wrapper_Check(v) (Py_TYPE(v) == &wrappertype)
+#define Wrapper_Check(v) (Py_TYPE(v) == &_PyMethodWrapper_Type)
 
 static void
 wrapper_dealloc(wrapperobject *wp)
 {
     PyObject_GC_UnTrack(wp);
     Py_TRASHCAN_SAFE_BEGIN(wp)
     Py_XDECREF(wp->descr);
     Py_XDECREF(wp->self);
     PyObject_GC_Del(wp);
     Py_TRASHCAN_SAFE_END(wp)
@@ skipping 145 matching lines @@
 
 static int
 wrapper_traverse(PyObject *self, visitproc visit, void *arg)
 {
     wrapperobject *wp = (wrapperobject *)self;
     Py_VISIT(wp->descr);
     Py_VISIT(wp->self);
     return 0;
 }
 
-static PyTypeObject wrappertype = {
+PyTypeObject _PyMethodWrapper_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
     "method-wrapper",                           /* tp_name */
     sizeof(wrapperobject),                      /* tp_basicsize */
     0,                                          /* tp_itemsize */
     /* methods */
     (destructor)wrapper_dealloc,                /* tp_dealloc */
     0,                                          /* tp_print */
     0,                                          /* tp_getattr */
     0,                                          /* tp_setattr */
     0,                                          /* tp_reserved */
@@ skipping 25 matching lines @@
 };
 
 PyObject *
 PyWrapper_New(PyObject *d, PyObject *self)
 {
     wrapperobject *wp;
     PyWrapperDescrObject *descr;
 
     assert(PyObject_TypeCheck(d, &PyWrapperDescr_Type));
     descr = (PyWrapperDescrObject *)d;
-    assert(PyObject_IsInstance(self, (PyObject *)PyDescr_TYPE(descr)));
-
-    wp = PyObject_GC_New(wrapperobject, &wrappertype);
+    assert(_PyObject_RealIsSubclass((PyObject *)Py_TYPE(self),
+                                    (PyObject *)PyDescr_TYPE(descr)));
+
+    wp = PyObject_GC_New(wrapperobject, &_PyMethodWrapper_Type);
     if (wp != NULL) {
         Py_INCREF(descr);
         wp->descr = descr;
         Py_INCREF(self);
         wp->self = self;
         _PyObject_GC_TRACK(wp);
     }
     return (PyObject *)wp;
 }
 
 
 /* A built-in 'property' type */
 
 /*
     class property(object):
 
     def __init__(self, fget=None, fset=None, fdel=None, doc=None):
         if doc is None and fget is not None and hasattr(fget, "__doc__"):
         doc = fget.__doc__
         self.__get = fget
         self.__set = fset
         self.__del = fdel
         self.__doc__ = doc
-        for f in (fget, fset, fdel):
-            if getattr(f, '__isabstractmethod__', False):
-                self.__isabstractmethod__ = True
-                break
 
     def __get__(self, inst, type=None):
         if inst is None:
         return self
         if self.__get is None:
         raise AttributeError, "unreadable attribute"
         return self.__get(inst)
 
     def __set__(self, inst, value):
         if self.__set is None:
         raise AttributeError, "can't set attribute"
         return self.__set(inst, value)
 
     def __delete__(self, inst):
         if self.__del is None:
         raise AttributeError, "can't delete attribute"
         return self.__del(inst)
 
 */
 
 typedef struct {
     PyObject_HEAD
     PyObject *prop_get;
     PyObject *prop_set;
     PyObject *prop_del;
     PyObject *prop_doc;
-    PyObject *prop_isabstract;

[durban, 2011/03/19 21:36:09]

An int flag probably would be enough (like getter_doc).

     int getter_doc;
 } propertyobject;
 
 static PyObject * property_copy(PyObject *, PyObject *, PyObject *,
                                   PyObject *);
 
 static PyMemberDef property_members[] = {
     {"fget", T_OBJECT, offsetof(propertyobject, prop_get), READONLY},
     {"fset", T_OBJECT, offsetof(propertyobject, prop_set), READONLY},
     {"fdel", T_OBJECT, offsetof(propertyobject, prop_del), READONLY},
     {"__doc__",  T_OBJECT, offsetof(propertyobject, prop_doc), READONLY},
-    {"__isabstractmethod__",  T_OBJECT,
-     offsetof(propertyobject, prop_isabstract), READONLY},
     {0}
 };
 
 
 PyDoc_STRVAR(getter_doc,
              "Descriptor to change the getter on a property.");
 
 static PyObject *
 property_getter(PyObject *self, PyObject *getter)
 {
@@ skipping 32 matching lines @@
 static void
 property_dealloc(PyObject *self)
 {
     propertyobject *gs = (propertyobject *)self;
 
     _PyObject_GC_UNTRACK(self);
     Py_XDECREF(gs->prop_get);
     Py_XDECREF(gs->prop_set);
     Py_XDECREF(gs->prop_del);
     Py_XDECREF(gs->prop_doc);
-    Py_XDECREF(gs->prop_isabstract);
     self->ob_type->tp_free(self);
 }
 
 static PyObject *
 property_descr_get(PyObject *self, PyObject *obj, PyObject *type)
 {
     propertyobject *gs = (propertyobject *)self;
 
     if (obj == NULL || obj == Py_None) {
         Py_INCREF(self);
@@ skipping 13 matching lines @@
     PyObject *func, *res;
 
     if (value == NULL)
         func = gs->prop_del;
     else
         func = gs->prop_set;
     if (func == NULL) {
         PyErr_SetString(PyExc_AttributeError,
                         value == NULL ?
                         "can't delete attribute" :
-                        "can't set attribute");
+                "can't set attribute");
         return -1;
     }
     if (value == NULL)
         res = PyObject_CallFunctionObjArgs(func, obj, NULL);
     else
         res = PyObject_CallFunctionObjArgs(func, obj, value, NULL);
     if (res == NULL)
         return -1;
     Py_DECREF(res);
     return 0;
@@ skipping 29 matching lines @@
         doc = pold->prop_doc ? pold->prop_doc : Py_None;
     }
 
     new =  PyObject_CallFunction(type, "OOOO", get, set, del, doc);
     Py_DECREF(type);
     if (new == NULL)
         return NULL;
     return new;
 }
 
-static void
-property_identify_abstract_method(PyObject *self, PyObject *method)
-{
-    /* Set self.__isabstractmethod__ if method is abstract */
-    if (method != NULL){
-        PyObject *is_abstract = PyObject_GetAttrString(method,

[durban, 2011/03/19 21:36:09]

NULL/exception check?

[dsdale24, 2011/03/19 22:13:42]

This is the part where I am weak. Can you point me to documentation? Why is an
exception check necessary? Do PyObject_IsTrue and Py_DECREF not know what to do
when passed NULL?

-                                                       "__isabstractmethod__");
-        if (PyObject_IsTrue(is_abstract) > 0){
-            Py_INCREF(Py_True);
-            PyObject_SetAttrString(self, "__isabstractmethod__", Py_True);

[durban, 2011/03/19 21:36:10]

(You could directly assign to the prop_isabstract field. But then the old value
should be DECREF'd.)

[dsdale24, 2011/03/19 22:13:42]

I got compilation errors when I tried.

-        }
-        Py_DECREF(is_abstract);

[durban, 2011/03/19 21:36:10]

Can be NULL.

-    }
-}
-
 static int
 property_init(PyObject *self, PyObject *args, PyObject *kwds)
 {
     PyObject *get = NULL, *set = NULL, *del = NULL, *doc = NULL;
     static char *kwlist[] = {"fget", "fset", "fdel", "doc", 0};
     propertyobject *prop = (propertyobject *)self;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOOO:property",
                                      kwlist, &get, &set, &del, &doc))
         return -1;
 
     if (get == Py_None)
         get = NULL;
     if (set == Py_None)
         set = NULL;
     if (del == Py_None)
         del = NULL;
 
     Py_XINCREF(get);
     Py_XINCREF(set);
     Py_XINCREF(del);
     Py_XINCREF(doc);
-    Py_INCREF(Py_False);
 
     prop->prop_get = get;
     prop->prop_set = set;
     prop->prop_del = del;
     prop->prop_doc = doc;
-    prop->prop_isabstract = Py_False;
     prop->getter_doc = 0;
 
     /* if no docstring given and the getter has one, use that one */
     if ((doc == NULL || doc == Py_None) && get != NULL) {
-        PyObject *get_doc = PyObject_GetAttrString(get, "__doc__");
+        _Py_IDENTIFIER(__doc__);
+        PyObject *get_doc = _PyObject_GetAttrId(get, &PyId___doc__);
         if (get_doc) {
             if (Py_TYPE(self) == &PyProperty_Type) {
                 Py_XDECREF(prop->prop_doc);
                 prop->prop_doc = get_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. */
-                int err = PyObject_SetAttrString(self, "__doc__", get_doc);
+                int err = _PyObject_SetAttrId(self, &PyId___doc__, get_doc);
                 Py_DECREF(get_doc);
                 if (err < 0)
                     return -1;
             }
             prop->getter_doc = 1;
         }
         else if (PyErr_ExceptionMatches(PyExc_Exception)) {
             PyErr_Clear();
         }
         else {
             return -1;
         }
     }
 
-    /* set __isabstractmethod__ if fget, fset, or fdel are abstract methods */

[durban, 2011/03/19 21:36:10]

This doesn't handle the case if the fget/fset/fdel functions are mutated after
the creation of the property object.  (Possibly it would be better to use a
descriptor as the __isabstractmethod__ attribute, that would check the methods
on __get__.)

[dsdale24, 2011/03/19 22:13:42]

Would that ever happen? I think the only way to change them is to call .getter
and .setter, which actually creates a new copy of the property instead of
modifying it in place.

-    property_identify_abstract_method(self, get);
-    property_identify_abstract_method(self, set);
-    property_identify_abstract_method(self, del);
-
     return 0;
 }
+
+static PyObject *
+property_get___isabstractmethod__(propertyobject *prop, void *closure)
+{
+    int res = _PyObject_IsAbstract(prop->prop_get);
+    if (res == -1) {
+        return NULL;
+    }
+    else if (res) {
+        Py_RETURN_TRUE;
+    }
+
+    res = _PyObject_IsAbstract(prop->prop_set);
+    if (res == -1) {
+        return NULL;
+    }
+    else if (res) {
+        Py_RETURN_TRUE;
+    }
+
+    res = _PyObject_IsAbstract(prop->prop_del);
+    if (res == -1) {
+        return NULL;
+    }
+    else if (res) {
+        Py_RETURN_TRUE;
+    }
+    Py_RETURN_FALSE;
+}
+
+static PyGetSetDef property_getsetlist[] = {
+    {"__isabstractmethod__",
+     (getter)property_get___isabstractmethod__, NULL,
+     NULL,
+     NULL},
+    {NULL} /* Sentinel */
+};
 
 PyDoc_STRVAR(property_doc,
 "property(fget=None, fset=None, fdel=None, doc=None) -> property attribute\n"
 "\n"
 "fget is a function to be used for getting an attribute value, and likewise\n"
 "fset is a function for setting, and fdel a function for del'ing, an\n"
 "attribute.  Typical use is to define a managed attribute x:\n"
 "class C(object):\n"
 "    def getx(self): return self._x\n"
 "    def setx(self, value): self._x = value\n"
@@ skipping 46 matching lines @@
         Py_TPFLAGS_BASETYPE,                    /* tp_flags */
     property_doc,                               /* tp_doc */
     property_traverse,                          /* tp_traverse */
     0,                                          /* tp_clear */
     0,                                          /* tp_richcompare */
     0,                                          /* tp_weaklistoffset */
     0,                                          /* tp_iter */
     0,                                          /* tp_iternext */
     property_methods,                           /* tp_methods */
     property_members,                           /* tp_members */
-    0,                                          /* tp_getset */
+    property_getsetlist,                        /* tp_getset */
     0,                                          /* tp_base */
     0,                                          /* tp_dict */
     property_descr_get,                         /* tp_descr_get */
     property_descr_set,                         /* tp_descr_set */
     0,                                          /* tp_dictoffset */
     property_init,                              /* tp_init */
     PyType_GenericAlloc,                        /* tp_alloc */
     PyType_GenericNew,                          /* tp_new */
     PyObject_GC_Del,                            /* tp_free */
 };