Numerous fix-ups to C API and docs. Added tests for C API.

4 files changed, 148 insertions, 26 deletions

diff --git a/Doc/api/concrete.tex b/Doc/api/concrete.tex
index 2f37be5..6c7721d 100644
--- a/Doc/api/concrete.tex
+++ b/Doc/api/concrete.tex
@@ -2959,14 +2959,16 @@ Likewise, the constructor functions work with any iterable Python object.
   Returns a new \class{set} containing objects returned by the
   \var{iterable}.  The \var{iterable} may be \NULL{} to create a
   new empty set.  Returns the new set on success or \NULL{} on
-  failure.
+  failure.  Raises \exception{TypeError} if \var{iterable} is
+  not actually iterable.
 \end{cfuncdesc}
 
 \begin{cfuncdesc}{PyObject*}{PyFrozenSet_New}{PyObject *iterable}
   Returns a new \class{frozenset} containing objects returned by the
   \var{iterable}.  The \var{iterable} may be \NULL{} to create a
   new empty frozenset.  Returns the new set on success or \NULL{} on
-  failure.
+  failure.  Raises \exception{TypeError} if \var{iterable} is
+  not actually iterable.
 \end{cfuncdesc}
 
 
@@ -2976,7 +2978,7 @@ The following functions and macros are available for instances of
 \begin{cfuncdesc}{int}{PySet_Size}{PyObject *anyset}
   Returns the length of a \class{set} or \class{frozenset} object.
   Equivalent to \samp{len(\var{anyset})}.  Raises a
-  \exception{PyExc_SystemError} if the argument is not a \class{set},
+  \exception{PyExc_SystemError} if \var{anyset} is not a \class{set},
   \class{frozenset}, or an instance of a subtype.
   \bifuncindex{len}
 \end{cfuncdesc}
@@ -2989,15 +2991,9 @@ The following functions and macros are available for instances of
   Returns 1 if found, 0 if not found, and -1 if an error is
   encountered.  Unlike the Python \method{__contains__()} method, this
   function does not automatically convert unhashable sets into temporary
-  frozensets.  Raises a \exception{TypeError} if the key is unhashable.
-\end{cfuncdesc}
-
-\begin{cfuncdesc}{int}{PySet_Discard}{PyObject *anyset, PyObject *key}
-  Returns 1 if found and removed, 0 if not found (no action taken),
-  and -1 if an error is encountered.  Does not raise \exception{KeyError}
-  for missing keys.  Raises a \exception{TypeError} if the key is unhashable.
-  Unlike the Python \method{discard()} method, this function does
-  not automatically convert unhashable sets into temporary frozensets.  
+  frozensets.  Raises a \exception{TypeError} if the \var{key} is unhashable.
+  Raises \exception{PyExc_SystemError} if \var{anyset} is not a \class{set},
+  \class{frozenset}, or an instance of a subtype.                         
 \end{cfuncdesc}
 
 
@@ -3007,17 +3003,27 @@ its subtypes but not for instances of \class{frozenset} or its subtypes.
 \begin{cfuncdesc}{int}{PySet_Add}{PyObject *set, PyObject *key}
   Adds \var{key} to a \class{set} instance.  Does not apply to
   \class{frozenset} instances.  Returns 0 on success or -1 on failure.
-  Raises a \exception{TypeError} if the key is unhashable.
+  Raises a \exception{TypeError} if the \var{key} is unhashable.
   Raises a \exception{MemoryError} if there is no room to grow.
-  Raises a \exception{SystemError} if \var{key} is an not an instance
+  Raises a \exception{SystemError} if \var{set} is an not an instance
   of \class{set} or its subtype.
 \end{cfuncdesc}
 
+\begin{cfuncdesc}{int}{PySet_Discard}{PyObject *set, PyObject *key}
+  Returns 1 if found and removed, 0 if not found (no action taken),
+  and -1 if an error is encountered.  Does not raise \exception{KeyError}
+  for missing keys.  Raises a \exception{TypeError} if the \var{key} is
+  unhashable.  Unlike the Python \method{discard()} method, this function
+  does not automatically convert unhashable sets into temporary frozensets.
+  Raises \exception{PyExc_SystemError} if \var{set} is an not an instance
+  of \class{set} or its subtype.                         
+\end{cfuncdesc}
+
 \begin{cfuncdesc}{PyObject*}{PySet_Pop}{PyObject *set}
   Returns a new reference to an arbitrary object in the \var{set},
   and removes the object from the \var{set}.  Returns \NULL{} on
   failure.  Raises \exception{KeyError} if the set is empty.
-  Raises a \exception{SystemError} if \var{key} is an not an instance
+  Raises a \exception{SystemError} if \var{set} is an not an instance
   of \class{set} or its subtype.                        
 \end{cfuncdesc}
 
diff --git a/Include/setobject.h b/Include/setobject.h
index b6849e1..1b0e5b1 100644
--- a/Include/setobject.h
+++ b/Include/setobject.h
@@ -79,7 +79,7 @@ PyAPI_FUNC(PyObject *) PyFrozenSet_New(PyObject *);
 PyAPI_FUNC(int) PySet_Size(PyObject *anyset);
 #define PySet_GET_SIZE(so) (((PySetObject *)(so))->used)
 PyAPI_FUNC(int) PySet_Contains(PyObject *anyset, PyObject *key);
-PyAPI_FUNC(int) PySet_Discard(PyObject *anyset, PyObject *key);
+PyAPI_FUNC(int) PySet_Discard(PyObject *set, PyObject *key);
 PyAPI_FUNC(int) PySet_Add(PyObject *set, PyObject *key);
 PyAPI_FUNC(PyObject *) PySet_Pop(PyObject *set);
 
diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py
index f393712..2ebeff6 100644
--- a/Lib/test/test_set.py
+++ b/Lib/test/test_set.py
@@ -6,6 +6,7 @@ import copy
 import pickle
 import os
 from random import randrange, shuffle
+import sys
 
 class PassThru(Exception):
     pass
@@ -402,6 +403,11 @@ class TestSet(TestJointOps):
         s = None
         self.assertRaises(ReferenceError, str, p)
 
+    # C API test only available in a debug build
+    if hasattr(sys, "gettotalrefcount"):
+        def test_c_api(self):
+            self.assertEqual(set('abc').test_c_api(), True)
+
 class SetSubclass(set):
     pass
 
@@ -1372,7 +1378,6 @@ class TestVariousIteratorArgs(unittest.TestCase):
 #==============================================================================
 
 def test_main(verbose=None):
-    import sys
     from test import test_sets
     test_classes = (
         TestSet,
diff --git a/Objects/setobject.c b/Objects/setobject.c
index 879b4e1..ba06cb6 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -1697,6 +1697,13 @@ static PySequenceMethods set_as_sequence = {
 
 /* set object ********************************************************/
 
+#ifdef Py_DEBUG
+static PyObject *test_c_api(PySetObject *so);
+
+PyDoc_STRVAR(test_c_api_doc, "Exercises C API.  Returns True.\n\
+All is well if assertions don't fail.");
+#endif
+
 static PyMethodDef set_methods[] = {
 	{"add",		(PyCFunction)set_add,		METH_O,
 	 add_doc},
@@ -1730,6 +1737,10 @@ static PyMethodDef set_methods[] = {
 	 symmetric_difference_doc},
 	{"symmetric_difference_update",(PyCFunction)set_symmetric_difference_update,	METH_O,
 	 symmetric_difference_update_doc},
+#ifdef Py_DEBUG
+	{"test_c_api",	(PyCFunction)test_c_api,	METH_NOARGS,
+	 test_c_api_doc},
+#endif
 	{"union",	(PyCFunction)set_union,		METH_O,
 	 union_doc},
 	{"update",	(PyCFunction)set_update,	METH_O,
@@ -1931,19 +1942,30 @@ PySet_New(PyObject *iterable)
 PyObject *
 PyFrozenSet_New(PyObject *iterable)
 {
-	PyObject *args = NULL, *result;
+	PyObject *args, *result;
 
-	if (iterable != NULL) {
+	if (iterable == NULL)
+		args = PyTuple_New(0);
+	else
 		args = PyTuple_Pack(1, iterable);
-		if (args == NULL)
-			return NULL;
-	}
+	if (args == NULL)
+		return NULL;
 	result = frozenset_new(&PyFrozenSet_Type, args, NULL);
-	Py_XDECREF(args);
+	Py_DECREF(args);
 	return result;
 }
 
 int
+PySet_Size(PyObject *anyset)
+{
+	if (!PyAnySet_Check(anyset)) {
+		PyErr_BadInternalCall();
+		return -1;
+	}
+	return ((PySetObject *)anyset)->used;
+}
+
+int
 PySet_Contains(PyObject *anyset, PyObject *key)
 {
 	if (!PyAnySet_Check(anyset)) {
@@ -1954,13 +1976,13 @@ PySet_Contains(PyObject *anyset, PyObject *key)
 }
 
 int
-PySet_Discard(PyObject *anyset, PyObject *key)
+PySet_Discard(PyObject *set, PyObject *key)
 {
-	if (!PyAnySet_Check(anyset)) {
+	if (!PyType_IsSubtype(set->ob_type, &PySet_Type)) {
 		PyErr_BadInternalCall();
 		return -1;
 	}
-	return set_discard_key((PySetObject *)anyset, key);
+	return set_discard_key((PySetObject *)set, key);
 }
 
 int
@@ -1982,3 +2004,92 @@ PySet_Pop(PyObject *set)
 	}
 	return set_pop((PySetObject *)set);
 }
+
+
+#ifdef Py_DEBUG
+
+/* Test code to be called with any three element set. 
+   Returns True and original set is restored. */
+
+#define assertRaises(call_return_value, exception)		\
+	do {							\
+		assert(call_return_value);			\
+		assert(PyErr_ExceptionMatches(exception));	\
+		PyErr_Clear();					\
+	} while(0)
+
+static PyObject *
+test_c_api(PySetObject *so)
+{
+	PyObject *elem, *dup, *t, *f, *ob = (PyObject *)so;
+
+	/* Verify preconditions and exercise type/size checks */
+	assert(PyAnySet_Check(ob));
+	assert(PyAnySet_CheckExact(ob));
+	assert(!PyFrozenSet_CheckExact(ob));
+	assert(PySet_Size(ob) == 3);
+	assert(PySet_GET_SIZE(ob) == 3);
+
+	/* Raise TypeError for non-iterable constructor arguments */
+	assertRaises(PySet_New(Py_None) == NULL, PyExc_TypeError);
+	assertRaises(PyFrozenSet_New(Py_None) == NULL, PyExc_TypeError);
+
+	/* Raise TypeError for unhashable key */
+	dup = PySet_New(ob);
+	assertRaises(PySet_Discard(ob, dup) == -1, PyExc_TypeError);
+	assertRaises(PySet_Contains(ob, dup) == -1, PyExc_TypeError);
+	assertRaises(PySet_Add(ob, dup) == -1, PyExc_TypeError);
+
+	/* Exercise successful pop, contains, add, and discard */
+	elem = PySet_Pop(ob);
+	assert(PySet_Contains(ob, elem) == 0);
+	assert(PySet_GET_SIZE(ob) == 2);
+	assert(PySet_Add(ob, elem) == 0);
+	assert(PySet_Contains(ob, elem) == 1);
+	assert(PySet_GET_SIZE(ob) == 3);
+	assert(PySet_Discard(ob, elem) == 1);
+	assert(PySet_GET_SIZE(ob) == 2);
+	assert(PySet_Discard(ob, elem) == 0);
+	assert(PySet_GET_SIZE(ob) == 2);
+
+	/* Raise SystemError when self argument is not a set or frozenset. */
+	t = PyTuple_New(0);
+	assertRaises(PySet_Size(t) == -1, PyExc_SystemError);
+	assertRaises(PySet_Contains(t, elem) == -1, PyExc_SystemError);
+	Py_DECREF(t);
+
+	/* Raise SystemError when self argument is not a set. */
+	f = PyFrozenSet_New(dup);
+	assert(PySet_Size(f) == 3);
+	assert(PyFrozenSet_CheckExact(f));
+	assertRaises(PySet_Add(f, elem) == -1, PyExc_SystemError);
+	assertRaises(PySet_Discard(f, elem) == -1, PyExc_SystemError);
+	assertRaises(PySet_Pop(f) == NULL, PyExc_SystemError);
+	Py_DECREF(f);
+
+	/* Raise KeyError when popping from an empty set */
+	set_clear_internal(so);
+	assert(PySet_GET_SIZE(ob) == 0);
+	assertRaises(PySet_Pop(ob) == NULL, PyExc_KeyError);
+
+	/* Restore the set from the copy and use the abstract API */
+	assert(PyObject_CallMethod(ob, "update", "O", dup) == Py_None);
+	Py_DECREF(Py_None);
+
+	/* Verify constructors accept NULL arguments */
+	f = PySet_New(NULL);
+	assert(f != NULL);
+	assert(PySet_GET_SIZE(f) == 0);
+	Py_DECREF(f);
+	f = PyFrozenSet_New(NULL);
+	assert(f != NULL);
+	assert(PyFrozenSet_CheckExact(f));
+	assert(PySet_GET_SIZE(f) == 0);
+	Py_DECREF(f);
+
+	Py_DECREF(elem);
+	Py_DECREF(dup);
+	Py_RETURN_TRUE;
+}
+
+#endif