Add a format specifier %R to PyUnicode_FromFormat(), which embeds

the result of a call to PyObject_Repr() into the string. This makes
it possible to simplify many repr implementations.

PyUnicode_FromFormat() uses two steps to create the final string: A first
pass through the format string determines the size of the final string and
a second pass creates the string. To avoid calling PyObject_Repr() twice
for each %R specifier, PyObject_Repr() is called during the size
calculation step and the results are stored in an array (whose size is
determined at the start by counting %R specifiers).

12 files changed, 125 insertions, 149 deletions

diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c
index 1311d4db5d..fc4ef156b5 100644
--- a/Modules/_collectionsmodule.c
+++ b/Modules/_collectionsmodule.c
@@ -627,14 +627,7 @@ deque_repr(PyObject *deque)
 		return NULL;
 	}
 
-	result = PyUnicode_FromString("deque(");
-	if (result == NULL) {
-		Py_DECREF(aslist);
-		Py_ReprLeave(deque);
-		return NULL;
-	}
-	PyUnicode_AppendAndDel(&result, PyObject_Repr(aslist));
-	PyUnicode_AppendAndDel(&result, PyUnicode_FromString(")"));
+	result = PyUnicode_FromFormat("deque(%R)", aslist);
 	Py_DECREF(aslist);
 	Py_ReprLeave(deque);
 	return result;
@@ -1208,25 +1201,18 @@ defdict_print(defdictobject *dd, FILE *fp, int flags)
 static PyObject *
 defdict_repr(defdictobject *dd)
 {
-	PyObject *defrepr;
 	PyObject *baserepr;
+	PyObject *def;
 	PyObject *result;
 	baserepr = PyDict_Type.tp_repr((PyObject *)dd);
 	if (baserepr == NULL)
 		return NULL;
 	if (dd->default_factory == NULL)
-		defrepr = PyUnicode_FromString("None");
+		def = Py_None;
 	else
-		defrepr = PyObject_Repr(dd->default_factory);
-	if (defrepr == NULL) {
-		Py_DECREF(baserepr);
-		return NULL;
-	}
-	result = PyUnicode_FromString("defaultdict(");
-	PyUnicode_AppendAndDel(&result, defrepr);
-	PyUnicode_AppendAndDel(&result, PyUnicode_FromString(", "));
-	PyUnicode_AppendAndDel(&result, baserepr);
-	PyUnicode_AppendAndDel(&result, PyUnicode_FromString(")"));
+		def = dd->default_factory;
+	result = PyUnicode_FromFormat("defaultdict(%R, %U)", def, baserepr);
+	Py_DECREF(baserepr);
 	return result;
 }
 
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index 442ab832fb..2ec2332dda 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -1118,17 +1118,7 @@ element_remove(ElementObject* self, PyObject* args)
 static PyObject*
 element_repr(ElementObject* self)
 {
-    PyObject* repr;
-    char buffer[100];
-    
-    repr = PyUnicode_FromString("<Element ");
-
-    PyUnicode_AppendAndDel(&repr, PyObject_Repr(self->tag));
-
-    sprintf(buffer, " at %p>", self);
-    PyUnicode_AppendAndDel(&repr, PyUnicode_FromString(buffer));
-
-    return repr;
+    return PyUnicode_FromFormat("<Element %R at %p>", self->tag, self);
 }
 
 static PyObject*
diff --git a/Modules/_tkinter.c b/Modules/_tkinter.c
index fe568fad07..790a1be164 100644
--- a/Modules/_tkinter.c
+++ b/Modules/_tkinter.c
@@ -809,10 +809,8 @@ PyTclObject_unicode(PyTclObject *self, void *ignored)
 static PyObject *
 PyTclObject_repr(PyTclObject *self)
 {
-	char buf[50];
-	PyOS_snprintf(buf, 50, "<%s object at %p>",
-		      self->value->typePtr->name, self->value);
-	return PyUnicode_FromString(buf);
+	return PyUnicode_FromFormat("<%s object at %p>",
+	                            self->value->typePtr->name, self->value);
 }
 
 static int
diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c
index d61b1ae2eb..734952749c 100644
--- a/Modules/arraymodule.c
+++ b/Modules/arraymodule.c
@@ -1567,29 +1567,23 @@ static PyObject *
 array_repr(arrayobject *a)
 {
 	char buf[256], typecode;
-	PyObject *s, *t, *v = NULL;
+	PyObject *s, *v = NULL;
 	Py_ssize_t len;
 
 	len = a->ob_size;
 	typecode = a->ob_descr->typecode;
 	if (len == 0) {
-		PyOS_snprintf(buf, sizeof(buf), "array('%c')", typecode);
-		return PyUnicode_FromString(buf);
+		return PyUnicode_FromFormat("array('%c')", typecode);
 	}
-		
 	if (typecode == 'c')
 		v = array_tostring(a, NULL);
 	else if (typecode == 'u')
 		v = array_tounicode(a, NULL);
 	else
 		v = array_tolist(a, NULL);
-	t = PyObject_Repr(v);
-	Py_XDECREF(v);
 
-	PyOS_snprintf(buf, sizeof(buf), "array('%c', ", typecode);
-	s = PyUnicode_FromString(buf);
-	PyUnicode_AppendAndDel(&s, t);
-	PyUnicode_AppendAndDel(&s, PyUnicode_FromString(")"));
+	s = PyUnicode_FromFormat("array('%c', %R)", typecode, v);
+	Py_DECREF(v);
 	return s;
 }
 
diff --git a/Modules/datetimemodule.c b/Modules/datetimemodule.c
index 5d3c67912d..e03148cb1f 100644
--- a/Modules/datetimemodule.c
+++ b/Modules/datetimemodule.c
@@ -1973,19 +1973,19 @@ delta_repr(PyDateTime_Delta *self)
 {
 	if (GET_TD_MICROSECONDS(self) != 0)
 		return PyUnicode_FromFormat("%s(%d, %d, %d)",
-					   self->ob_type->tp_name,
-					   GET_TD_DAYS(self),
-					   GET_TD_SECONDS(self),
-					   GET_TD_MICROSECONDS(self));
+		                            self->ob_type->tp_name,
+		                            GET_TD_DAYS(self),
+		                            GET_TD_SECONDS(self),
+		                            GET_TD_MICROSECONDS(self));
 	if (GET_TD_SECONDS(self) != 0)
 		return PyUnicode_FromFormat("%s(%d, %d)",
-					   self->ob_type->tp_name,
-					   GET_TD_DAYS(self),
-					   GET_TD_SECONDS(self));
+		                            self->ob_type->tp_name,
+		                            GET_TD_DAYS(self),
+		                            GET_TD_SECONDS(self));
 
 	return PyUnicode_FromFormat("%s(%d)",
-				   self->ob_type->tp_name,
-				   GET_TD_DAYS(self));
+	                            self->ob_type->tp_name,
+	                            GET_TD_DAYS(self));
 }
 
 static PyObject *
@@ -2402,15 +2402,9 @@ date_subtract(PyObject *left, PyObject *right)
 static PyObject *
 date_repr(PyDateTime_Date *self)
 {
-	char buffer[1028];
-	const char *type_name;
-
-	type_name = self->ob_type->tp_name;
-	PyOS_snprintf(buffer, sizeof(buffer), "%s(%d, %d, %d)",
-		      type_name,
-		      GET_YEAR(self), GET_MONTH(self), GET_DAY(self));
-
-	return PyUnicode_FromString(buffer);
+	return PyUnicode_FromFormat("%s(%d, %d, %d)",
+	                            self->ob_type->tp_name,
+	                            GET_YEAR(self), GET_MONTH(self), GET_DAY(self));
 }
 
 static PyObject *
@@ -3114,7 +3108,6 @@ time_tzname(PyDateTime_Time *self, PyObject *unused) {
 static PyObject *
 time_repr(PyDateTime_Time *self)
 {
-	char buffer[100];
 	const char *type_name = self->ob_type->tp_name;
 	int h = TIME_GET_HOUR(self);
 	int m = TIME_GET_MINUTE(self);
@@ -3123,15 +3116,13 @@ time_repr(PyDateTime_Time *self)
 	PyObject *result = NULL;
 
 	if (us)
-		PyOS_snprintf(buffer, sizeof(buffer),
-			      "%s(%d, %d, %d, %d)", type_name, h, m, s, us);
+		result = PyUnicode_FromFormat("%s(%d, %d, %d, %d)",
+		                              type_name, h, m, s, us);
 	else if (s)
-		PyOS_snprintf(buffer, sizeof(buffer),
-			      "%s(%d, %d, %d)", type_name, h, m, s);
+		result = PyUnicode_FromFormat("%s(%d, %d, %d)",
+		                              type_name, h, m, s);
 	else
-		PyOS_snprintf(buffer, sizeof(buffer),
-			      "%s(%d, %d)", type_name, h, m);
-	result = PyUnicode_FromString(buffer);
+		result = PyUnicode_FromFormat("%s(%d, %d)", type_name, h, m);
 	if (result != NULL && HASTZINFO(self))
 		result = append_keyword_tzinfo(result, self->tzinfo);
 	return result;
@@ -4020,7 +4011,7 @@ datetime_repr(PyDateTime_DateTime *self)
 	PyObject *baserepr;
 
 	if (DATE_GET_MICROSECOND(self)) {
-		PyOS_snprintf(buffer, sizeof(buffer),
+		baserepr = PyUnicode_FromFormat(
 			      "%s(%d, %d, %d, %d, %d, %d, %d)",
 			      type_name,
 			      GET_YEAR(self), GET_MONTH(self), GET_DAY(self),
@@ -4029,7 +4020,7 @@ datetime_repr(PyDateTime_DateTime *self)
 			      DATE_GET_MICROSECOND(self));
 	}
 	else if (DATE_GET_SECOND(self)) {
-		PyOS_snprintf(buffer, sizeof(buffer),
+		baserepr = PyUnicode_FromFormat(
 			      "%s(%d, %d, %d, %d, %d, %d)",
 			      type_name,
 			      GET_YEAR(self), GET_MONTH(self), GET_DAY(self),
@@ -4037,13 +4028,12 @@ datetime_repr(PyDateTime_DateTime *self)
 			      DATE_GET_SECOND(self));
 	}
 	else {
-		PyOS_snprintf(buffer, sizeof(buffer),
+		baserepr = PyUnicode_FromFormat(
 			      "%s(%d, %d, %d, %d, %d)",
 			      type_name,
 			      GET_YEAR(self), GET_MONTH(self), GET_DAY(self),
 			      DATE_GET_HOUR(self), DATE_GET_MINUTE(self));
 	}
-	baserepr = PyUnicode_FromString(buffer);
 	if (baserepr == NULL || ! HASTZINFO(self))
 		return baserepr;
 	return append_keyword_tzinfo(baserepr, self->tzinfo);
diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c
index c3f198e847..2685ca4478 100644
--- a/Modules/itertoolsmodule.c
+++ b/Modules/itertoolsmodule.c
@@ -2389,20 +2389,10 @@ repeat_next(repeatobject *ro)
 static PyObject *
 repeat_repr(repeatobject *ro)
 {
-	PyObject *result, *objrepr;
-
-	objrepr = PyObject_Repr(ro->element);
-	if (objrepr == NULL)
-		return NULL;
-
 	if (ro->cnt == -1)
-		result = PyUnicode_FromFormat("repeat(%U)",
-			objrepr);
+		return PyUnicode_FromFormat("repeat(%R)", ro->element);
 	else
-		result = PyUnicode_FromFormat("repeat(%U, %zd)",
-			objrepr, ro->cnt);
-	Py_DECREF(objrepr);
-	return result;
+		return PyUnicode_FromFormat("repeat(%R, %zd)", ro->element, ro->cnt);
 }	
 
 static PyObject *
diff --git a/Objects/classobject.c b/Objects/classobject.c
index e4687a3695..b7711d56d7 100644
--- a/Objects/classobject.c
+++ b/Objects/classobject.c
@@ -261,11 +261,9 @@ method_repr(PyMethodObject *a)
 		result = PyUnicode_FromFormat("<unbound method %s.%s>",
 					     sklassname, sfuncname);
 	else {
-		result = PyUnicode_FromFormat("<bound method %s.%s of ",
-		                              sklassname, sfuncname);
-		/* XXX Shouldn't use repr() here! */
-		PyUnicode_AppendAndDel(&result, PyObject_Repr(self));
-		PyUnicode_AppendAndDel(&result, PyUnicode_FromString(">"));
+		/* XXX Shouldn't use repr()/%R here! */
+		result = PyUnicode_FromFormat("<bound method %s.%s of %R>",
+		                              sklassname, sfuncname, self);
 	}
 	Py_XDECREF(funcname);
 	Py_XDECREF(klassname);
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index f4c265a329..f7189e21a6 100644
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -98,27 +98,14 @@ BaseException_str(PyBaseExceptionObject *self)
 static PyObject *
 BaseException_repr(PyBaseExceptionObject *self)
 {
-    PyObject *repr_suffix;
-    PyObject *repr;
     char *name;
     char *dot;
 
-    repr_suffix = PyObject_Repr(self->args);
-    if (!repr_suffix)
-        return NULL;
-
     name = (char *)self->ob_type->tp_name;
     dot = strrchr(name, '.');
     if (dot != NULL) name = dot+1;
 
-    repr = PyUnicode_FromString(name);
-    if (!repr) {
-        Py_DECREF(repr_suffix);
-        return NULL;
-    }
-
-    PyUnicode_AppendAndDel(&repr, repr_suffix);
-    return repr;
+    return PyUnicode_FromFormat("%s%R", name, self->args);
 }
 
 /* Pickling support */
diff --git a/Objects/intobject.c b/Objects/intobject.c
index 6c9745aae2..ab64f79e09 100644
--- a/Objects/intobject.c
+++ b/Objects/intobject.c
@@ -431,9 +431,7 @@ int_print(PyIntObject *v, FILE *fp, int flags)
 static PyObject *
 int_repr(PyIntObject *v)
 {
-	char buf[64];
-	PyOS_snprintf(buf, sizeof(buf), "%ld", v->ob_ival);
-	return PyUnicode_FromString(buf);
+	return PyUnicode_FromFormat("%ld", v->ob_ival);
 }
 
 static int
diff --git a/Objects/setobject.c b/Objects/setobject.c
index 795efc5929..8d4291beb5 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -612,10 +612,8 @@ set_tp_print(PySetObject *so, FILE *fp, int flags)
 static PyObject *
 set_repr(PySetObject *so)
 {
-	PyObject *keys, *result=NULL, *listrepr;
-	int newsize;
+	PyObject *keys, *result=NULL;
 	Py_UNICODE *u;
-	const char *s;
 	int status = Py_ReprEnter((PyObject*)so);
 
 	if (status != 0) {
@@ -633,35 +631,32 @@ set_repr(PySetObject *so)
 	keys = PySequence_List((PyObject *)so);
 	if (keys == NULL)
 		goto done;
-	listrepr = PyObject_Repr(keys);
-	Py_DECREF(keys);
-	if (listrepr == NULL)
-		goto done;
 
-	newsize = PyUnicode_GET_SIZE(listrepr);
-	if (so->ob_type != &PySet_Type)
-		newsize += strlen(so->ob_type->tp_name)+2;
-	result = PyUnicode_FromUnicode(NULL, newsize);
-	if (result) {
-		u = PyUnicode_AS_UNICODE(result);
-		if (so->ob_type != &PySet_Type) {
-			for (s = so->ob_type->tp_name; *s;)
-				*u++ = *s++;
-			*u++ = '(';
-			Py_UNICODE_COPY(u, PyUnicode_AS_UNICODE(listrepr),
-									 PyUnicode_GET_SIZE(listrepr));
-			u += PyUnicode_GET_SIZE(listrepr);
-			*u++ = ')';
-		} else {
+	if (so->ob_type != &PySet_Type) {
+		result = PyUnicode_FromFormat("%s(%R)", so->ob_type->tp_name, keys);
+		Py_DECREF(keys);
+	}
+	else {
+		PyObject *listrepr = PyObject_Repr(keys);
+		Py_ssize_t newsize;
+		Py_DECREF(keys);
+		if (listrepr == NULL) {
+			Py_DECREF(keys);
+			goto done;
+		}
+		newsize = PyUnicode_GET_SIZE(listrepr);
+		result = PyUnicode_FromUnicode(NULL, newsize);
+		if (result) {
+			u = PyUnicode_AS_UNICODE(result);
 			*u++ = '{';
 			/* Omit the brackets from the listrepr */
 			Py_UNICODE_COPY(u, PyUnicode_AS_UNICODE(listrepr)+1,
-									 PyUnicode_GET_SIZE(listrepr)-2);
-			u += PyUnicode_GET_SIZE(listrepr)-2;
+			                   PyUnicode_GET_SIZE(listrepr)-2);
+			u += newsize-2;
 			*u++ = '}';
 		}
+		Py_DECREF(listrepr);
 	}
-	Py_DECREF(listrepr);
 done:
 	Py_ReprLeave((PyObject*)so);
 	return result;
diff --git a/Objects/sliceobject.c b/Objects/sliceobject.c
index d526a079db..a30edefb46 100644
--- a/Objects/sliceobject.c
+++ b/Objects/sliceobject.c
@@ -226,18 +226,7 @@ slice_dealloc(PySliceObject *r)
 static PyObject *
 slice_repr(PySliceObject *r)
 {
-	PyObject *s, *comma;
-
-	s = PyUnicode_FromString("slice(");
-	comma = PyUnicode_FromString(", ");
-	PyUnicode_AppendAndDel(&s, PyObject_Repr(r->start));
-	PyUnicode_Append(&s, comma);
-	PyUnicode_AppendAndDel(&s, PyObject_Repr(r->stop));
-	PyUnicode_Append(&s, comma);
-	PyUnicode_AppendAndDel(&s, PyObject_Repr(r->step));
-	PyUnicode_AppendAndDel(&s, PyUnicode_FromString(")"));
-	Py_DECREF(comma);
-	return s;
+	return PyUnicode_FromFormat("slice(%R, %R, %R)", r->start, r->stop, r->step);
 }
 
 static PyMemberDef slice_members[] = {
diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c
index b46093e5d4..f2c5afa125 100644
--- a/Objects/unicodeobject.c
+++ b/Objects/unicodeobject.c
@@ -484,6 +484,9 @@ PyObject *
 PyUnicode_FromFormatV(const char *format, va_list vargs)
 {
 	va_list count;
+	Py_ssize_t callcount = 0;
+	PyObject **callresults = NULL;
+	PyObject **callresult;
 	Py_ssize_t n = 0;
 	const char* f;
 	Py_UNICODE *s;
@@ -501,7 +504,23 @@ PyUnicode_FromFormatV(const char *format, va_list vargs)
 	count = vargs;
 #endif
 #endif
-	/* step 1: figure out how large a buffer we need */
+	/* step 1: count the number of %R format specifications
+	 * (we call PyObject_Repr() for these objects once during step 3
+	 * and put the result in an array) */
+	for (f = format; *f; f++) {
+		if (*f == '%' && *(f+1)=='R')
+			++callcount;
+	}
+	/* step 2: allocate memory for the results of PyObject_Repr() calls */
+	if (callcount) {
+		callresults = PyMem_Malloc(sizeof(PyObject *)*callcount);
+		if (!callresults) {
+			PyErr_NoMemory();
+			return NULL;
+		}
+		callresult = callresults;
+	}
+	/* step 3: figure out how large a buffer we need */
 	for (f = format; *f; f++) {
 		if (*f == '%') {
 			const char* p = f;
@@ -539,6 +558,19 @@ PyUnicode_FromFormatV(const char *format, va_list vargs)
 				n += PyUnicode_GET_SIZE(obj);
 				break;
 			}
+			case 'R':
+			{
+				PyObject *obj = va_arg(count, PyObject *);
+				PyObject *repr;
+				assert(obj);
+				repr = PyObject_Repr(obj);
+				if (!repr)
+					goto fail;
+				n += PyUnicode_GET_SIZE(repr);
+				/* Remember the repr and switch to the next slot */
+				*callresult++ = repr;
+				break;
+			}
 			case 'p':
 				(void) va_arg(count, int);
 				/* maximum 64-bit pointer representation:
@@ -562,14 +594,16 @@ PyUnicode_FromFormatV(const char *format, va_list vargs)
 			n++;
 	}
  expand:
-	/* step 2: fill the buffer */
+	/* step 4: fill the buffer */
 	/* Since we've analyzed how much space we need for the worst case,
-	   we don't have to resize the string. */
+	   we don't have to resize the string.
+	   There can be no errors beyond this point. */
 	string = PyUnicode_FromUnicode(NULL, n);
 	if (!string)
 		return NULL;
 
 	s = PyUnicode_AS_UNICODE(string);
+	callresult = callresults;
 
 	for (f = format; *f; f++) {
 		if (*f == '%') {
@@ -649,6 +683,21 @@ PyUnicode_FromFormatV(const char *format, va_list vargs)
 					*s++ = ucopy[upos++];
 				break;
 			}
+			case 'R':
+			{
+				/* unused, since we already have the result */
+				(void) va_arg(vargs, PyObject *);
+				Py_UNICODE *ucopy = PyUnicode_AS_UNICODE(*callresult);
+				Py_ssize_t usize = PyUnicode_GET_SIZE(*callresult);
+				Py_ssize_t upos;
+				for (upos = 0; upos<usize;)
+					*s++ = ucopy[upos++];
+				/* We're done with the repr() => forget it */
+				Py_DECREF(*callresult);
+				/* switch to next repr() result */
+				++callresult;
+				break;
+			}
 			case 'p':
 				sprintf(buffer, "%p", va_arg(vargs, void*));
 				/* %p is ill-defined:  ensure leading 0x. */
@@ -673,8 +722,20 @@ PyUnicode_FromFormatV(const char *format, va_list vargs)
 	}
 
  end:
+	if (callresults)
+		PyMem_Free(callresults);
 	_PyUnicode_Resize(&string, s - PyUnicode_AS_UNICODE(string));
 	return string;
+ fail:
+	if (callresults) {
+		PyObject **callresult2 = callresults;
+		while (callresult2 <= callresult) {
+			Py_DECREF(*callresult2);
+			++callresult2;
+		}
+		PyMem_Free(callresults);
+	}
+	return NULL;
 }
 
 #undef appendstring