Merge pull request #577 from kmod/tp_richcompare

tp_richcompare

libunwind_patches/0003-use-a-sorted-array-for-registered-objects-and-do-a-b.patch

@@ -119,7 +119,7 @@ diff --git a/src/mi/dyn-register.c b/src/mi/dyn-register.c
 index c28954a..c4f88b1 100644
 --- a/src/mi/dyn-register.c
 +++ b/src/mi/dyn-register.c
-@@ -32,13 +32,28 @@ _U_dyn_register (unw_dyn_info_t *di)
+@@ -32,13 +32,27 @@ _U_dyn_register (unw_dyn_info_t *di)
  {
    mutex_lock (&_U_dyn_info_list_lock);
    {
@@ -148,8 +148,7 @@ index c28954a..c4f88b1 100644
 +          break;
 +      }
 +
-+      if (_U_dyn_info_list_size > 1)
-+          memmove(&_U_dyn_info_list[i+1], &_U_dyn_info_list[i], (_U_dyn_info_list_size - i) * sizeof(unw_dyn_info_t*));
++      memmove(&_U_dyn_info_list[i+1], &_U_dyn_info_list[i], (_U_dyn_info_list_size - i) * sizeof(unw_dyn_info_t*));
 +      _U_dyn_info_list[i] = di;
 +      _U_dyn_info_list_size ++;
    }

src/capi/object.cpp

@@ -764,7 +764,7 @@ static int try_3way_compare(PyObject* v, PyObject* w) {
     0 if v == w;
     1 if v >  w.
 */
-static int default_3way_compare(PyObject* v, PyObject* w) {
+/* Pyston change: static*/ int default_3way_compare(PyObject* v, PyObject* w) {
     int c;
     const char* vname, *wname;
 
@@ -865,7 +865,7 @@ extern "C" int PyObject_Compare(PyObject* v, PyObject* w) noexcept {
 }
 
 /* Return (new reference to) Py_True or Py_False. */
-static PyObject* convert_3way_to_object(int op, int c) noexcept {
+/* Pyston change: static */ PyObject* convert_3way_to_object(int op, int c) noexcept {
     PyObject* result;
     switch (op) {
         case Py_LT:

src/capi/typeobject.cpp

@@ -761,7 +761,15 @@ static PyObject* half_richcompare(PyObject* self, PyObject* other, int op) noexc
     return res;
 }
 
-static PyObject* slot_tp_richcompare(PyObject* self, PyObject* other, int op) noexcept {
+/* Pyston change: static*/ PyObject* slot_tp_richcompare(PyObject* self, PyObject* other, int op) noexcept {
+    static StatCounter slowpath_richcompare("slowpath_richcompare");
+    slowpath_richcompare.log();
+#if 0
+    std::string per_name_stat_name = "slowpath_richcompare." + std::string(self->cls->tp_name);
+    int id = Stats::getStatId(per_name_stat_name);
+    Stats::log(id);
+#endif
+
     PyObject* res;
 
     if (Py_TYPE(self)->tp_richcompare == slot_tp_richcompare) {

src/capi/typeobject.h

@@ -33,6 +33,8 @@ void commonClassSetup(BoxedClass* cls);
 PyTypeObject* best_base(PyObject* bases) noexcept;
 PyObject* mro_external(PyObject* self) noexcept;
 int type_set_bases(PyTypeObject* type, PyObject* value, void* context) noexcept;
+
+PyObject* slot_tp_richcompare(PyObject* self, PyObject* other, int op) noexcept;
 }
 
 #endif

src/capi/types.h

@@ -237,6 +237,9 @@ public:
     }
 };
 
+PyObject* convert_3way_to_object(int op, int c) noexcept;
+int default_3way_compare(PyObject* v, PyObject* w);
+
 } // namespace pyston
 
 #endif

src/runtime/builtin_modules/builtins.cpp

@@ -162,10 +162,11 @@ extern "C" Box* min(Box* arg0, BoxedTuple* args) {
         if (!minElement) {
             minElement = e;
         } else {
-            Box* comp_result = compareInternal(minElement, e, AST_TYPE::Gt, NULL);
-            if (nonzero(comp_result)) {
+            int r = PyObject_RichCompareBool(minElement, e, Py_GT);
+            if (r == -1)
+                throwCAPIException();
+            if (r)
                 minElement = e;
-            }
         }
     }
 
@@ -192,10 +193,11 @@ extern "C" Box* max(Box* arg0, BoxedTuple* args) {
         if (!maxElement) {
             maxElement = e;
         } else {
-            Box* comp_result = compareInternal(maxElement, e, AST_TYPE::Lt, NULL);
-            if (nonzero(comp_result)) {
+            int r = PyObject_RichCompareBool(maxElement, e, Py_LT);
+            if (r == -1)
+                throwCAPIException();
+            if (r)
                 maxElement = e;
-            }
         }
     }
 

src/runtime/capi.cpp

@@ -586,8 +586,11 @@ extern "C" long _Py_HashPointer(void* p) noexcept {
 }
 
 extern "C" int PyObject_IsTrue(PyObject* o) noexcept {
+    if (o->cls == bool_cls)
+        return o == True;
+
     try {
-        return nonzero(o);
+        return o->nonzeroIC();
     } catch (ExcInfo e) {
         fatalOrError(PyExc_NotImplementedError, "unimplemented");
         return -1;
@@ -1647,6 +1650,13 @@ static Box* methodGetDoc(Box* b, void*) {
     return None;
 }
 
+static Box* wrapperdescrGetDoc(Box* b, void*) {
+    assert(b->cls == wrapperdescr_cls);
+    auto s = static_cast<BoxedWrapperDescriptor*>(b)->wrapper->doc;
+    assert(s.size());
+    return boxString(s);
+}
+
 /* extension modules might be compiled with GC support so these
    functions must always be available */
 
@@ -1735,6 +1745,8 @@ void setupCAPI() {
                                new BoxedFunction(boxRTFunction((void*)BoxedWrapperDescriptor::__get__, UNKNOWN, 3)));
     wrapperdescr_cls->giveAttr("__call__", new BoxedFunction(boxRTFunction((void*)BoxedWrapperDescriptor::__call__,
                                                                            UNKNOWN, 2, 0, true, true)));
+    wrapperdescr_cls->giveAttr("__doc__",
+                               new (pyston_getset_cls) BoxedGetsetDescriptor(wrapperdescrGetDoc, NULL, NULL));
     wrapperdescr_cls->freeze();
 
     wrapperobject_cls->giveAttr(

src/runtime/int.cpp

@@ -566,109 +566,6 @@ extern "C" Box* intTruediv(BoxedInt* lhs, Box* rhs) {
     }
 }
 
-extern "C" Box* intEqInt(BoxedInt* lhs, BoxedInt* rhs) {
-    assert(isSubclass(lhs->cls, int_cls));
-    assert(isSubclass(rhs->cls, int_cls));
-    return boxBool(lhs->n == rhs->n);
-}
-
-extern "C" Box* intEq(BoxedInt* lhs, Box* rhs) {
-    if (!isSubclass(lhs->cls, int_cls))
-        raiseExcHelper(TypeError, "descriptor '__eq__' requires a 'int' object but received a '%s'", getTypeName(lhs));
-
-    if (isSubclass(rhs->cls, int_cls)) {
-        BoxedInt* rhs_int = static_cast<BoxedInt*>(rhs);
-        return boxBool(lhs->n == rhs_int->n);
-    } else {
-        return NotImplemented;
-    }
-}
-
-extern "C" Box* intNeInt(BoxedInt* lhs, BoxedInt* rhs) {
-    assert(isSubclass(lhs->cls, int_cls));
-    assert(isSubclass(rhs->cls, int_cls));
-    return boxBool(lhs->n != rhs->n);
-}
-
-extern "C" Box* intNe(BoxedInt* lhs, Box* rhs) {
-    if (!isSubclass(lhs->cls, int_cls))
-        raiseExcHelper(TypeError, "descriptor '__ne__' requires a 'int' object but received a '%s'", getTypeName(lhs));
-
-    if (!isSubclass(rhs->cls, int_cls)) {
-        return NotImplemented;
-    }
-    BoxedInt* rhs_int = static_cast<BoxedInt*>(rhs);
-    return boxBool(lhs->n != rhs_int->n);
-}
-
-extern "C" Box* intLtInt(BoxedInt* lhs, BoxedInt* rhs) {
-    assert(isSubclass(lhs->cls, int_cls));
-    assert(isSubclass(rhs->cls, int_cls));
-    return boxBool(lhs->n < rhs->n);
-}
-
-extern "C" Box* intLt(BoxedInt* lhs, Box* rhs) {
-    if (!isSubclass(lhs->cls, int_cls))
-        raiseExcHelper(TypeError, "descriptor '__lt__' requires a 'int' object but received a '%s'", getTypeName(lhs));
-
-    if (!isSubclass(rhs->cls, int_cls)) {
-        return NotImplemented;
-    }
-    BoxedInt* rhs_int = static_cast<BoxedInt*>(rhs);
-    return boxBool(lhs->n < rhs_int->n);
-}
-
-extern "C" Box* intLeInt(BoxedInt* lhs, BoxedInt* rhs) {
-    assert(isSubclass(lhs->cls, int_cls));
-    assert(isSubclass(rhs->cls, int_cls));
-    return boxBool(lhs->n <= rhs->n);
-}
-
-extern "C" Box* intLe(BoxedInt* lhs, Box* rhs) {
-    if (!isSubclass(lhs->cls, int_cls))
-        raiseExcHelper(TypeError, "descriptor '__le__' requires a 'int' object but received a '%s'", getTypeName(lhs));
-
-    if (!isSubclass(rhs->cls, int_cls)) {
-        return NotImplemented;
-    }
-    BoxedInt* rhs_int = static_cast<BoxedInt*>(rhs);
-    return boxBool(lhs->n <= rhs_int->n);
-}
-
-extern "C" Box* intGtInt(BoxedInt* lhs, BoxedInt* rhs) {
-    assert(isSubclass(lhs->cls, int_cls));
-    assert(isSubclass(rhs->cls, int_cls));
-    return boxBool(lhs->n > rhs->n);
-}
-
-extern "C" Box* intGt(BoxedInt* lhs, Box* rhs) {
-    if (!isSubclass(lhs->cls, int_cls))
-        raiseExcHelper(TypeError, "descriptor '__gt__' requires a 'int' object but received a '%s'", getTypeName(lhs));
-
-    if (!isSubclass(rhs->cls, int_cls)) {
-        return NotImplemented;
-    }
-    BoxedInt* rhs_int = static_cast<BoxedInt*>(rhs);
-    return boxBool(lhs->n > rhs_int->n);
-}
-
-extern "C" Box* intGeInt(BoxedInt* lhs, BoxedInt* rhs) {
-    assert(isSubclass(lhs->cls, int_cls));
-    assert(isSubclass(rhs->cls, int_cls));
-    return boxBool(lhs->n >= rhs->n);
-}
-
-extern "C" Box* intGe(BoxedInt* lhs, Box* rhs) {
-    if (!isSubclass(lhs->cls, int_cls))
-        raiseExcHelper(TypeError, "descriptor '__ge__' requires a 'int' object but received a '%s'", getTypeName(lhs));
-
-    if (!isSubclass(rhs->cls, int_cls)) {
-        return NotImplemented;
-    }
-    BoxedInt* rhs_int = static_cast<BoxedInt*>(rhs);
-    return boxBool(lhs->n >= rhs_int->n);
-}
-
 extern "C" Box* intLShiftInt(BoxedInt* lhs, BoxedInt* rhs) {
     assert(isSubclass(lhs->cls, int_cls));
     assert(isSubclass(rhs->cls, int_cls));
@@ -1119,6 +1016,33 @@ static int64_t int_hash(BoxedInt* o) noexcept {
     return n;
 }
 
+static PyObject* int_richcompare(PyObject* v, PyObject* w, int op) noexcept {
+    if (!PyInt_Check(v) || !PyInt_Check(w)) {
+        Py_INCREF(Py_NotImplemented);
+        return Py_NotImplemented;
+    }
+
+    int64_t lhs = static_cast<BoxedInt*>(v)->n;
+    int64_t rhs = static_cast<BoxedInt*>(w)->n;
+
+    switch (op) {
+        case Py_EQ:
+            return boxBool(lhs == rhs);
+        case Py_NE:
+            return boxBool(lhs != rhs);
+        case Py_LT:
+            return boxBool(lhs < rhs);
+        case Py_LE:
+            return boxBool(lhs <= rhs);
+        case Py_GT:
+            return boxBool(lhs > rhs);
+        case Py_GE:
+            return boxBool(lhs >= rhs);
+        default:
+            RELEASE_ASSERT(0, "%d", op);
+    }
+}
+
 void setupInt() {
     for (int i = 0; i < NUM_INTERNED_INTS; i++) {
         interned_ints[i] = new BoxedInt(i);
@@ -1138,12 +1062,8 @@ void setupInt() {
     int_cls->giveAttr("__pow__",
                       new BoxedFunction(boxRTFunction((void*)intPow, UNKNOWN, 3, 1, false, false), { None }));
 
-    _addFuncIntUnknown("__eq__", BOXED_BOOL, (void*)intEqInt, (void*)intEq);
-    _addFuncIntUnknown("__ne__", BOXED_BOOL, (void*)intNeInt, (void*)intNe);
-    _addFuncIntUnknown("__lt__", BOXED_BOOL, (void*)intLtInt, (void*)intLt);
-    _addFuncIntUnknown("__le__", BOXED_BOOL, (void*)intLeInt, (void*)intLe);
-    _addFuncIntUnknown("__gt__", BOXED_BOOL, (void*)intGtInt, (void*)intGt);
-    _addFuncIntUnknown("__ge__", BOXED_BOOL, (void*)intGeInt, (void*)intGe);
+    // Note: CPython implements int comparisons using tp_compare
+    int_cls->tp_richcompare = int_richcompare;
 
     _addFuncIntUnknown("__lshift__", UNKNOWN, (void*)intLShiftInt, (void*)intLShift);
     _addFuncIntUnknown("__rshift__", UNKNOWN, (void*)intRShiftInt, (void*)intRShift);

src/runtime/long.cpp

@@ -20,6 +20,8 @@
 
 #include "llvm/Support/raw_ostream.h"
 
+#include "capi/typeobject.h"
+#include "capi/types.h"
 #include "core/common.h"
 #include "core/options.h"
 #include "core/stats.h"
@@ -846,104 +848,18 @@ extern "C" Box* longXor(BoxedLong* v1, Box* _v2) {
     return NotImplemented;
 }
 
-// TODO reduce duplication between these 6 functions, and add double support
-Box* longGt(BoxedLong* v1, Box* _v2) {
-    if (!isSubclass(v1->cls, long_cls))
-        raiseExcHelper(TypeError, "descriptor '__gt__' requires a 'long' object but received a '%s'", getTypeName(v1));
-
-    if (isSubclass(_v2->cls, long_cls)) {
-        BoxedLong* v2 = static_cast<BoxedLong*>(_v2);
-
-        return boxBool(mpz_cmp(v1->n, v2->n) > 0);
-    } else if (isSubclass(_v2->cls, int_cls)) {
-        BoxedInt* v2 = static_cast<BoxedInt*>(_v2);
-
-        return boxBool(mpz_cmp_si(v1->n, v2->n) > 0);
-    } else {
-        return NotImplemented;
-    }
-}
-
-Box* longGe(BoxedLong* v1, Box* _v2) {
-    if (!isSubclass(v1->cls, long_cls))
-        raiseExcHelper(TypeError, "descriptor '__ge__' requires a 'long' object but received a '%s'", getTypeName(v1));
-
-    if (isSubclass(_v2->cls, long_cls)) {
-        BoxedLong* v2 = static_cast<BoxedLong*>(_v2);
-
-        return boxBool(mpz_cmp(v1->n, v2->n) >= 0);
-    } else if (isSubclass(_v2->cls, int_cls)) {
-        BoxedInt* v2 = static_cast<BoxedInt*>(_v2);
-
-        return boxBool(mpz_cmp_si(v1->n, v2->n) >= 0);
-    } else {
-        return NotImplemented;
-    }
-}
-
-Box* longLt(BoxedLong* v1, Box* _v2) {
-    if (!isSubclass(v1->cls, long_cls))
-        raiseExcHelper(TypeError, "descriptor '__lt__' requires a 'long' object but received a '%s'", getTypeName(v1));
-
-    if (isSubclass(_v2->cls, long_cls)) {
-        BoxedLong* v2 = static_cast<BoxedLong*>(_v2);
-
-        return boxBool(mpz_cmp(v1->n, v2->n) < 0);
-    } else if (isSubclass(_v2->cls, int_cls)) {
-        BoxedInt* v2 = static_cast<BoxedInt*>(_v2);
-
-        return boxBool(mpz_cmp_si(v1->n, v2->n) < 0);
-    } else {
-        return NotImplemented;
-    }
-}
-
-Box* longLe(BoxedLong* v1, Box* _v2) {
-    if (!isSubclass(v1->cls, long_cls))
-        raiseExcHelper(TypeError, "descriptor '__le__' requires a 'long' object but received a '%s'", getTypeName(v1));
-
-    if (isSubclass(_v2->cls, long_cls)) {
-        BoxedLong* v2 = static_cast<BoxedLong*>(_v2);
-
-        return boxBool(mpz_cmp(v1->n, v2->n) <= 0);
-    } else if (isSubclass(_v2->cls, int_cls)) {
-        BoxedInt* v2 = static_cast<BoxedInt*>(_v2);
-
-        return boxBool(mpz_cmp_si(v1->n, v2->n) <= 0);
-    } else {
-        return NotImplemented;
-    }
-}
-
-Box* longEq(BoxedLong* v1, Box* _v2) {
-    if (!isSubclass(v1->cls, long_cls))
-        raiseExcHelper(TypeError, "descriptor '__eq__' requires a 'long' object but received a '%s'", getTypeName(v1));
-
-    if (isSubclass(_v2->cls, long_cls)) {
-        BoxedLong* v2 = static_cast<BoxedLong*>(_v2);
-
-        return boxBool(mpz_cmp(v1->n, v2->n) == 0);
-    } else if (isSubclass(_v2->cls, int_cls)) {
-        BoxedInt* v2 = static_cast<BoxedInt*>(_v2);
-
-        return boxBool(mpz_cmp_si(v1->n, v2->n) == 0);
-    } else {
-        return NotImplemented;
-    }
-}
-
-Box* longNe(BoxedLong* v1, Box* _v2) {
-    if (!isSubclass(v1->cls, long_cls))
-        raiseExcHelper(TypeError, "descriptor '__ne__' requires a 'long' object but received a '%s'", getTypeName(v1));
+static PyObject* long_richcompare(Box* _v1, Box* _v2, int op) noexcept {
+    RELEASE_ASSERT(isSubclass(_v1->cls, long_cls), "");
+    BoxedLong* v1 = static_cast<BoxedLong*>(_v1);
 
     if (isSubclass(_v2->cls, long_cls)) {
         BoxedLong* v2 = static_cast<BoxedLong*>(_v2);
 
-        return boxBool(mpz_cmp(v1->n, v2->n) != 0);
+        return convert_3way_to_object(op, mpz_cmp(v1->n, v2->n));
     } else if (isSubclass(_v2->cls, int_cls)) {
         BoxedInt* v2 = static_cast<BoxedInt*>(_v2);
 
-        return boxBool(mpz_cmp_si(v1->n, v2->n) != 0);
+        return convert_3way_to_object(op, mpz_cmp_si(v1->n, v2->n));
     } else {
         return NotImplemented;
     }
@@ -1457,12 +1373,8 @@ void setupLong() {
     long_cls->giveAttr("__xor__", new BoxedFunction(boxRTFunction((void*)longXor, UNKNOWN, 2)));
     long_cls->giveAttr("__rxor__", long_cls->getattr("__xor__"));
 
-    long_cls->giveAttr("__gt__", new BoxedFunction(boxRTFunction((void*)longGt, UNKNOWN, 2)));
-    long_cls->giveAttr("__ge__", new BoxedFunction(boxRTFunction((void*)longGe, UNKNOWN, 2)));
-    long_cls->giveAttr("__lt__", new BoxedFunction(boxRTFunction((void*)longLt, UNKNOWN, 2)));
-    long_cls->giveAttr("__le__", new BoxedFunction(boxRTFunction((void*)longLe, UNKNOWN, 2)));
-    long_cls->giveAttr("__eq__", new BoxedFunction(boxRTFunction((void*)longEq, UNKNOWN, 2)));
-    long_cls->giveAttr("__ne__", new BoxedFunction(boxRTFunction((void*)longNe, UNKNOWN, 2)));
+    // Note: CPython implements long comparisons using tp_compare
+    long_cls->tp_richcompare = long_richcompare;
 
     long_cls->giveAttr("__lshift__", new BoxedFunction(boxRTFunction((void*)longLshift, UNKNOWN, 2)));
     long_cls->giveAttr("__rshift__", new BoxedFunction(boxRTFunction((void*)longRshift, UNKNOWN, 2)));
@@ -1488,6 +1400,7 @@ void setupLong() {
     long_cls->giveAttr("numerator", new (pyston_getset_cls) BoxedGetsetDescriptor(longLong, NULL, NULL));
     long_cls->giveAttr("denominator", new (pyston_getset_cls) BoxedGetsetDescriptor(long1, NULL, NULL));
 
+    add_operators(long_cls);
     long_cls->freeze();
 
     long_cls->tp_as_number->nb_power = long_pow;

src/runtime/objmodel.cpp

@@ -137,26 +137,19 @@ size_t PyHasher::operator()(Box* b) const {
 bool PyEq::operator()(Box* lhs, Box* rhs) const {
     STAT_TIMER(t0, "us_timer_PyEq");
 
-    if (lhs == rhs)
-        return true;
-
-    if (lhs->cls == rhs->cls) {
-        if (lhs->cls == str_cls) {
-            return static_cast<BoxedString*>(lhs)->s() == static_cast<BoxedString*>(rhs)->s();
-        }
-    }
-
-    // TODO fix this
-    Box* cmp = compareInternal(lhs, rhs, AST_TYPE::Eq, NULL);
-    return cmp->nonzeroIC();
+    int r = PyObject_RichCompareBool(lhs, rhs, Py_EQ);
+    if (r == -1)
+        throwCAPIException();
+    return (bool)r;
 }
 
 bool PyLt::operator()(Box* lhs, Box* rhs) const {
     STAT_TIMER(t0, "us_timer_PyLt");
 
-    // TODO fix this
-    Box* cmp = compareInternal(lhs, rhs, AST_TYPE::Lt, NULL);
-    return cmp->nonzeroIC();
+    int r = PyObject_RichCompareBool(lhs, rhs, Py_LT);
+    if (r == -1)
+        throwCAPIException();
+    return (bool)r;
 }
 
 extern "C" Box* deopt(AST_expr* expr, Box* value) {
@@ -3705,6 +3698,11 @@ Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrit
         return boxBool(b);
     }
 
+    if (isUserDefined(lhs->cls) || isUserDefined(rhs->cls)) {
+        rewrite_args = NULL;
+        REWRITE_ABORTED("");
+    }
+
     // Can do the guard checks after the Is/IsNot handling, since that is
     // irrespective of the object classes
     if (rewrite_args) {
@@ -3717,6 +3715,48 @@ Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrit
         rewrite_args->rhs->addAttrGuard(BOX_CLS_OFFSET, (intptr_t)rhs->cls);
     }
 
+    // TODO: switch from our op types to cpythons
+    int cpython_op_type;
+    switch (op_type) {
+        case AST_TYPE::Eq:
+            cpython_op_type = Py_EQ;
+            break;
+        case AST_TYPE::NotEq:
+            cpython_op_type = Py_NE;
+            break;
+        case AST_TYPE::Lt:
+            cpython_op_type = Py_LT;
+            break;
+        case AST_TYPE::LtE:
+            cpython_op_type = Py_LE;
+            break;
+        case AST_TYPE::Gt:
+            cpython_op_type = Py_GT;
+            break;
+        case AST_TYPE::GtE:
+            cpython_op_type = Py_GE;
+            break;
+        default:
+            RELEASE_ASSERT(0, "%d", op_type);
+    }
+
+    if (rewrite_args && lhs->cls == rhs->cls && !PyInstance_Check(lhs) && lhs->cls->tp_richcompare != NULL
+        && lhs->cls->tp_richcompare != slot_tp_richcompare) {
+        // This branch is the `v->ob_type == w->ob_type` branch of PyObject_RichCompare, but
+        // simplified by using the assumption that tp_richcompare exists and never returns NotImplemented
+        // for builtin types when both arguments are the right type.
+
+        assert(!isUserDefined(lhs->cls));
+
+        Box* r = lhs->cls->tp_richcompare(lhs, rhs, cpython_op_type);
+        RELEASE_ASSERT(r != NotImplemented, "%s returned notimplemented?", lhs->cls->tp_name);
+        rewrite_args->out_rtn
+            = rewrite_args->rewriter->call(true, (void*)lhs->cls->tp_richcompare, rewrite_args->lhs, rewrite_args->rhs,
+                                           rewrite_args->rewriter->loadConst(cpython_op_type));
+        rewrite_args->out_success = true;
+        return r;
+    }
+
     const std::string& op_name = getOpName(op_type);
 
     Box* lrtn;
@@ -3735,11 +3775,8 @@ Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrit
 
     if (lrtn) {
         if (lrtn != NotImplemented) {
-            bool can_patchpoint = !isUserDefined(lhs->cls) && !isUserDefined(rhs->cls);
             if (rewrite_args) {
-                if (can_patchpoint) {
-                    rewrite_args->out_success = true;
-                }
+                rewrite_args->out_success = true;
             }
             return lrtn;
         }
@@ -3783,38 +3820,8 @@ Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrit
     }
 #endif
 
-    // TODO
-    // According to http://docs.python.org/2/library/stdtypes.html#comparisons
-    // CPython implementation detail: Objects of different types except numbers are ordered by their type names; objects
-    // of the same types that don’t support proper comparison are ordered by their address.
-
-    if (op_type == AST_TYPE::Gt || op_type == AST_TYPE::GtE || op_type == AST_TYPE::Lt || op_type == AST_TYPE::LtE) {
-        intptr_t cmp1, cmp2;
-        if (lhs->cls == rhs->cls) {
-            cmp1 = (intptr_t)lhs;
-            cmp2 = (intptr_t)rhs;
-        } else {
-            // This isn't really necessary, but try to make sure that numbers get sorted first
-            if (lhs->cls == int_cls || lhs->cls == float_cls)
-                cmp1 = 0;
-            else
-                cmp1 = (intptr_t)lhs->cls;
-            if (rhs->cls == int_cls || rhs->cls == float_cls)
-                cmp2 = 0;
-            else
-                cmp2 = (intptr_t)rhs->cls;
-        }
-
-        if (op_type == AST_TYPE::Gt)
-            return boxBool(cmp1 > cmp2);
-        if (op_type == AST_TYPE::GtE)
-            return boxBool(cmp1 >= cmp2);
-        if (op_type == AST_TYPE::Lt)
-            return boxBool(cmp1 < cmp2);
-        if (op_type == AST_TYPE::LtE)
-            return boxBool(cmp1 <= cmp2);
-    }
-    RELEASE_ASSERT(0, "%d", op_type);
+    int c = default_3way_compare(lhs, rhs);
+    return convert_3way_to_object(cpython_op_type, c);
 }
 
 extern "C" Box* compare(Box* lhs, Box* rhs, int op_type) {
@@ -3827,21 +3834,52 @@ extern "C" Box* compare(Box* lhs, Box* rhs, int op_type) {
     std::unique_ptr<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 3, "compare"));
 
-    Box* rtn;
     if (rewriter.get()) {
         // rewriter->trap();
         CompareRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getArg(1),
                                         rewriter->getReturnDestination());
-        rtn = compareInternal(lhs, rhs, op_type, &rewrite_args);
+        Box* rtn = compareInternal(lhs, rhs, op_type, &rewrite_args);
         if (!rewrite_args.out_success) {
             rewriter.reset(NULL);
         } else
             rewriter->commitReturning(rewrite_args.out_rtn);
+        return rtn;
     } else {
-        rtn = compareInternal(lhs, rhs, op_type, NULL);
+        // TODO: switch from our op types to cpythons
+        int cpython_op_type;
+        if (op_type == AST_TYPE::In || op_type == AST_TYPE::NotIn)
+            return compareInternal(lhs, rhs, op_type, NULL);
+        if (op_type == AST_TYPE::Is)
+            return boxBool(lhs == rhs);
+        if (op_type == AST_TYPE::IsNot)
+            return boxBool(lhs != rhs);
+        switch (op_type) {
+            case AST_TYPE::Eq:
+                cpython_op_type = Py_EQ;
+                break;
+            case AST_TYPE::NotEq:
+                cpython_op_type = Py_NE;
+                break;
+            case AST_TYPE::Lt:
+                cpython_op_type = Py_LT;
+                break;
+            case AST_TYPE::LtE:
+                cpython_op_type = Py_LE;
+                break;
+            case AST_TYPE::Gt:
+                cpython_op_type = Py_GT;
+                break;
+            case AST_TYPE::GtE:
+                cpython_op_type = Py_GE;
+                break;
+            default:
+                RELEASE_ASSERT(0, "%d", op_type);
+        }
+        Box* r = PyObject_RichCompare(lhs, rhs, cpython_op_type);
+        if (!r)
+            throwCAPIException();
+        return r;
     }
-
-    return rtn;
 }
 
 extern "C" Box* unaryop(Box* operand, int op_type) {

src/runtime/str.cpp

@@ -23,6 +23,7 @@
 
 #include "Python.h"
 
+#include "capi/typeobject.h"
 #include "capi/types.h"
 #include "core/common.h"
 #include "core/types.h"
@@ -1154,64 +1155,46 @@ extern "C" Box* strMul(BoxedString* lhs, Box* rhs) {
     return boxString(buf);
 }
 
-extern "C" Box* strLt(BoxedString* lhs, Box* rhs) {
+Box* str_richcompare(Box* lhs, Box* rhs, int op) {
     assert(isSubclass(lhs->cls, str_cls));
 
-    if (!isSubclass(rhs->cls, str_cls))
-        return NotImplemented;
-
-    BoxedString* srhs = static_cast<BoxedString*>(rhs);
-    return boxBool(lhs->s() < srhs->s());
-}
-
-extern "C" Box* strLe(BoxedString* lhs, Box* rhs) {
-    assert(isSubclass(lhs->cls, str_cls));
-
-    if (!isSubclass(rhs->cls, str_cls))
-        return NotImplemented;
+    // Note: it is somehow about 50% faster to do this check inside the switch
+    // statement, rather than out here.  It's functionally equivalent but the
+    // generated assembly is somehow quite better:
+    // if (unlikely(!PyString_Check(rhs)))
+    // return NotImplemented;
 
+    BoxedString* slhs = static_cast<BoxedString*>(lhs);
     BoxedString* srhs = static_cast<BoxedString*>(rhs);
-    return boxBool(lhs->s() <= srhs->s());
-}
 
-extern "C" Box* strGt(BoxedString* lhs, Box* rhs) {
-    assert(isSubclass(lhs->cls, str_cls));
-
-    if (!isSubclass(rhs->cls, str_cls))
-        return NotImplemented;
-
-    BoxedString* srhs = static_cast<BoxedString*>(rhs);
-    return boxBool(lhs->s() > srhs->s());
-}
-
-extern "C" Box* strGe(BoxedString* lhs, Box* rhs) {
-    assert(isSubclass(lhs->cls, str_cls));
-
-    if (!isSubclass(rhs->cls, str_cls))
-        return NotImplemented;
-
-    BoxedString* srhs = static_cast<BoxedString*>(rhs);
-    return boxBool(lhs->s() >= srhs->s());
-}
-
-extern "C" Box* strEq(BoxedString* lhs, Box* rhs) {
-    assert(isSubclass(lhs->cls, str_cls));
-
-    if (!isSubclass(rhs->cls, str_cls))
-        return NotImplemented;
-
-    BoxedString* srhs = static_cast<BoxedString*>(rhs);
-    return boxBool(lhs->s() == srhs->s());
-}
-
-extern "C" Box* strNe(BoxedString* lhs, Box* rhs) {
-    assert(isSubclass(lhs->cls, str_cls));
-
-    if (!isSubclass(rhs->cls, str_cls))
-        return NotImplemented;
-
-    BoxedString* srhs = static_cast<BoxedString*>(rhs);
-    return boxBool(lhs->s() != srhs->s());
+    switch (op) {
+        case Py_EQ:
+            if (unlikely(!PyString_Check(rhs)))
+                return NotImplemented;
+            return boxBool(slhs->s() == srhs->s());
+        case Py_NE:
+            if (unlikely(!PyString_Check(rhs)))
+                return NotImplemented;
+            return boxBool(slhs->s() != srhs->s());
+        case Py_LT:
+            if (unlikely(!PyString_Check(rhs)))
+                return NotImplemented;
+            return boxBool(slhs->s() < srhs->s());
+        case Py_LE:
+            if (unlikely(!PyString_Check(rhs)))
+                return NotImplemented;
+            return boxBool(slhs->s() <= srhs->s());
+        case Py_GT:
+            if (unlikely(!PyString_Check(rhs)))
+                return NotImplemented;
+            return boxBool(slhs->s() > srhs->s());
+        case Py_GE:
+            if (unlikely(!PyString_Check(rhs)))
+                return NotImplemented;
+            return boxBool(slhs->s() >= srhs->s());
+        default:
+            llvm_unreachable("invalid op");
+    }
 }
 
 #define JUST_LEFT 0
@@ -2734,12 +2717,7 @@ void setupStr() {
     // TODO not sure if this is right in all cases:
     str_cls->giveAttr("__rmul__", new BoxedFunction(boxRTFunction((void*)strMul, UNKNOWN, 2)));
 
-    str_cls->giveAttr("__lt__", new BoxedFunction(boxRTFunction((void*)strLt, UNKNOWN, 2)));
-    str_cls->giveAttr("__le__", new BoxedFunction(boxRTFunction((void*)strLe, UNKNOWN, 2)));
-    str_cls->giveAttr("__gt__", new BoxedFunction(boxRTFunction((void*)strGt, UNKNOWN, 2)));
-    str_cls->giveAttr("__ge__", new BoxedFunction(boxRTFunction((void*)strGe, UNKNOWN, 2)));
-    str_cls->giveAttr("__eq__", new BoxedFunction(boxRTFunction((void*)strEq, UNKNOWN, 2)));
-    str_cls->giveAttr("__ne__", new BoxedFunction(boxRTFunction((void*)strNe, UNKNOWN, 2)));
+    str_cls->tp_richcompare = str_richcompare;
 
     BoxedString* spaceChar = boxStrConstant(" ");
     str_cls->giveAttr("ljust",
@@ -2763,6 +2741,7 @@ void setupStr() {
     str_cls->giveAttr("__new__",
                       new BoxedFunction(boxRTFunction((void*)strNew, UNKNOWN, 2, 1, false, false), { EmptyString }));
 
+    add_operators(str_cls);
     str_cls->freeze();
 
     basestring_cls->giveAttr(

src/runtime/tuple.cpp

@@ -225,89 +225,6 @@ Box* tupleRepr(BoxedTuple* t) {
     return boxString(os.str());
 }
 
-Box* _tupleCmp(BoxedTuple* lhs, BoxedTuple* rhs, AST_TYPE::AST_TYPE op_type) {
-    int lsz = lhs->size();
-    int rsz = rhs->size();
-
-    bool is_order
-        = (op_type == AST_TYPE::Lt || op_type == AST_TYPE::LtE || op_type == AST_TYPE::Gt || op_type == AST_TYPE::GtE);
-
-    int n = std::min(lsz, rsz);
-    for (int i = 0; i < n; i++) {
-        Box* is_eq = compareInternal(lhs->elts[i], rhs->elts[i], AST_TYPE::Eq, NULL);
-        bool bis_eq = nonzero(is_eq);
-
-        if (bis_eq)
-            continue;
-
-        if (op_type == AST_TYPE::Eq) {
-            return boxBool(false);
-        } else if (op_type == AST_TYPE::NotEq) {
-            return boxBool(true);
-        } else {
-            Box* r = compareInternal(lhs->elts[i], rhs->elts[i], op_type, NULL);
-            return r;
-        }
-    }
-
-    if (op_type == AST_TYPE::Lt)
-        return boxBool(lsz < rsz);
-    else if (op_type == AST_TYPE::LtE)
-        return boxBool(lsz <= rsz);
-    else if (op_type == AST_TYPE::Gt)
-        return boxBool(lsz > rsz);
-    else if (op_type == AST_TYPE::GtE)
-        return boxBool(lsz >= rsz);
-    else if (op_type == AST_TYPE::Eq)
-        return boxBool(lsz == rsz);
-    else if (op_type == AST_TYPE::NotEq)
-        return boxBool(lsz != rsz);
-
-    RELEASE_ASSERT(0, "%d", op_type);
-}
-
-Box* tupleLt(BoxedTuple* self, Box* rhs) {
-    if (!isSubclass(rhs->cls, tuple_cls)) {
-        return NotImplemented;
-    }
-    return _tupleCmp(self, static_cast<BoxedTuple*>(rhs), AST_TYPE::Lt);
-}
-
-Box* tupleLe(BoxedTuple* self, Box* rhs) {
-    if (!isSubclass(rhs->cls, tuple_cls)) {
-        return NotImplemented;
-    }
-    return _tupleCmp(self, static_cast<BoxedTuple*>(rhs), AST_TYPE::LtE);
-}
-
-Box* tupleGt(BoxedTuple* self, Box* rhs) {
-    if (!isSubclass(rhs->cls, tuple_cls)) {
-        return NotImplemented;
-    }
-    return _tupleCmp(self, static_cast<BoxedTuple*>(rhs), AST_TYPE::Gt);
-}
-
-Box* tupleGe(BoxedTuple* self, Box* rhs) {
-    if (!isSubclass(rhs->cls, tuple_cls)) {
-        return NotImplemented;
-    }
-    return _tupleCmp(self, static_cast<BoxedTuple*>(rhs), AST_TYPE::GtE);
-}
-
-Box* tupleEq(BoxedTuple* self, Box* rhs) {
-    if (!isSubclass(rhs->cls, tuple_cls)) {
-        return NotImplemented;
-    }
-    return _tupleCmp(self, static_cast<BoxedTuple*>(rhs), AST_TYPE::Eq);
-}
-
-Box* tupleNe(BoxedTuple* self, Box* rhs) {
-    if (!isSubclass(rhs->cls, tuple_cls)) {
-        return NotImplemented;
-    }
-    return _tupleCmp(self, static_cast<BoxedTuple*>(rhs), AST_TYPE::NotEq);
-}
-
 Box* tupleNonzero(BoxedTuple* self) {
     RELEASE_ASSERT(isSubclass(self->cls, tuple_cls), "");
     return boxBool(self->size() != 0);
@@ -442,6 +359,90 @@ static int64_t tuple_hash(BoxedTuple* v) noexcept {
     return x;
 }
 
+static PyObject* tuplerichcompare(PyObject* v, PyObject* w, int op) noexcept {
+    BoxedTuple* vt, *wt;
+    Py_ssize_t i;
+    Py_ssize_t vlen, wlen;
+
+    if (!PyTuple_Check(v) || !PyTuple_Check(w)) {
+        Py_INCREF(Py_NotImplemented);
+        return Py_NotImplemented;
+    }
+
+    vt = (BoxedTuple*)v;
+    wt = (BoxedTuple*)w;
+
+    vlen = Py_SIZE(vt);
+    wlen = Py_SIZE(wt);
+
+    /* Note:  the corresponding code for lists has an "early out" test
+     * here when op is EQ or NE and the lengths differ.  That pays there,
+     * but Tim was unable to find any real code where EQ/NE tuple
+     * compares don't have the same length, so testing for it here would
+     * have cost without benefit.
+     */
+
+    /* Search for the first index where items are different.
+     * Note that because tuples are immutable, it's safe to reuse
+     * vlen and wlen across the comparison calls.
+     */
+    for (i = 0; i < vlen && i < wlen; i++) {
+        int k = PyObject_RichCompareBool(vt->elts[i], wt->elts[i], Py_EQ);
+        if (k < 0)
+            return NULL;
+        if (!k)
+            break;
+    }
+
+    if (i >= vlen || i >= wlen) {
+        /* No more items to compare -- compare sizes */
+        int cmp;
+        PyObject* res;
+        switch (op) {
+            case Py_LT:
+                cmp = vlen < wlen;
+                break;
+            case Py_LE:
+                cmp = vlen <= wlen;
+                break;
+            case Py_EQ:
+                cmp = vlen == wlen;
+                break;
+            case Py_NE:
+                cmp = vlen != wlen;
+                break;
+            case Py_GT:
+                cmp = vlen > wlen;
+                break;
+            case Py_GE:
+                cmp = vlen >= wlen;
+                break;
+            default:
+                return NULL; /* cannot happen */
+        }
+        if (cmp)
+            res = Py_True;
+        else
+            res = Py_False;
+        Py_INCREF(res);
+        return res;
+    }
+
+    /* We have an item that differs -- shortcuts for EQ/NE */
+    if (op == Py_EQ) {
+        Py_INCREF(Py_False);
+        return Py_False;
+    }
+    if (op == Py_NE) {
+        Py_INCREF(Py_True);
+        return Py_True;
+    }
+
+    /* Compare the final item again using the proper operator */
+    return PyObject_RichCompare(vt->elts[i], wt->elts[i], op);
+}
+
+
 void setupTuple() {
     tuple_iterator_cls = BoxedHeapClass::create(type_cls, object_cls, &tupleIteratorGCHandler, 0, 0,
                                                 sizeof(BoxedTupleIterator), false, "tuple");
@@ -460,12 +461,7 @@ void setupTuple() {
                         new BoxedFunction(boxRTFunction((void*)tupleIter, typeFromClass(tuple_iterator_cls), 1)));
 
 
-    tuple_cls->giveAttr("__lt__", new BoxedFunction(boxRTFunction((void*)tupleLt, UNKNOWN, 2)));
-    tuple_cls->giveAttr("__le__", new BoxedFunction(boxRTFunction((void*)tupleLe, UNKNOWN, 2)));
-    tuple_cls->giveAttr("__gt__", new BoxedFunction(boxRTFunction((void*)tupleGt, UNKNOWN, 2)));
-    tuple_cls->giveAttr("__ge__", new BoxedFunction(boxRTFunction((void*)tupleGe, UNKNOWN, 2)));
-    tuple_cls->giveAttr("__eq__", new BoxedFunction(boxRTFunction((void*)tupleEq, UNKNOWN, 2)));
-    tuple_cls->giveAttr("__ne__", new BoxedFunction(boxRTFunction((void*)tupleNe, UNKNOWN, 2)));
+    tuple_cls->tp_richcompare = tuplerichcompare;
 
     tuple_cls->giveAttr("__nonzero__", new BoxedFunction(boxRTFunction((void*)tupleNonzero, BOXED_BOOL, 1)));
 

src/runtime/types.cpp

@@ -2294,6 +2294,63 @@ Box* decodeUTF8StringPtr(const std::string* s) {
     return rtn;
 }
 
+static PyObject* type_richcompare(PyObject* v, PyObject* w, int op) noexcept {
+    PyObject* result;
+    Py_uintptr_t vv, ww;
+    int c;
+
+    /* Make sure both arguments are types. */
+    if (!PyType_Check(v) || !PyType_Check(w) ||
+        /* If there is a __cmp__ method defined, let it be called instead
+           of our dumb function designed merely to warn.  See bug
+           #7491. */
+        Py_TYPE(v)->tp_compare || Py_TYPE(w)->tp_compare) {
+        result = Py_NotImplemented;
+        goto out;
+    }
+
+    /* Py3K warning if comparison isn't == or !=  */
+    if (Py_Py3kWarningFlag && op != Py_EQ && op != Py_NE
+        && PyErr_WarnEx(PyExc_DeprecationWarning, "type inequality comparisons not supported "
+                                                  "in 3.x",
+                        1) < 0) {
+        return NULL;
+    }
+
+    /* Compare addresses */
+    vv = (Py_uintptr_t)v;
+    ww = (Py_uintptr_t)w;
+    switch (op) {
+        case Py_LT:
+            c = vv < ww;
+            break;
+        case Py_LE:
+            c = vv <= ww;
+            break;
+        case Py_EQ:
+            c = vv == ww;
+            break;
+        case Py_NE:
+            c = vv != ww;
+            break;
+        case Py_GT:
+            c = vv > ww;
+            break;
+        case Py_GE:
+            c = vv >= ww;
+            break;
+        default:
+            result = Py_NotImplemented;
+            goto out;
+    }
+    result = c ? Py_True : Py_False;
+
+/* incref and return */
+out:
+    Py_INCREF(result);
+    return result;
+}
+
 bool TRACK_ALLOCATIONS = false;
 void setupRuntime() {
 
@@ -2563,6 +2620,8 @@ void setupRuntime() {
                        new BoxedMemberDescriptor(BoxedMemberDescriptor::OBJECT, offsetof(BoxedClass, tp_mro)));
     type_cls->giveAttr("__subclasses__", new BoxedFunction(boxRTFunction((void*)typeSubclasses, UNKNOWN, 1)));
     type_cls->giveAttr("mro", new BoxedFunction(boxRTFunction((void*)typeMro, UNKNOWN, 1)));
+    type_cls->tp_richcompare = type_richcompare;
+    add_operators(type_cls);
     type_cls->freeze();
 
     none_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)noneRepr, STR, 1)));

test/tests/comparisons.py

 def f(a, b):
-    print a, b, "<", a < b
-    print a, b, "<=", a <= b
-    print a, b, ">", a > b
-    print a, b, ">=", a >= b
-    print a, b, "==", a == b
-    print a, b, "!=", a != b
-    print a, b, "is", a is b
-    print a, b, "is not", a is not b
+    print repr(a), repr(b), "<", a < b
+    print repr(a), repr(b), "<=", a <= b
+    print repr(a), repr(b), ">", a > b
+    print repr(a), repr(b), ">=", a >= b
+    print repr(a), repr(b), "==", a == b
+    print repr(a), repr(b), "!=", a != b
+    print repr(a), repr(b), "is", a is b
+    print repr(a), repr(b), "is not", a is not b
 
 class C(object):
     pass
@@ -14,7 +14,7 @@ class C(object):
 class Z(object):
     pass
 
-args = [0, 1, 0.1, 1.1, "hello", float('nan'), float('inf'), float('-inf')]#, C(), Z()]
+args = [0, 1, 0.1, 1.1, "hello", float('nan'), float('inf'), float('-inf'), 0L, 1L]#, C(), Z()]
 
 for i in xrange(len(args)):
     for j in xrange(i):