Switch to calling tp_richcompare

The notable places that are changed are PyEq, PyLt, and
compare/compareInternal.

The old codepaths are still in there (though thankfully
now with reduced duplication with cpython), since for anything
that defines a Python-level __lt__, it's better to call that
with our rewriting support, rather than calling it through
slot_tp_richcompare.

The control flow is kind of messy, since I don't think we
know what the right long-term organization is for these kinds of
things.  But basically, it's:
- if we can't rewrite, just call the C slot
- if we can rewrite, and we think calling tp_richcompare is profitable
  (ie it's not slot_tp_richcompare), we call that and emit a call to
  it in the patchpoint
- otherwise, we try calling the python attribute

Actual conversion of our attributes to tp_richcompare in the next commit.

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;

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) {

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):