Merge pull request #573 from kmod/tp_hash

Switch hash to using tp_hash

src/capi/typeobject.cpp

@@ -685,6 +685,9 @@ static PyObject* slot_tp_str(PyObject* self) noexcept {
 }
 
 static long slot_tp_hash(PyObject* self) noexcept {
+    static StatCounter slowpath_hash("slowpath_hash");
+    slowpath_hash.log();
+
     PyObject* func;
     static PyObject* hash_str, *eq_str, *cmp_str;
     long h;

src/runtime/builtin_modules/builtins.cpp

@@ -719,7 +719,15 @@ extern "C" PyObject* PyErr_NewException(char* name, PyObject* _base, PyObject* d
         Box* cls = runtimeCall(type_cls, ArgPassSpec(3), boxedName, BoxedTuple::create({ base }), dict, NULL, NULL);
         return cls;
     } catch (ExcInfo e) {
-        abort();
+        // PyErr_NewException isn't supposed to fail, and callers sometimes take advantage of that
+        // by not checking the return value.  Since failing probably indicates a bug anyway,
+        // to be safe just print the traceback and die.
+        e.printExcAndTraceback();
+        RELEASE_ASSERT(0, "PyErr_NewException failed");
+
+        // The proper way of handling it:
+        setCAPIException(e);
+        return NULL;
     }
 }
 

src/runtime/capi.cpp

@@ -506,15 +506,6 @@ extern "C" int PyObject_DelItem(PyObject* o, PyObject* key) noexcept {
     }
 }
 
-extern "C" long PyObject_Hash(PyObject* o) noexcept {
-    try {
-        return hash(o)->n;
-    } catch (ExcInfo e) {
-        fatalOrError(PyExc_NotImplementedError, "unimplemented");
-        return -1;
-    }
-}
-
 extern "C" long PyObject_HashNotImplemented(PyObject* self) noexcept {
     PyErr_Format(PyExc_TypeError, "unhashable type: '%.200s'", Py_TYPE(self)->tp_name);
     return -1;

src/runtime/dict.cpp

@@ -227,10 +227,14 @@ extern "C" int PyDict_SetItem(PyObject* mp, PyObject* _key, PyObject* _item) noe
     Box* key = static_cast<Box*>(_key);
     Box* item = static_cast<Box*>(_item);
 
+    assert(key);
+    assert(item);
+
     try {
         setitem(b, key, item);
     } catch (ExcInfo e) {
-        abort();
+        setCAPIException(e);
+        return -1;
     }
     return 0;
 }

src/runtime/int.cpp

@@ -17,6 +17,7 @@
 #include <cmath>
 #include <sstream>
 
+#include "capi/typeobject.h"
 #include "core/common.h"
 #include "core/options.h"
 #include "core/stats.h"
@@ -1109,6 +1110,13 @@ static Box* int1(Box*, void*) {
     return boxInt(1);
 }
 
+static int64_t int_hash(BoxedInt* o) noexcept {
+    int64_t n = o->n;
+    if (n == -1)
+        return -2;
+    return n;
+}
+
 void setupInt() {
     for (int i = 0; i < NUM_INTERNED_INTS; i++) {
         interned_ints[i] = new BoxedInt(i);
@@ -1143,7 +1151,7 @@ void setupInt() {
     int_cls->giveAttr("__neg__", new BoxedFunction(boxRTFunction((void*)intNeg, UNKNOWN, 1)));
     int_cls->giveAttr("__nonzero__", new BoxedFunction(boxRTFunction((void*)intNonzero, BOXED_BOOL, 1)));
     int_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)intRepr, STR, 1)));
-    int_cls->giveAttr("__hash__", new BoxedFunction(boxRTFunction((void*)intHash, BOXED_INT, 1)));
+    int_cls->tp_hash = (hashfunc)int_hash;
     int_cls->giveAttr("__divmod__", new BoxedFunction(boxRTFunction((void*)intDivmod, UNKNOWN, 2)));
 
     int_cls->giveAttr("__hex__", new BoxedFunction(boxRTFunction((void*)intHex, STR, 1)));
@@ -1164,6 +1172,7 @@ void setupInt() {
     int_cls->giveAttr("numerator", new (pyston_getset_cls) BoxedGetsetDescriptor(intInt, NULL, NULL));
     int_cls->giveAttr("denominator", new (pyston_getset_cls) BoxedGetsetDescriptor(int1, NULL, NULL));
 
+    add_operators(int_cls);
     int_cls->freeze();
 }
 

src/runtime/objmodel.cpp

@@ -131,10 +131,7 @@ size_t PyHasher::operator()(Box* b) const {
         return H(s->data(), s->size());
     }
 
-    BoxedInt* i = hash(b);
-    assert(sizeof(size_t) == sizeof(i->n));
-    size_t rtn = i->n;
-    return rtn;
+    return hashUnboxed(b);
 }
 
 bool PyEq::operator()(Box* lhs, Box* rhs) const {
@@ -2224,30 +2221,43 @@ extern "C" bool exceptionMatches(Box* obj, Box* cls) {
     return rtn;
 }
 
-extern "C" BoxedInt* hash(Box* obj) {
-    static StatCounter slowpath_hash("slowpath_hash");
-    slowpath_hash.log();
-
-    // goes through descriptor logic
-    Box* hash = getclsattrInternal(obj, "__hash__", NULL);
+/* Macro to get the tp_richcompare field of a type if defined */
+#define RICHCOMPARE(t) (PyType_HasFeature((t), Py_TPFLAGS_HAVE_RICHCOMPARE) ? (t)->tp_richcompare : NULL)
 
-    if (hash == NULL) {
-        ASSERT(isUserDefined(obj->cls) || obj->cls == function_cls || obj->cls == object_cls || obj->cls == classobj_cls
-                   || obj->cls == module_cls || obj->cls == capifunc_cls || obj->cls == instancemethod_cls,
-               "%s.__hash__", getTypeName(obj));
-        // TODO not the best way to handle this...
-        return static_cast<BoxedInt*>(boxInt((i64)obj));
+extern "C" long PyObject_Hash(PyObject* v) noexcept {
+    PyTypeObject* tp = v->cls;
+    if (tp->tp_hash != NULL)
+        return (*tp->tp_hash)(v);
+#if 0 // pyston change
+    /* To keep to the general practice that inheriting
+     * solely from object in C code should work without
+     * an explicit call to PyType_Ready, we implicitly call
+     * PyType_Ready here and then check the tp_hash slot again
+     */
+    if (tp->tp_dict == NULL) {
+        if (PyType_Ready(tp) < 0)
+            return -1;
+        if (tp->tp_hash != NULL)
+            return (*tp->tp_hash)(v);
     }
-
-    if (hash == None) {
-        raiseExcHelper(TypeError, "unhashable type: '%s'", obj->cls->tp_name);
+#endif
+    if (tp->tp_compare == NULL && RICHCOMPARE(tp) == NULL) {
+        return _Py_HashPointer(v); /* Use address as hash value */
     }
+    /* If there's a cmp but no hash defined, the object can't be hashed */
+    return PyObject_HashNotImplemented(v);
+}
 
-    Box* rtn = runtimeCallInternal(hash, NULL, ArgPassSpec(0), NULL, NULL, NULL, NULL, NULL);
-    if (rtn->cls != int_cls) {
-        raiseExcHelper(TypeError, "an integer is required");
-    }
-    return static_cast<BoxedInt*>(rtn);
+int64_t hashUnboxed(Box* obj) {
+    auto r = PyObject_Hash(obj);
+    if (r == -1)
+        throwCAPIException();
+    return r;
+}
+
+extern "C" BoxedInt* hash(Box* obj) {
+    int64_t r = hashUnboxed(obj);
+    return new BoxedInt(r);
 }
 
 extern "C" BoxedInt* lenInternal(Box* obj, LenRewriteArgs* rewrite_args) {
@@ -2373,8 +2383,11 @@ extern "C" void dumpEx(void* p, int levels) {
         return;
     }
 
-    if (al->kind_id == gc::GCKind::PYTHON) {
-        printf("Python object\n");
+    if (al->kind_id == gc::GCKind::PYTHON || al->kind_id == gc::GCKind::CONSERVATIVE_PYTHON) {
+        if (al->kind_id == gc::GCKind::PYTHON)
+            printf("Python object (precisely scanned)\n");
+        else
+            printf("Python object (conservatively scanned)\n");
         Box* b = (Box*)p;
 
         printf("Class: %s", getFullTypeName(b).c_str());

src/runtime/objmodel.h

@@ -65,6 +65,7 @@ extern "C" BoxedString* strOrNull(Box* obj);  // similar to str, but returns NUL
 extern "C" Box* strOrUnicode(Box* obj);
 extern "C" bool exceptionMatches(Box* obj, Box* cls);
 extern "C" BoxedInt* hash(Box* obj);
+extern "C" int64_t hashUnboxed(Box* obj);
 extern "C" Box* abs_(Box* obj);
 // extern "C" Box* chr(Box* arg);
 extern "C" Box* compare(Box*, Box*, int);

src/runtime/tuple.cpp

@@ -18,6 +18,7 @@
 
 #include "llvm/Support/raw_ostream.h"
 
+#include "capi/typeobject.h"
 #include "core/ast.h"
 #include "core/common.h"
 #include "core/stats.h"
@@ -337,20 +338,6 @@ Box* tupleIndex(BoxedTuple* self, Box* elt) {
     raiseExcHelper(ValueError, "tuple.index(x): x not in tuple");
 }
 
-Box* tupleHash(BoxedTuple* self) {
-    STAT_TIMER(t0, "us_timer_tupleHash");
-    assert(isSubclass(self->cls, tuple_cls));
-
-    int64_t rtn = 3527539;
-    for (auto e : *self) {
-        BoxedInt* h = hash(e);
-        assert(isSubclass(h->cls, int_cls));
-        rtn ^= h->n + 0x9e3779b9 + (rtn << 6) + (rtn >> 2);
-    }
-
-    return boxInt(rtn);
-}
-
 extern "C" Box* tupleNew(Box* _cls, BoxedTuple* args, BoxedDict* kwargs) {
     if (!isSubclass(_cls->cls, type_cls))
         raiseExcHelper(TypeError, "tuple.__new__(X): X is not a type object (%s)", getTypeName(_cls));
@@ -434,6 +421,26 @@ extern "C" void tupleIteratorGCHandler(GCVisitor* v, Box* b) {
     v->visit(it->t);
 }
 
+static int64_t tuple_hash(BoxedTuple* v) noexcept {
+    long x, y;
+    Py_ssize_t len = Py_SIZE(v);
+    PyObject** p;
+    long mult = 1000003L;
+    x = 0x345678L;
+    p = v->elts;
+    while (--len >= 0) {
+        y = PyObject_Hash(*p++);
+        if (y == -1)
+            return -1;
+        x = (x ^ y) * mult;
+        /* the cast might truncate len; that doesn't change hash stability */
+        mult += (long)(82520L + len + len);
+    }
+    x += 97531L;
+    if (x == -1)
+        x = -2;
+    return x;
+}
 
 void setupTuple() {
     tuple_iterator_cls = BoxedHeapClass::create(type_cls, object_cls, &tupleIteratorGCHandler, 0, 0,
@@ -462,13 +469,15 @@ void setupTuple() {
 
     tuple_cls->giveAttr("__nonzero__", new BoxedFunction(boxRTFunction((void*)tupleNonzero, BOXED_BOOL, 1)));
 
-    tuple_cls->giveAttr("__hash__", new BoxedFunction(boxRTFunction((void*)tupleHash, BOXED_INT, 1)));
     tuple_cls->giveAttr("__len__", new BoxedFunction(boxRTFunction((void*)tupleLen, BOXED_INT, 1)));
     tuple_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)tupleRepr, STR, 1)));
     tuple_cls->giveAttr("__add__", new BoxedFunction(boxRTFunction((void*)tupleAdd, BOXED_TUPLE, 2)));
     tuple_cls->giveAttr("__mul__", new BoxedFunction(boxRTFunction((void*)tupleMul, BOXED_TUPLE, 2)));
     tuple_cls->giveAttr("__rmul__", new BoxedFunction(boxRTFunction((void*)tupleMul, BOXED_TUPLE, 2)));
 
+    tuple_cls->tp_hash = (hashfunc)tuple_hash;
+    add_operators(tuple_cls);
+
     tuple_cls->freeze();
 
     CLFunction* hasnext = boxRTFunction((void*)tupleiterHasnextUnboxed, BOOL, 1);

src/runtime/types.cpp

@@ -2440,6 +2440,8 @@ void setupRuntime() {
     wrapperobject_cls->tp_mro = BoxedTuple::create({ wrapperobject_cls, object_cls });
     wrapperdescr_cls->tp_mro = BoxedTuple::create({ wrapperdescr_cls, object_cls });
 
+    object_cls->tp_hash = (hashfunc)_Py_HashPointer;
+
     STR = typeFromClass(str_cls);
     BOXED_INT = typeFromClass(int_cls);
     BOXED_FLOAT = typeFromClass(float_cls);
@@ -2536,8 +2538,6 @@ void setupRuntime() {
 
     object_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)objectRepr, UNKNOWN, 1, 0, false, false)));
     object_cls->giveAttr("__str__", new BoxedFunction(boxRTFunction((void*)objectStr, UNKNOWN, 1, 0, false, false)));
-    object_cls->giveAttr("__hash__",
-                         new BoxedFunction(boxRTFunction((void*)objectHash, BOXED_INT, 1, 0, false, false)));
     object_cls->giveAttr("__subclasshook__",
                          boxInstanceMethod(object_cls,
                                            new BoxedFunction(boxRTFunction((void*)objectSubclasshook, UNKNOWN, 2)),
@@ -2557,7 +2557,7 @@ void setupRuntime() {
     type_cls->giveAttr("__new__",
                        new BoxedFunction(boxRTFunction((void*)typeNew, UNKNOWN, 4, 2, false, false), { NULL, NULL }));
     type_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)typeRepr, STR, 1)));
-    type_cls->giveAttr("__hash__", new BoxedFunction(boxRTFunction((void*)typeHash, BOXED_INT, 1)));
+    type_cls->tp_hash = (hashfunc)_Py_HashPointer;
     type_cls->giveAttr("__module__", new (pyston_getset_cls) BoxedGetsetDescriptor(typeModule, typeSetModule, NULL));
     type_cls->giveAttr("__mro__",
                        new BoxedMemberDescriptor(BoxedMemberDescriptor::OBJECT, offsetof(BoxedClass, tp_mro)));
@@ -2566,8 +2566,8 @@ void setupRuntime() {
     type_cls->freeze();
 
     none_cls->giveAttr("__repr__", new BoxedFunction(boxRTFunction((void*)noneRepr, STR, 1)));
-    none_cls->giveAttr("__hash__", new BoxedFunction(boxRTFunction((void*)noneHash, UNKNOWN, 1)));
     none_cls->giveAttr("__nonzero__", new BoxedFunction(boxRTFunction((void*)noneNonzero, BOXED_BOOL, 1)));
+    none_cls->tp_hash = (hashfunc)_Py_HashPointer;
     none_cls->freeze();
 
     module_cls->giveAttr("__init__",
@@ -2780,6 +2780,10 @@ void setupRuntime() {
     assert(object_cls->tp_setattro == PyObject_GenericSetAttr);
     assert(none_cls->tp_setattro == PyObject_GenericSetAttr);
 
+    assert(object_cls->tp_hash == (hashfunc)_Py_HashPointer);
+    assert(none_cls->tp_hash == (hashfunc)_Py_HashPointer);
+    assert(type_cls->tp_hash == (hashfunc)_Py_HashPointer);
+
     setupSysEnd();
 
     TRACK_ALLOCATIONS = true;