Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Lib/test/test_float.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,24 @@ class F(float, H):
value = F('nan')
self.assertEqual(hash(value), object.__hash__(value))

def test_issue_gh143006(self):
# When comparing negative non-integer float and int with the
# same number of bits in the integer part, __neg__() in the
# int subclass returning not an int caused an assertion error.
class EvilInt(int):
def __neg__(self):
return ""

i = -1 << 50
f = float(i) - 0.5
i = EvilInt(i)
self.assertFalse(f == i)
self.assertTrue(f != i)
self.assertTrue(f < i)
self.assertTrue(f <= i)
self.assertFalse(f > i)
self.assertFalse(f >= i)


@unittest.skipUnless(hasattr(float, "__getformat__"), "requires __getformat__")
class FormatFunctionsTestCase(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a possible assertion error when comparing negative non-integer ``float``
and ``int`` with the same number of bits in the integer part.
81 changes: 34 additions & 47 deletions Objects/floatobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,82 +435,69 @@ float_richcompare(PyObject *v, PyObject *w, int op)
assert(vsign != 0); /* if vsign were 0, then since wsign is
* not 0, we would have taken the
* vsign != wsign branch at the start */
/* We want to work with non-negative numbers. */
if (vsign < 0) {
/* "Multiply both sides" by -1; this also swaps the
* comparator.
*/
i = -i;
op = _Py_SwappedOp[op];
}
assert(i > 0.0);
(void) frexp(i, &exponent);
/* exponent is the # of bits in v before the radix point;
* we know that nbits (the # of bits in w) > 48 at this point
*/
if (exponent < nbits) {
i = 1.0;
j = 2.0;
j = i;
i = 0.0;
goto Compare;
}
if (exponent > nbits) {
i = 2.0;
j = 1.0;
j = 0.0;
goto Compare;
}
/* v and w have the same number of bits before the radix
* point. Construct two ints that have the same comparison
* outcome.
* point. Construct an int from the integer part of v and
* update op if necessary, so comparing two ints has the same outcome.
*/
{
double fracpart;
double intpart;
PyObject *result = NULL;
PyObject *vv = NULL;
PyObject *ww = w;

if (wsign < 0) {
ww = PyNumber_Negative(w);
if (ww == NULL)
goto Error;
fracpart = modf(i, &intpart);
if (fracpart != 0.0) {
switch (op) {
case Py_EQ:
Py_RETURN_FALSE;
case Py_NE:
Py_RETURN_TRUE;
case Py_LE:
if (vsign > 0) {
op = Py_LT; // v <= w <=> trunc(v) < w
}
break;
case Py_GE:
if (vsign < 0) {
op = Py_GT; // v >= w <=> trunc(v) > w
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, it might worth to do sign canonicalization, as before, to get rid of such cases. But I think we have to expose long_neg() for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will only remove 2 of 6 cases and simplify other 2. The code for negation of both arguments is more complex.

We could also use two tables, this would make the code smaller: op = table[vsign > 0][op].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was not just to reduce number of lines, but make algorithm more clear. Though, now I'm not sure we gain too much. I'm happy with current code.

break;
case Py_LT:
if (vsign < 0) {
op = Py_LE; // v < w <=> trunc(v) <= w
}
break;
case Py_GT:
if (vsign > 0) {
op = Py_GE; // v > w <=> trunc(v) >= w
}
break;
}
}
else
Py_INCREF(ww);

fracpart = modf(i, &intpart);
vv = PyLong_FromDouble(intpart);
if (vv == NULL)
goto Error;

if (fracpart != 0.0) {
/* Shift left, and or a 1 bit into vv
* to represent the lost fraction.
*/
PyObject *temp;

temp = _PyLong_Lshift(ww, 1);
if (temp == NULL)
goto Error;
Py_SETREF(ww, temp);

temp = _PyLong_Lshift(vv, 1);
if (temp == NULL)
goto Error;
Py_SETREF(vv, temp);

temp = PyNumber_Or(vv, _PyLong_GetOne());
if (temp == NULL)
goto Error;
Py_SETREF(vv, temp);
}

r = PyObject_RichCompareBool(vv, ww, op);
r = PyObject_RichCompareBool(vv, w, op);
if (r < 0)
goto Error;
result = PyBool_FromLong(r);
Error:
Py_XDECREF(vv);
Py_XDECREF(ww);
return result;
}
} /* else if (PyLong_Check(w)) */
Expand Down
Loading