[Patches] __contains__ hook, done right

[Guido van Rossum]

Moshe,

I've finally had some time to review your patch.  I think it's close.

> diff -r -c ../../../python/dist/src/Include/object.h ./Include/object.h
> *** ../../../python/dist/src/Include/object.h	Wed Feb  2 16:02:51 2000
> --- ./Include/object.h	Fri Feb  4 13:39:13 2000
> ***************
> *** 151,156 ****
> --- 151,160 ----
>   typedef int (*getsegcountproc) Py_PROTO((PyObject *, int *));
>   typedef int (*getcharbufferproc) Py_PROTO((PyObject *, int, const char **));
>   
> + /* tentative (by Moshe) */

Please don't put comments like this in :-)  (See the patches style
guide: http://www.python.org/patches/style.html).

> + typedef int(*objobjproc) Py_PROTO((PyObject *, PyObject *));
> + 
> + 
>   typedef struct {
>   	binaryfunc nb_add;
>   	binaryfunc nb_subtract;
> ***************
> *** 185,190 ****
> --- 189,195 ----
>   	intintargfunc sq_slice;
>   	intobjargproc sq_ass_item;
>   	intintobjargproc sq_ass_slice;
> + 	objobjproc sq_contains;
>   } PySequenceMethods;
>   
>   typedef struct {
> ***************
> *** 316,321 ****
> --- 321,327 ----
>   
>   /* PyBufferProcs contains bf_getcharbuffer */
>   #define Py_TPFLAGS_HAVE_GETCHARBUFFER  (1L<<0)
> + #define Py_TPFLAGS_HAVE_SEQUENCE_IN    (1L<<1)
>   
>   #define Py_TPFLAGS_DEFAULT  (Py_TPFLAGS_HAVE_GETCHARBUFFER)

I would modify this to include Py_TPFLAGS_HAVE_SEQUENCE_IN by default.
The flag means that the code knows that the sq_contains field exists;
not that this particular object has a non-NULL value in it.  So it can
always be on in code compiled with this version of the header file.

> diff -r -c ../../../python/dist/src/Objects/abstract.c ./Objects/abstract.c
> *** ../../../python/dist/src/Objects/abstract.c	Fri Oct 15 14:09:02 1999
> --- ./Objects/abstract.c	Fri Feb  4 14:00:45 2000
> ***************
> *** 1110,1116 ****
> --- 1110,1121 ----
>   		}
>   		return 0;
>   	}
> + 	if(PyType_HasFeature(w->ob_type, Py_TPFLAGS_HAVE_SEQUENCE_IN)) {
> + 		sq = w->ob_type->tp_as_sequence;

Here you have to check that sq != NULL first.  (This is because I
changed the meaning of the flag!)

> + 		if(sq->sq_contains != NULL)
> + 			return (*sq->sq_contains)(w, v);
>   
> + 	}

I would put the blank line *after* the brace :-)

>   	sq = w->ob_type->tp_as_sequence;
>   	if (sq == NULL || sq->sq_item == NULL) {
>   		PyErr_SetString(PyExc_TypeError,
> diff -r -c ../../../python/dist/src/Objects/classobject.c ./Objects/classobject.c
> *** ../../../python/dist/src/Objects/classobject.c	Wed Feb  2 16:03:19 2000
> --- ./Objects/classobject.c	Fri Feb  4 13:41:03 2000
> ***************
> *** 1065,1070 ****
> --- 1065,1096 ----
>   	return 0;
>   }
>   
> + static int instance_contains(PyInstanceObject *inst, PyObject *member)
> + {
> + 	static PyObject *__contains__;
> + 	PyObject *func, *arg, *res;
> + 	int ret;
> + 
> + 	if(__contains__ == NULL)
> + 		__contains__ = PyString_InternFromString("__contains__");

Wot?  No error check?  What if we run out of memory?  (See examples
using PyString_InternFromString() elsewhere in classobject.c.)

> + 	func = instance_getattr(inst, __contains__);
> + 	if(func == NULL)
> + 		return -1;
> + 	arg = Py_BuildValue("(O)", member);
> + 	if(arg == NULL) {
> + 		Py_DECREF(func);
> + 		return -1;
> + 	}
> + 	res = PyEval_CallObject(func, arg);
> + 	Py_DECREF(func);
> + 	Py_DECREF(arg);
> + 	if(res == NULL)
> + 		return -1;
> + 	ret = PyObject_IsTrue(res);
> + 	Py_DECREF(res);
> + 	return ret;
> + }
> + 

Good!

>   static PySequenceMethods instance_as_sequence = {
>   	(inquiry)instance_length, /*sq_length*/
>   	0, /*sq_concat*/
> ***************
> *** 1073,1078 ****
> --- 1099,1105 ----
>   	(intintargfunc)instance_slice, /*sq_slice*/
>   	(intobjargproc)instance_ass_item, /*sq_ass_item*/
>   	(intintobjargproc)instance_ass_slice, /*sq_ass_slice*/
> + 	(objobjproc)instance_contains, /* sq_contains */
>   };
>   
>   static PyObject *
> ***************
> *** 1405,1410 ****
> --- 1432,1439 ----
>   	0,			/*tp_str*/
>   	(getattrofunc)instance_getattr, /*tp_getattro*/
>   	(setattrofunc)instance_setattr, /*tp_setattro*/
> + 	0,                     /* tp_as_buffer */
> + 	Py_TPFLAGS_HAVE_SEQUENCE_IN, /* tp_flags */

This could be Py_TPFLAGS_DEFAULT.

>   };
>   
>   
>

I was hoping that the special casing for strings, lists and tuples
would also be moved to those respective files, from the code in
abstract.c.

--Guido van Rossum (home page: http://www.python.org/~guido/)