Issue #5463: Remove deprecated float coercion from struct module, along

with the _PY_STRUCT_FLOAT_COERCE constant.  Simplify tests accordingly,
and reenable (now-fixed) broken tests.

Lib/test/test_struct.py

@@ -11,13 +11,6 @@ ISBIGENDIAN = sys.byteorder == "big"
 IS32BIT = sys.maxsize == 0x7fffffff
 del sys
 
-try:
-    import _struct
-except ImportError:
-    PY_STRUCT_FLOAT_COERCE = 2
-else:
-    PY_STRUCT_FLOAT_COERCE = getattr(_struct, '_PY_STRUCT_FLOAT_COERCE', 0)
-
 def string_reverse(s):
     return s[::-1]
 
@@ -27,40 +20,7 @@ def bigendian_to_native(value):
     else:
         return string_reverse(value)
 
-def with_warning_restore(func):
-    @wraps(func)
-    def decorator(*args, **kw):
-        with warnings.catch_warnings():
-            # We need this function to warn every time, so stick an
-            # unqualifed 'always' at the head of the filter list
-            warnings.simplefilter("always")
-            warnings.filterwarnings("error", category=DeprecationWarning)
-            return func(*args, **kw)
-    return decorator
-
 class StructTest(unittest.TestCase):
-
-    @with_warning_restore
-    def check_float_coerce(self, format, number):
-        # SF bug 1530559. struct.pack raises TypeError where it used to convert.
-        if PY_STRUCT_FLOAT_COERCE == 2:
-            # Test for pre-2.5 struct module
-            packed = struct.pack(format, number)
-            floored = struct.unpack(format, packed)[0]
-            self.assertEqual(floored, int(number),
-                             "did not correcly coerce float to int")
-            return
-        try:
-            struct.pack(format, number)
-        except (struct.error, TypeError):
-            if PY_STRUCT_FLOAT_COERCE:
-                self.fail("expected DeprecationWarning for float coerce")
-        except DeprecationWarning:
-            if not PY_STRUCT_FLOAT_COERCE:
-                self.fail("expected to raise struct.error for float coerce")
-        else:
-            self.fail("did not raise error for float coerce")
-
     def test_isbigendian(self):
         self.assertEqual((struct.pack('=i', 1)[0] == 0), ISBIGENDIAN)
 
@@ -270,10 +230,8 @@ class StructTest(unittest.TestCase):
 
                 else:
                     # x is out of range -- verify pack realizes that.
-                    self.assertRaises((struct.error, OverflowError),
-                                      pack, ">" + code, x)
-                    self.assertRaises((struct.error, OverflowError),
-                                      pack, "<" + code, x)
+                    self.assertRaises(struct.error, pack, ">" + code, x)
+                    self.assertRaises(struct.error, pack, "<" + code, x)
 
                 # Much the same for unsigned.
                 code = self.unsigned_code
@@ -317,10 +275,8 @@ class StructTest(unittest.TestCase):
 
                 else:
                     # x is out of range -- verify pack realizes that.
-                    self.assertRaises((struct.error, OverflowError),
-                                      pack, ">" + code, x)
-                    self.assertRaises((struct.error, OverflowError),
-                                      pack, "<" + code, x)
+                    self.assertRaises(struct.error, pack, ">" + code, x)
+                    self.assertRaises(struct.error, pack, "<" + code, x)
 
             def run(self):
                 from random import randrange
@@ -353,8 +309,8 @@ class StructTest(unittest.TestCase):
                 # Some error cases.
                 for direction in "<>":
                     for code in self.formatpair:
-                        for badobject in "a string", 3+42j, randrange:
-                            self.assertRaises((struct.error, TypeError),
+                        for badobject in "a string", 3+42j, randrange, -1729.0:
+                            self.assertRaises(struct.error,
                                               struct.pack, direction + code,
                                               badobject)
 
@@ -437,13 +393,14 @@ class StructTest(unittest.TestCase):
             self.assertRaises((struct.error, OverflowError), struct.pack,
                               endian + 'L', sys.maxsize * 4)
 
-    def XXXtest_1530559(self):
-        # XXX This is broken: see the bug report
-        # SF bug 1530559. struct.pack raises TypeError where it used to convert.
+    def test_1530559(self):
         for endian in ('', '>', '<'):
-            for fmt in ('B', 'H', 'I', 'L', 'b', 'h', 'i', 'l'):
-                self.check_float_coerce(endian + fmt, 1.0)
-                self.check_float_coerce(endian + fmt, 1.5)
+            for fmt in ('B', 'H', 'I', 'L', 'Q', 'b', 'h', 'i', 'l', 'q'):
+                self.assertRaises(struct.error, struct.pack, endian + fmt, 1.0)
+                self.assertRaises(struct.error, struct.pack, endian + fmt, 1.5)
+        self.assertRaises(struct.error, struct.pack, 'P', 1.0)
+        self.assertRaises(struct.error, struct.pack, 'P', 1.5)
+
 
     def test_unpack_from(self):
         test_string = b'abcd01234'

Misc/NEWS

@@ -87,6 +87,11 @@ Library
 Extension Modules
 -----------------
 
+- Issue #5463: In struct module, remove deprecated float coercion
+  for integer type codes: struct.pack('L', 0.3) should now raise
+  an error.  The _PY_STRUCT_FLOAT_COERCE constant has been removed.
+  The version number has been bumped to 0.3.
+
 - Issue #5359: Readd the Berkley-DB detection code to allow _dbm be built
   using Berkley-DB.
 

Modules/_struct.c

@@ -12,17 +12,6 @@
 
 static PyTypeObject PyStructType;
 
-/* If PY_STRUCT_FLOAT_COERCE is defined, the struct module will allow float
-   arguments for integer formats with a warning for backwards
-   compatibility. */
-
-#define PY_STRUCT_FLOAT_COERCE 1
-
-#ifdef PY_STRUCT_FLOAT_COERCE
-#define FLOAT_COERCE "integer argument expected, got float"
-#endif
-
-
 /* The translation function for each format character is table driven */
 typedef struct _formatdef {
 	char format;
@@ -100,58 +89,40 @@ typedef struct { char c; _Bool x; } s_bool;
 #pragma options align=reset
 #endif
 
-/* Helper to get a PyLongObject by hook or by crook.  Caller should decref. */
+/* Helper to get a PyLongObject.  Caller should decref. */
 
 static PyObject *
 get_pylong(PyObject *v)
 {
-	PyNumberMethods *m;
-
 	assert(v != NULL);
-	if (PyLong_Check(v)) {
-		Py_INCREF(v);
-		return v;
-	}
-	m = Py_TYPE(v)->tp_as_number;
-	if (m != NULL && m->nb_int != NULL) {
-		v = m->nb_int(v);
-		if (v == NULL)
-			return NULL;
-		if (PyLong_Check(v))
-			return v;
-		Py_DECREF(v);
-	}
+	if (!PyLong_Check(v)) {
 		PyErr_SetString(StructError,
-			"cannot convert argument to long");
+				"required argument is not an integer");
 		return NULL;
+	}
+
+	Py_INCREF(v);
+	return v;
 }
 
-/* Helper routine to get a Python integer and raise the appropriate error
-   if it isn't one */
+/* Helper routine to get a C long and raise the appropriate error if it isn't
+   one */
 
 static int
 get_long(PyObject *v, long *p)
 {
-	long x = PyLong_AsLong(v);
-	if (x == -1 && PyErr_Occurred()) {
-#ifdef PY_STRUCT_FLOAT_COERCE
-		if (PyFloat_Check(v)) {
-			PyObject *o;
-			int res;
-			PyErr_Clear();
-			if (PyErr_WarnEx(PyExc_DeprecationWarning, FLOAT_COERCE, 2) < 0)
-				return -1;
-			o = PyNumber_Long(v);
-			if (o == NULL)
+	long x;
+
+	if (!PyLong_Check(v)) {
+		PyErr_SetString(StructError,
+				"required argument is not an integer");
 		return -1;
-			res = get_long(o, p);
-			Py_DECREF(o);
-			return res;
 	}
-#endif
-		if (PyErr_ExceptionMatches(PyExc_TypeError))
+	x = PyLong_AsLong(v);
+	if (x == -1 && PyErr_Occurred()) {
+		if (PyErr_ExceptionMatches(PyExc_OverflowError))
 			PyErr_SetString(StructError,
-					"required argument is not an integer");
+					"argument out of range");
 		return -1;
 	}
 	*p = x;
@@ -164,20 +135,21 @@ get_long(PyObject *v, long *p)
 static int
 get_ulong(PyObject *v, unsigned long *p)
 {
-	if (PyLong_Check(v)) {
-		unsigned long x = PyLong_AsUnsignedLong(v);
-		if (x == (unsigned long)(-1) && PyErr_Occurred())
+	unsigned long x;
+
+	if (!PyLong_Check(v)) {
+		PyErr_SetString(StructError,
+				"required argument is not an integer");
 		return -1;
-		*p = x;
-		return 0;
 	}
-	if (get_long(v, (long *)p) < 0)
-		return -1;
-	if (((long)*p) < 0) {
+	x = PyLong_AsUnsignedLong(v);
+	if (x == (unsigned long)-1 && PyErr_Occurred()) {
+		if (PyErr_ExceptionMatches(PyExc_OverflowError))
 			PyErr_SetString(StructError,
-				"unsigned argument is < 0");
+					"argument out of range");
 		return -1;
 	}
+	*p = x;
 	return 0;
 }
 
@@ -189,15 +161,18 @@ static int
 get_longlong(PyObject *v, PY_LONG_LONG *p)
 {
 	PY_LONG_LONG x;
-
-	v = get_pylong(v);
-	if (v == NULL)
+	if (!PyLong_Check(v)) {
+		PyErr_SetString(StructError,
+				"required argument is not an integer");
 		return -1;
-	assert(PyLong_Check(v));
+	}
 	x = PyLong_AsLongLong(v);
-	Py_DECREF(v);
-	if (x == (PY_LONG_LONG)-1 && PyErr_Occurred())
+	if (x == -1 && PyErr_Occurred()) {
+		if (PyErr_ExceptionMatches(PyExc_OverflowError))
+			PyErr_SetString(StructError,
+					"argument out of range");
 		return -1;
+	}
 	*p = x;
 	return 0;
 }
@@ -208,15 +183,18 @@ static int
 get_ulonglong(PyObject *v, unsigned PY_LONG_LONG *p)
 {
 	unsigned PY_LONG_LONG x;
-
-	v = get_pylong(v);
-	if (v == NULL)
+	if (!PyLong_Check(v)) {
+		PyErr_SetString(StructError,
+				"required argument is not an integer");
 		return -1;
-	assert(PyLong_Check(v));
+	}
 	x = PyLong_AsUnsignedLongLong(v);
-	Py_DECREF(v);
-	if (x == (unsigned PY_LONG_LONG)-1 && PyErr_Occurred())
+	if (x == -1 && PyErr_Occurred()) {
+		if (PyErr_ExceptionMatches(PyExc_OverflowError))
+			PyErr_SetString(StructError,
+					"argument out of range");
 		return -1;
+	}
 	*p = x;
 	return 0;
 }
@@ -1962,7 +1940,7 @@ PyInit__struct(void)
 {
 	PyObject *ver, *m;
 
-	ver = PyBytes_FromString("0.2");
+	ver = PyBytes_FromString("0.3");
 	if (ver == NULL)
 		return NULL;
 
@@ -2028,9 +2006,5 @@ PyInit__struct(void)
 
 	PyModule_AddObject(m, "__version__", ver);
 
-#ifdef PY_STRUCT_FLOAT_COERCE
-	PyModule_AddIntConstant(m, "_PY_STRUCT_FLOAT_COERCE", 1);
-#endif
 	return m;
-
 }