Issue #5816: Simplify code for parsing and printing of complex numbers.

nans and infs are no longer given special treatment; as a result,
repr(complex(z)) recovers z for any complex number z.

4 files changed, 173 insertions, 236 deletions

diff --git a/Lib/test/test_complex.py b/Lib/test/test_complex.py
index ab3c1d1..836360a 100644
--- a/Lib/test/test_complex.py
+++ b/Lib/test/test_complex.py
@@ -2,7 +2,7 @@ import unittest, os
 from test import support
 
 from random import random
-from math import atan2
+from math import atan2, isnan, copysign
 
 INF = float("inf")
 NAN = float("nan")
@@ -37,6 +37,29 @@ class ComplexTest(unittest.TestCase):
         # check that relative difference < eps
         self.assert_(abs((x-y)/y) < eps)
 
+    def assertFloatsAreIdentical(self, x, y):
+        """assert that floats x and y are identical, in the sense that:
+        (1) both x and y are nans, or
+        (2) both x and y are infinities, with the same sign, or
+        (3) both x and y are zeros, with the same sign, or
+        (4) x and y are both finite and nonzero, and x == y
+
+        """
+        msg = 'floats {!r} and {!r} are not identical'
+
+        if isnan(x) or isnan(y):
+            if isnan(x) and isnan(y):
+                return
+        elif x == y:
+            if x != 0.0:
+                return
+            # both zero; check that signs match
+            elif copysign(1.0, x) == copysign(1.0, y):
+                return
+            else:
+                msg += ': zeros have different signs'
+        self.fail(msg.format(x, y))
+
     def assertClose(self, x, y, eps=1e-9):
         """Return true iff complexes x and y "are close\""""
         self.assertCloseAbs(x.real, y.real, eps)
@@ -202,6 +225,8 @@ class ComplexTest(unittest.TestCase):
         self.assertAlmostEqual(complex("+1"), +1)
         self.assertAlmostEqual(complex("(1+2j)"), 1+2j)
         self.assertAlmostEqual(complex("(1.3+2.2j)"), 1.3+2.2j)
+        self.assertAlmostEqual(complex("3.14+1J"), 3.14+1j)
+        self.assertAlmostEqual(complex(" ( +3.14-6J )"), 3.14-6j)
 
         class complex2(complex): pass
         self.assertAlmostEqual(complex(complex2(1+1j)), 1+1j)
@@ -229,8 +254,6 @@ class ComplexTest(unittest.TestCase):
         self.assertRaises(TypeError, complex, "1", "1")
         self.assertRaises(TypeError, complex, 1, "1")
 
-        self.assertEqual(complex("  3.14+J  "), 3.14+1j)
-
         # SF bug 543840:  complex(string) accepts strings with \0
         # Fixed in 2.3.
         self.assertRaises(ValueError, complex, '1+1j\0j')
@@ -254,6 +277,11 @@ class ComplexTest(unittest.TestCase):
         self.assertRaises(ValueError, complex, "(1+2j)123")
         self.assertRaises(ValueError, complex, "1"*500)
         self.assertRaises(ValueError, complex, "x")
+        self.assertRaises(ValueError, complex, "J")
+        self.assertRaises(ValueError, complex, "1j+2")
+        self.assertRaises(ValueError, complex, "1e1ej")
+        self.assertRaises(ValueError, complex, "1e++1ej")
+        self.assertRaises(ValueError, complex, ")1+2j(")
 
         class EvilExc(Exception):
             pass
@@ -318,17 +346,17 @@ class ComplexTest(unittest.TestCase):
         self.assertEqual(-6j,complex(repr(-6j)))
         self.assertEqual(6j,complex(repr(6j)))
 
-        self.assertEqual(repr(complex(1., INF)), "(1+inf*j)")
-        self.assertEqual(repr(complex(1., -INF)), "(1-inf*j)")
+        self.assertEqual(repr(complex(1., INF)), "(1+infj)")
+        self.assertEqual(repr(complex(1., -INF)), "(1-infj)")
         self.assertEqual(repr(complex(INF, 1)), "(inf+1j)")
-        self.assertEqual(repr(complex(-INF, INF)), "(-inf+inf*j)")
+        self.assertEqual(repr(complex(-INF, INF)), "(-inf+infj)")
         self.assertEqual(repr(complex(NAN, 1)), "(nan+1j)")
-        self.assertEqual(repr(complex(1, NAN)), "(1+nan*j)")
-        self.assertEqual(repr(complex(NAN, NAN)), "(nan+nan*j)")
+        self.assertEqual(repr(complex(1, NAN)), "(1+nanj)")
+        self.assertEqual(repr(complex(NAN, NAN)), "(nan+nanj)")
 
-        self.assertEqual(repr(complex(0, INF)), "inf*j")
-        self.assertEqual(repr(complex(0, -INF)), "-inf*j")
-        self.assertEqual(repr(complex(0, NAN)), "nan*j")
+        self.assertEqual(repr(complex(0, INF)), "infj")
+        self.assertEqual(repr(complex(0, -INF)), "-infj")
+        self.assertEqual(repr(complex(0, NAN)), "nanj")
 
     def test_neg(self):
         self.assertEqual(-(1+6j), -1-6j)
@@ -367,6 +395,21 @@ class ComplexTest(unittest.TestCase):
             self.assertEquals(atan2(z1.imag, -1.), atan2(0., -1.))
             self.assertEquals(atan2(z2.imag, -1.), atan2(-0., -1.))
 
+    @unittest.skipUnless(float.__getformat__("double").startswith("IEEE"),
+                         "test requires IEEE 754 doubles")
+    def test_repr_roundtrip(self):
+        # complex(repr(z)) should recover z exactly, even for complex numbers
+        # involving an infinity, nan, or negative zero
+        vals = [0.0, 1e-200, 0.0123, 3.1415, 1e50, INF, NAN]
+        vals += [-v for v in vals]
+        for x in vals:
+            for y in vals:
+                z = complex(x, y)
+                roundtrip = complex(repr(z))
+                self.assertFloatsAreIdentical(z.real, roundtrip.real)
+                self.assertFloatsAreIdentical(z.imag, roundtrip.imag)
+
+
 def test_main():
     support.run_unittest(ComplexTest)
 
diff --git a/Misc/NEWS b/Misc/NEWS
index c69672f0..f4ffbb7 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 3.1 beta 1?
 Core and Builtins
 -----------------
 
+- Issue #5816: complex(repr(z)) now recovers z exactly, even when
+  z involves nans, infs or negative zeros.
+
 - Issue #3166: Make long -> float (and int -> float) conversions
   correctly rounded.
 
diff --git a/Objects/complexobject.c b/Objects/complexobject.c
index 721db8f..27765ad 100644
--- a/Objects/complexobject.c
+++ b/Objects/complexobject.c
@@ -332,99 +332,64 @@ complex_dealloc(PyObject *op)
 static PyObject *
 complex_format(PyComplexObject *v, char format_code)
 {
-    PyObject *result = NULL;
-    Py_ssize_t len;
-
-    /* If these are non-NULL, they'll need to be freed. */
-    char *pre = NULL;
-    char *pim = NULL;
-    char *buf = NULL;
-
-    /* These do not need to be freed. They're either aliases for pim
-       and pre, or pointers to constants. */
-    char *re = NULL;
-    char *im = NULL;
-    char *lead = "";
-    char *tail = "";
-
-
-    if (v->cval.real == 0.) {
-        re = "";
-        if (!Py_IS_FINITE(v->cval.imag)) {
-            if (Py_IS_NAN(v->cval.imag))
-                im = "nan*";
-            else if (copysign(1, v->cval.imag) == 1)
-                im = "inf*";
-            else
-                im = "-inf*";
-        }
-        else {
-            pim = PyOS_double_to_string(v->cval.imag, format_code,
-                                        0, 0, NULL);
-            if (!pim) {
-                PyErr_NoMemory();
-                goto done;
-            }
-            im = pim;
-        }
-    } else {
-        /* Format imaginary part with sign, real part without */
-        if (!Py_IS_FINITE(v->cval.real)) {
-            if (Py_IS_NAN(v->cval.real))
-                re = "nan";
-            /* else if (copysign(1, v->cval.real) == 1) */
-            else if (v->cval.real > 0)
-                re = "inf";
-            else
-                re = "-inf";
-        }
-        else {
-            pre = PyOS_double_to_string(v->cval.real, format_code,
-                                        0, 0, NULL);
-            if (!pre) {
-                PyErr_NoMemory();
-                goto done;
-            }
-            re = pre;
-        }
-
-        if (!Py_IS_FINITE(v->cval.imag)) {
-            if (Py_IS_NAN(v->cval.imag))
-                im = "+nan*";
-            /* else if (copysign(1, v->cval.imag) == 1) */
-            else if (v->cval.imag > 0)
-                im = "+inf*";
-            else
-                im = "-inf*";
-        }
-        else {
-            pim = PyOS_double_to_string(v->cval.imag, format_code,
-                                        0, Py_DTSF_SIGN, NULL);
-            if (!pim) {
-                PyErr_NoMemory();
-                goto done;
-            }
-            im = pim;
-        }
-        lead = "(";
-        tail = ")";
-    }
-    /* Alloc the final buffer. Add one for the "j" in the format string, and
-       one for the trailing zero. */
-    len = strlen(lead) + strlen(re) + strlen(im) + strlen(tail) + 2;
-    buf = PyMem_Malloc(len);
-    if (!buf) {
-        PyErr_NoMemory();
-        goto done;
-    }
-    PyOS_snprintf(buf, len, "%s%s%sj%s", lead, re, im, tail);
-    result = PyUnicode_FromString(buf);
-done:
-    PyMem_Free(pim);
-    PyMem_Free(pre);
-    PyMem_Free(buf);
-
-    return result;
+	PyObject *result = NULL;
+	Py_ssize_t len;
+
+	/* If these are non-NULL, they'll need to be freed. */
+	char *pre = NULL;
+	char *im = NULL;
+	char *buf = NULL;
+
+	/* These do not need to be freed. re is either an alias
+	   for pre or a pointer to a constant.  lead and tail
+	   are pointers to constants. */
+	char *re = NULL;
+	char *lead = "";
+	char *tail = "";
+
+	if (v->cval.real == 0. && copysign(1.0, v->cval.real)==1.0) {
+		re = "";
+		im = PyOS_double_to_string(v->cval.imag, format_code,
+					   0, 0, NULL);
+		if (!im) {
+			PyErr_NoMemory();
+			goto done;
+		}
+	} else {
+		/* Format imaginary part with sign, real part without */
+		pre = PyOS_double_to_string(v->cval.real, format_code,
+					    0, 0, NULL);
+		if (!pre) {
+			PyErr_NoMemory();
+			goto done;
+		}
+		re = pre;
+
+		im = PyOS_double_to_string(v->cval.imag, format_code,
+					   0, Py_DTSF_SIGN, NULL);
+		if (!im) {
+			PyErr_NoMemory();
+			goto done;
+		}
+		lead = "(";
+		tail = ")";
+	}
+	/* Alloc the final buffer. Add one for the "j" in the format string,
+	   and one for the trailing zero. */
+	len = strlen(lead) + strlen(re) + strlen(im) + strlen(tail) + 2;
+	buf = PyMem_Malloc(len);
+	if (!buf) {
+		PyErr_NoMemory();
+		goto done;
+	}
+	PyOS_snprintf(buf, len, "%s%s%sj%s", lead, re, im, tail);
+	result = PyUnicode_FromString(buf);
+  done:
+	PyMem_Free(im);
+	PyMem_Free(pre);
+	PyMem_Free(buf);
+
+	return result;
 }
 
 static PyObject *
@@ -757,11 +722,7 @@ complex_subtype_from_string(PyTypeObject *type, PyObject *v)
 	const char *s, *start;
 	char *end;
 	double x=0.0, y=0.0, z;
-	int got_re=0, got_im=0, got_bracket=0, done=0;
-	int digit_or_dot;
-	int sw_error=0;
-	int sign;
-	char buffer[256]; /* For errors */
+	int got_bracket=0;
 	char s_buffer[256];
 	Py_ssize_t len;
 
@@ -785,16 +746,13 @@ complex_subtype_from_string(PyTypeObject *type, PyObject *v)
 		return NULL;
 	}
 
+	errno = 0;
+
 	/* position on first nonblank */
 	start = s;
 	while (*s && isspace(Py_CHARMASK(*s)))
 		s++;
-	if (s[0] == '\0') {
-		PyErr_SetString(PyExc_ValueError,
-				"complex() arg is an empty string");
-		return NULL;
-	}
-	if (s[0] == '(') {
+	if (*s == '(') {
 		/* Skip over possible bracket from repr(). */
 		got_bracket = 1;
 		s++;
@@ -802,120 +760,50 @@ complex_subtype_from_string(PyTypeObject *type, PyObject *v)
 			s++;
 	}
 
-	z = -1.0;
-	sign = 1;
-	do {
-
-		switch (*s) {
-
-		case '\0':
-			if (s-start != len) {
-				PyErr_SetString(
-					PyExc_ValueError,
-					"complex() arg contains a null byte");
-				return NULL;
-			}
-			if(!done) sw_error=1;
-			break;
-
-		case ')':
-			if (!got_bracket || !(got_re || got_im)) {
-				sw_error=1;
-				break;
-			}
-			got_bracket=0;
-			done=1;
-			s++;
-			while (*s && isspace(Py_CHARMASK(*s)))
-				s++;
-			if (*s) sw_error=1;
-			break;
-
-		case '-':
-			sign = -1;
-				/* Fallthrough */
-		case '+':
-			if (done)  sw_error=1;
-			s++;
-			if  (  *s=='\0'||*s=='+'||*s=='-'||*s==')'||
-			       isspace(Py_CHARMASK(*s))  )  sw_error=1;
-			break;
-
-		case 'J':
-		case 'j':
-			if (got_im || done) {
-				sw_error = 1;
-				break;
-			}
-			if  (z<0.0) {
-				y=sign;
-			}
-			else{
-				y=sign*z;
-			}
-			got_im=1;
-			s++;
-			if  (*s!='+' && *s!='-' )
-				done=1;
-			break;
-
-		default:
-			if (isspace(Py_CHARMASK(*s))) {
-				while (*s && isspace(Py_CHARMASK(*s)))
-					s++;
-				if (*s && *s != ')')
-					sw_error=1;
-				else
-					done = 1;
-				break;
-			}
-			digit_or_dot =
-				(*s=='.' || isdigit(Py_CHARMASK(*s)));
-			if  (done||!digit_or_dot) {
-				sw_error=1;
-				break;
-			}
-			errno = 0;
-			PyFPE_START_PROTECT("strtod", return 0)
-				z = PyOS_ascii_strtod(s, &end) ;
-			PyFPE_END_PROTECT(z)
-				if (errno != 0) {
-					PyOS_snprintf(buffer, sizeof(buffer),
-					  "float() out of range: %.150s", s);
-					PyErr_SetString(
-						PyExc_ValueError,
-						buffer);
-					return NULL;
-				}
-			s=end;
-			if  (*s=='J' || *s=='j') {
-
-				break;
-			}
-			if  (got_re) {
-				sw_error=1;
-				break;
-			}
-
-				/* accept a real part */
-			x=sign*z;
-			got_re=1;
-			if  (got_im)  done=1;
-			z = -1.0;
-			sign = 1;
-			break;
-
-		}  /* end of switch  */
-
-	} while (s - start < len && !sw_error);
-
-	if (sw_error || got_bracket) {
-		PyErr_SetString(PyExc_ValueError,
-				"complex() arg is a malformed string");
-		return NULL;
+	/* get float---might be real or imaginary part */
+	z = PyOS_ascii_strtod(s, &end);
+	if (end == s)
+		goto error;
+	s = end;
+	if (*s == '+' || *s == '-') {
+		/* we've got a real part *and* an imaginary part */
+		x = z;
+		y = PyOS_ascii_strtod(s, &end);
+		if (end == s || !(*end == 'j' || *end == 'J'))
+			goto error;
+		s = ++end;
+	}
+	else if (*s == 'j' || *s == 'J') {
+		/* no real part; z was the imaginary part */
+		s++;
+		y = z;
+	}
+	else
+		/* no imaginary part */
+		x = z;
+
+	/* trailing whitespace and closing bracket */
+	while (*s && isspace(Py_CHARMASK(*s)))
+		s++;
+	if (got_bracket && *s == ')') {
+		got_bracket = 0;
+		s++;
+		while (*s && isspace(Py_CHARMASK(*s)))
+		       s++;
 	}
+	/* we should now be at the end of the string */
+	if (s-start != len || got_bracket)
+		goto error;
 
 	return complex_subtype_from_doubles(type, x, y);
+
+  error:
+	/* check for PyOS_ascii_strtod failure due to lack of memory */
+	if (errno == ENOMEM)
+		return PyErr_NoMemory();
+	PyErr_SetString(PyExc_ValueError,
+			"complex() arg is a malformed string");
+	return NULL;
 }
 
 static PyObject *
diff --git a/Python/pystrtod.c b/Python/pystrtod.c
index 95fbd89..002714f 100644
--- a/Python/pystrtod.c
+++ b/Python/pystrtod.c
@@ -630,8 +630,9 @@ PyAPI_FUNC(char *) PyOS_double_to_string(double val,
 	}
 	p = result;
 
-	/* Never add sign for nan/inf, even if asked. */
-	if (flags & Py_DTSF_SIGN && buf[0] != '-' && t == Py_DTST_FINITE)
+	/* Add sign when requested.  It's convenient (esp. when formatting
+	 complex numbers) to include a sign even for inf and nan. */
+	if (flags & Py_DTSF_SIGN && buf[0] != '-')
 		*p++ = '+';
 
 	strcpy(p, buf);
@@ -733,6 +734,10 @@ format_float_short(double d, char format_code,
 		   so convert Infinity to inf and NaN to nan, and
 		   ignore sign of nan. Then return. */
 
+		/* ignore the actual sign of a nan */
+		if (digits[0] == 'n' || digits[0] == 'N')
+			sign = 0;
+
 		/* We only need 5 bytes to hold the result "+inf\0" . */
 		bufsize = 5; /* Used later in an assert. */
 		buf = (char *)PyMem_Malloc(bufsize);
@@ -742,13 +747,13 @@ format_float_short(double d, char format_code,
 		}
 		p = buf;
 
+		if (sign == 1) {
+			*p++ = '-';
+		}
+		else if (always_add_sign) {
+			*p++ = '+';
+		}
 		if (digits[0] == 'i' || digits[0] == 'I') {
-			if (sign == 1) {
-				*p++ = '-';
-			}
-			else if (always_add_sign) {
-				*p++ = '+';
-			}
 			strncpy(p, float_strings[OFS_INF], 3);
 			p += 3;
 
@@ -756,8 +761,6 @@ format_float_short(double d, char format_code,
 				*type = Py_DTST_INFINITE;
 		}
 		else if (digits[0] == 'n' || digits[0] == 'N') {
-			/* note that we *never* add a sign for a nan,
-			   even if one has explicitly been requested */
 			strncpy(p, float_strings[OFS_NAN], 3);
 			p += 3;