ExtensionClass plans (was Re: [ZODB-Dev] Another ExtensionClass incompatibility: __pow__())

Neil Schemenauer nas@mems-exchange.org
Mon, 10 Dec 2001 14:14:26 -0500


--Nq2Wo0NMKNjxTN9z
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Dec 01, 2001 at 12:18:40PM -0500, Jim Fulton wrote:
> Greg Ward wrote:
> > Are the little incompatibility bugs between classic Python classes
> > and ExtensionClass ever going to be fixed? 
> 
> Probably not by us. Patches are gratefully accepted.

Attached are two patches.  They are not without issues however.  First,
I have almost no understanding of how ExtensionClasses work.  I'm sure
there are problems due to my lack of understanding.  Second, both
patches change ExtensionClass in an incompatible way.

The first patch adds support for reverse numeric operations (__radd__,
and friends).  To make this work the Py_TPFLAGS_CHECKTYPES type flag is
set.  If an existing extension class written in C defines numeric slots
then it will most likely break (it would expect the first argument to be
the right type).  Luckly, ZODB does not seem to define any numeric
slots.

The second patch adds rich comparisons.  Again, it does so in a
incompatible way.  The ExtensionClass structure is not large enough to
hold the tp_richcompare slot.  All C extension classes would need to be
modified.

Jim, I would appreciate any comments you have on these patches.  We
probably will not use the second patch because it breaks ZODB.

I'm looking forward to the new ZODB.

  Neil


--Nq2Wo0NMKNjxTN9z
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="rops2.diff"

--- ExtensionClass.h.orig	Mon Dec 10 09:10:53 2001
+++ ExtensionClass.h	Mon Dec 10 12:39:07 2001
@@ -143,9 +143,12 @@
 	reprfunc tp_str;
 	getattrofunc tp_getattro;
 	setattrofunc tp_setattro;
-	/* Space for future expansion */
-	long tp_xxx3;
-	long tp_xxx4;
+
+	/* Functions to access object as input/output buffer */
+	PyBufferProcs *tp_as_buffer;
+	
+	/* Flags to define presence of optional/expanded features */
+	long tp_flags;
 
 	char *tp_doc; /* Documentation string */
 
@@ -338,6 +341,7 @@
 	0, # NAME, sizeof(PyPureMixinObject), 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
 	0, 0, 0, 0, 0, 0, 0, DOC, {METHODS, NULL}, \
         EXTENSIONCLASS_BASICNEW_FLAG}
+
 
 /* The following macros provide limited access to extension-class
    method facilities. */
--- ExtensionClass.c.orig	Mon Dec 10 09:10:47 2001
+++ ExtensionClass.c	Mon Dec 10 13:59:38 2001
@@ -59,6 +59,8 @@
 
 #include <stdio.h>
 #include "ExtensionClass.h"
+#include "structmember.h" /* we need the offsetof() macro from there */
+
 
 static void
 PyVar_Assign(PyObject **v,  PyObject *e)
@@ -101,39 +103,51 @@
 
 #define METH_BY_NAME (2 << 16)
 
-static PyObject *py__add__, *py__sub__, *py__mul__, *py__div__,
-  *py__mod__, *py__pow__, *py__divmod__, *py__lshift__, *py__rshift__,
-  *py__and__, *py__or__, *py__xor__, *py__coerce__, *py__neg__,
-  *py__pos__, *py__abs__, *py__nonzero__, *py__inv__, *py__int__,
-  *py__long__, *py__float__, *py__oct__, *py__hex__,
-  *py__of__, *py__call__, *py__call_method__,
-  *py__getitem__, *py__setitem__, *py__delitem__,
+static PyObject *py__add__, *py__radd__, *py__sub__, *py__rsub__,
+  *py__mul__, *py__rmul__, *py__div__, *py__rdiv__, *py__mod__, *py__rmod__,
+  *py__pow__, *py__rpow__, *py__divmod__, *py__rdivmod__, *py__lshift__,
+  *py__rlshift__, *py__rshift__, *py__rrshift__, *py__and__, *py__rand__,
+  *py__or__, *py__ror__, *py__xor__, *py__rxor__, *py__coerce__, *py__neg__,
+  *py__pos__, *py__abs__, *py__nonzero__, *py__inv__, *py__int__, *py__long__,
+  *py__float__, *py__oct__, *py__hex__, *py__of__, *py__call__,
+  *py__call_method__, *py__getitem__, *py__setitem__, *py__delitem__,
   *py__getslice__, *py__setslice__, *py__delslice__, *py__len__,
-  *py__getattr__, *py__setattr__, *py__delattr__,
-  *py__del__, *py__repr__, *py__str__, *py__class__, *py__name__,
-  *py__hash__, *py__cmp__, *py__var_size__, *py__init__, *py__getinitargs__,
-  *py__getstate__, *py__setstate__, *py__dict__, *pyclass_,
-  *py__module__;
+  *py__getattr__, *py__setattr__, *py__delattr__, *py__del__, *py__repr__,
+  *py__str__, *py__class__, *py__name__, *py__hash__, *py__cmp__,
+  *py__var_size__, *py__init__, *py__getinitargs__, *py__getstate__,
+  *py__setstate__, *py__dict__, *pyclass_, *py__module__;
 
 static PyObject *concat_fmt=0;
 static PyObject *subclass_watcher=0;  /* Object that subclass events */
 
 static void
-init_py_names()
+init_py_names(void)
 {
 #define INIT_PY_NAME(N) py ## N = PyString_FromString(#N)
   INIT_PY_NAME(__add__);
+  INIT_PY_NAME(__radd__);
   INIT_PY_NAME(__sub__);
+  INIT_PY_NAME(__rsub__);
   INIT_PY_NAME(__mul__);
+  INIT_PY_NAME(__rmul__);
   INIT_PY_NAME(__div__);
+  INIT_PY_NAME(__rdiv__);
   INIT_PY_NAME(__mod__);
+  INIT_PY_NAME(__rmod__);
   INIT_PY_NAME(__pow__);
+  INIT_PY_NAME(__rpow__);
   INIT_PY_NAME(__divmod__);
+  INIT_PY_NAME(__rdivmod__);
   INIT_PY_NAME(__lshift__);
+  INIT_PY_NAME(__rlshift__);
   INIT_PY_NAME(__rshift__);
+  INIT_PY_NAME(__rrshift__);
   INIT_PY_NAME(__and__);
+  INIT_PY_NAME(__rand__);
   INIT_PY_NAME(__or__);
+  INIT_PY_NAME(__ror__);
   INIT_PY_NAME(__xor__);
+  INIT_PY_NAME(__rxor__);
   INIT_PY_NAME(__coerce__);
   INIT_PY_NAME(__neg__);
   INIT_PY_NAME(__pos__);
@@ -622,6 +636,7 @@
   return NULL;
 }
 
+#if 0
 static int
 CMethod_setattro(CMethod *self, PyObject *oname, PyObject *v)
 {
@@ -639,6 +654,7 @@
   PyErr_SetObject(PyExc_AttributeError, oname);
   return -1;
 }
+#endif
 
 static PyTypeObject CMethodType = {
   PyObject_HEAD_INIT(NULL)
@@ -940,6 +956,7 @@
   return PyObject_GetAttr(self->meth, oname);
 }
 
+#if 0
 static int
 PMethod_setattro(PMethod *self, PyObject *oname, PyObject *v)
 {
@@ -975,6 +992,7 @@
   PyErr_SetObject(PyExc_AttributeError, oname);
   return -1;
 }
+#endif
 
 static PyTypeObject PMethodType = {
   PyObject_HEAD_INIT(NULL)
@@ -2443,80 +2461,73 @@
   return m;
 }  
 
-#define BINSUB(M,N,A) \
-static PyObject * \
-subclass_ ## M(PyObject *self, PyObject *v) \
-{ \
-  PyObject *m; \
-  UNLESS(m=subclass_getspecial(self,py__ ## N ## __)) return NULL; \
-  if (UnboundCMethod_Check(m) \
-     && AsCMethod(m)->meth==(PyCFunction)M ## _by_name \
-     && SubclassInstance_Check(self,AsCMethod(m)->type) \
-     && ! HasMethodHook(self)) \
-    ASSIGN(m,AsCMethod(m)->type->tp_as_number->nb_ ## M(self,v)); \
-  else if (UnboundEMethod_Check(m)) \
-    ASSIGN(m,PyObject_CallFunction(m,"OO",self,v)); \
-  else \
-    ASSIGN(m,PyObject_CallFunction(m,"O",v)); \
-  return m; \
-}  
-
-static PyObject * 
-subclass_add(PyObject *self, PyObject *v)
-{ 
-  PyObject *m; 
-
-  UNLESS(m=subclass_getspecial(self,py__add__)) return NULL; 
+#define NB_SLOT(x) offsetof(PyNumberMethods, x)
+#define NB_BINOP(nb_methods, slot) \
+		((binaryfunc*)(& ((char*)nb_methods)[slot] ))
 
-  if (UnboundCMethod_Check(m)
-     && AsCMethod(m)->meth==(PyCFunction)concat_by_name 
-     && SubclassInstance_Check(self,AsCMethod(m)->type)
-     && ! HasMethodHook(self)) 
+static PyObject *
+subclass_binop1(PyObject *self, PyObject *v, PyObject *opname,
+		int slot, PyCFunction meth_by_name)
+{
+  PyObject *m;
+  if (!ExtensionInstance_Check(self)) {
+    Py_INCREF(Py_NotImplemented);
+    return Py_NotImplemented;
+  }
+  UNLESS(m=subclass_getspecial(self,opname)) return NULL;
+  if (opname == py__add__
+      && UnboundCMethod_Check(m)
+      && AsCMethod(m)->meth==(PyCFunction)concat_by_name 
+      && SubclassInstance_Check(self,AsCMethod(m)->type)
+      && ! HasMethodHook(self)) 
     ASSIGN(m,AsCMethod(m)->type->tp_as_sequence->sq_concat(self,v)); 
+  else if (opname == py__mul__
+           && UnboundCMethod_Check(m)
+           && AsCMethod(m)->meth==(PyCFunction)multiply_by_name 
+           && SubclassInstance_Check(self,AsCMethod(m)->type)
+           && ! HasMethodHook(self)) {
+    int i = PyInt_AsLong(v);
+    if (i==-1 && PyErr_Occurred()) return NULL;
+    ASSIGN(m,AsCMethod(m)->type->tp_as_sequence->sq_repeat(self,i));
+  }
   else if (UnboundCMethod_Check(m)
-	  && AsCMethod(m)->meth==(PyCFunction)add_by_name 
-	  && SubclassInstance_Check(self,AsCMethod(m)->type)
-	  && ! HasMethodHook(self)) 
-    ASSIGN(m,AsCMethod(m)->type->tp_as_number->nb_add(self,v)); 
-  else if (UnboundEMethod_Check(m)) 
-    ASSIGN(m,PyObject_CallFunction(m,"OO",self,v)); 
-  else 
-    ASSIGN(m,PyObject_CallFunction(m,"O",v)); 
-
-  return m; 
-}  
-
-BINSUB(subtract,sub,Subtract)
-
-static PyObject * 
-subclass_multiply(PyObject *self, PyObject *v)
-{ 
-  PyObject *m; 
+           && meth_by_name != NULL
+           && AsCMethod(m)->meth==meth_by_name
+           && SubclassInstance_Check(self,AsCMethod(m)->type)
+           && ! HasMethodHook(self))
+    ASSIGN(m,(*NB_BINOP(AsCMethod(m)->type->tp_as_number, slot))(self,v));
+  else if (UnboundEMethod_Check(m))
+    ASSIGN(m,PyObject_CallFunction(m,"OO",self,v));
+  else
+    ASSIGN(m,PyObject_CallFunction(m,"O",v));
+  return m;
+}
 
-  UNLESS(m=subclass_getspecial(self,py__mul__)) return NULL; 
-  if (UnboundCMethod_Check(m)
-     && AsCMethod(m)->meth==(PyCFunction)repeat_by_name 
-     && SubclassInstance_Check(self,AsCMethod(m)->type)
-     && ! HasMethodHook(self))
-    {
-      int i;
+static PyObject *
+subclass_binop(PyObject *self, PyObject *v,
+	       PyObject *opname, PyObject *ropname,
+               int slot, PyCFunction meth_by_name)
+{
+  PyObject *result = subclass_binop1(self, v, opname, slot, meth_by_name);
+  if (result == Py_NotImplemented) {
+    Py_DECREF(result);
+    result = subclass_binop1(v, self, ropname, slot, NULL);
+  }
+  return result;
+}
 
-      i=PyInt_AsLong(v);
-      if (i==-1 && PyErr_Occurred()) return NULL;
-      ASSIGN(m,AsCMethod(m)->type->tp_as_sequence->sq_repeat(self,i));
-    }
-  else if (UnboundCMethod_Check(m)
-	  && AsCMethod(m)->meth==(PyCFunction)multiply_by_name 
-	  && SubclassInstance_Check(self,AsCMethod(m)->type)
-	  && ! HasMethodHook(self)) 
-    ASSIGN(m,AsCMethod(m)->type->tp_as_number->nb_multiply(self,v)); 
-  else if (UnboundEMethod_Check(m)) 
-    ASSIGN(m,PyObject_CallFunction(m,"OO",self,v)); 
-  else 
-    ASSIGN(m,PyObject_CallFunction(m,"O",v)); 
-  return m; 
-}  
+#define BINSUB(M,N,A) \
+static PyObject * \
+subclass_ ## M(PyObject *self, PyObject *v) \
+{ \
+  return subclass_binop(self, v, py__ ## N ## __, py__r ## N ## __, \
+			NB_SLOT(nb_ ## M), \
+			(PyCFunction)M ## _by_name); \
+} 
 
+BINSUB(add,add,Add)
+BINSUB(subtract,sub,Subtract)
+BINSUB(multiply,mul,Multiply)
 BINSUB(divide,div,Divide)
 BINSUB(remainder,mod,Remainder)
 
@@ -2524,6 +2535,11 @@
 subclass_power(PyObject *self, PyObject *v, PyObject *w) 
 { 
   PyObject *m; 
+  if (!ExtensionInstance_Check(self)) {
+    Py_INCREF(Py_NotImplemented);
+    return Py_NotImplemented;
+  }
+  /* XXX add support for __rpow__ */
   UNLESS(m=subclass_getspecial(self,py__pow__)) return NULL; 
   if (UnboundCMethod_Check(m)
      && AsCMethod(m)->meth==(PyCFunction)power_by_name
@@ -3405,6 +3421,7 @@
   subclass_set(hash,hash);
   subclass_set(call,call);
   subclass_set(str,str);
+  self->tp_flags = Py_TPFLAGS_CHECKTYPES;
   self->tp_doc=0;
 
   /* Implement __module__=__name__ */
@@ -3527,7 +3544,7 @@
 };
 
 void
-initExtensionClass()
+initExtensionClass(void)
 {
   PyObject *m, *d;
   char *rev="$Revision: 1.1 $";

--Nq2Wo0NMKNjxTN9z
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="richcomp1.diff"

--- ExtensionClass.c~	Mon Dec 10 12:40:13 2001
+++ ExtensionClass.c	Mon Dec 10 12:37:51 2001
@@ -1078,6 +1078,16 @@
 } 
 
 static PyObject *
+richcompare_by_name(PyObject *self, PyObject *args, PyTypeObject *ob_type)
+{
+  PyObject *other;
+
+  UNLESS(PyArg_ParseTuple(args,"O", &other)) return NULL; 
+  /* XXX where to get third argument? */
+  return ob_type->tp_richcompare(self,other,Py_EQ); 
+} 
+
+static PyObject *
 getattr_by_name(PyObject *self, PyObject *args, PyTypeObject *ob_type)
 {
   char *name;
@@ -1923,6 +1933,7 @@
 	  SET_SPECIAL(setattro,setattr);
 	  */
 	  SET_SPECIAL(compare,cmp);
+	  SET_SPECIAL(richcompare,richcmp);
 	  SET_SPECIAL(hash,hash);
 	  SET_SPECIAL(repr,repr);
 	  SET_SPECIAL(call,call);
@@ -2353,6 +2364,80 @@
   return r;
 }  
 
+/* Map rich comparison operators to their __xx__ namesakes */
+static char *name_op[] = {
+	"__lt__",
+	"__le__",
+	"__eq__",
+	"__ne__",
+	"__gt__",
+	"__ge__",
+};
+
+static PyObject *
+subclass_half_richcompare(PyObject *self, PyObject *v, int op)
+{
+  PyObject *name;
+  PyObject *m;
+
+  name = PyString_InternFromString(name_op[op]);
+  if (name == NULL)
+    return NULL;
+
+  m = subclass_getspecial(self, name);
+  Py_DECREF(name);
+  if (m == NULL) {
+    if (!PyErr_ExceptionMatches(PyExc_AttributeError))
+      return NULL;
+    PyErr_Clear();
+    m = Py_NotImplemented;
+    Py_INCREF(m);
+    return m;
+  }
+
+  if (UnboundCMethod_Check(m)
+      && AsCMethod(m)->meth==(PyCFunction)richcompare_by_name
+      && SubclassInstance_Check(self,AsCMethod(m)->type)
+      && ! HasMethodHook(self))
+    ASSIGN(m, AsCMethod(m)->type->tp_richcompare(self,v,op));
+  else
+    {
+      if (UnboundEMethod_Check(m))
+	{
+	  UNLESS_ASSIGN(m,PyObject_CallFunction(m,"OO",self,v)) return NULL;
+	}
+      else UNLESS_ASSIGN(m,PyObject_CallFunction(m,"O",v)) return NULL;
+    }
+  return m;
+}
+
+/* Map rich comparison operators to their swapped version, e.g. LT --> GT */
+static int swapped_op[] = {Py_GT, Py_GE, Py_EQ, Py_NE, Py_LT, Py_LE};
+
+static PyObject *
+subclass_richcompare(PyObject *v, PyObject *w, int op)
+{
+  PyObject *res;
+
+  if (ExtensionInstance_Check(v)) {
+    res = subclass_half_richcompare(v, w, op);
+    if (res != Py_NotImplemented)
+      return res;
+    Py_DECREF(res);
+  }
+
+  if (ExtensionInstance_Check(w)) {
+    res = subclass_half_richcompare(w, v, swapped_op[op]);
+    if (res != Py_NotImplemented)
+      return res;
+    Py_DECREF(res);
+  }
+
+  Py_INCREF(Py_NotImplemented);
+  return Py_NotImplemented;
+}
+
+
 static long
 subclass_hash(PyObject *self)
 {
@@ -3467,6 +3552,7 @@
   self->tp_ ##OP = subclass_ ##OP
   
   subclass_set(compare,cmp);
+  subclass_set(richcompare,richcmp);
   subclass_set(repr,repr);
 
   if (subclass_hasattr(self,py__of__))
@@ -3505,7 +3591,7 @@
   subclass_set(hash,hash);
   subclass_set(call,call);
   subclass_set(str,str);
-  self->tp_flags = Py_TPFLAGS_CHECKTYPES;
+  self->tp_flags = Py_TPFLAGS_CHECKTYPES | Py_TPFLAGS_HAVE_RICHCOMPARE;
   self->tp_doc=0;
 
   /* Implement __module__=__name__ */
@@ -3567,6 +3653,7 @@
   /* Space for future expansion */
   0L,0L,
   "C classes", /* Documentation string */
+  0L,0L,0L,
   METHOD_CHAIN(ExtensionClass_methods)
 };
 
--- ExtensionClass.h~	Mon Dec 10 12:39:07 2001
+++ ExtensionClass.h	Mon Dec 10 12:37:51 2001
@@ -152,6 +152,13 @@
 
 	char *tp_doc; /* Documentation string */
 
+	/* GC */
+	traverseproc tp_traverse;
+	inquiry tp_clear;
+
+	/* rich comparisons */
+	richcmpfunc tp_richcompare;
+
 #ifdef COUNT_ALLOCS
 	/* these must be last */
 	int tp_alloc;
@@ -339,7 +346,7 @@
 #define PURE_MIXIN_CLASS(NAME,DOC,METHODS) \
 static PyExtensionClass NAME ## Type = { PyObject_HEAD_INIT(NULL) \
 	0, # NAME, sizeof(PyPureMixinObject), 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
-	0, 0, 0, 0, 0, 0, 0, DOC, {METHODS, NULL}, \
+	0, 0, 0, 0, 0, 0, 0, DOC, 0, 0, 0, {METHODS, NULL}, \
         EXTENSIONCLASS_BASICNEW_FLAG}
 
 

--Nq2Wo0NMKNjxTN9z--