Merge pull request #1145 from undingen/fix_set_insert_order

fix set.add() for existing keys and fix set ast node when encountering keys with same hashes

src/codegen/ast_interpreter.cpp

@@ -1590,20 +1590,20 @@ Value ASTInterpreter::visit_dict(AST_Dict* node) {
 }
 
 Value ASTInterpreter::visit_set(AST_Set* node) {
-    llvm::SmallVector<RewriterVar*, 8> items;
-
-    BoxedSet::Set set;
-    for (AST_expr* e : node->elts) {
-        Value v = visit_expr(e);
-        auto&& p = set.insert(v.o);
-        if (!p.second /* already exists */) {
-            Py_DECREF(p.first->value);
-            *p.first = v.o;
+    try {
+        // insert the elements in reverse like cpython does
+        // important for {1, 1L}
+        llvm::SmallVector<RewriterVar*, 8> items;
+        BoxedSet* set = (BoxedSet*)createSet();
+        for (auto it = node->elts.rbegin(), it_end = node->elts.rend(); it != it_end; ++it) {
+            Value v = visit_expr(*it);
+            _setAddStolen(set, v.o);
+            items.push_back(v);
         }
-        items.push_back(v);
+        return Value(set, jit ? jit->emitCreateSet(items) : NULL);
+    } catch (ExcInfo e) {
+        RELEASE_ASSERT(0, "this leaks in case of an exception");
     }
-
-    return Value(new BoxedSet(std::move(set)), jit ? jit->emitCreateSet(items) : NULL);
 }
 
 Value ASTInterpreter::visit_str(AST_Str* node) {

src/codegen/baseline_jit.cpp

@@ -871,15 +871,15 @@ Box* JitFragmentWriter::createListHelper(uint64_t num, Box** data) {
 }
 
 Box* JitFragmentWriter::createSetHelper(uint64_t num, Box** data) {
-    BoxedSet* set = (BoxedSet*)createSet();
-    for (int i = 0; i < num; ++i) {
-        auto&& p = set->s.insert(data[i]);
-        if (!p.second /* already exists */) {
-            Py_DECREF(p.first->value);
-            *p.first = data[i];
+    try {
+        BoxedSet* set = (BoxedSet*)createSet();
+        for (int i = 0; i < num; ++i) {
+            _setAddStolen(set, data[i]);
         }
+        return set;
+    } catch (ExcInfo e) {
+        RELEASE_ASSERT(0, "this leaks in case of an exception");
     }
-    return set;
 }
 
 Box* JitFragmentWriter::createTupleHelper(uint64_t num, Box** data) {

src/codegen/irgen/irgenerator.cpp

@@ -1445,8 +1445,10 @@ private:
 
         static BoxedString* add_str = getStaticString("add");
 
-        for (int i = 0; i < node->elts.size(); i++) {
-            CompilerVariable* elt = elts[i];
+        // insert the elements in reverse like cpython does
+        // important for {1, 1L}
+        for (auto it = elts.rbegin(), it_end = elts.rend(); it != it_end; ++it) {
+            CompilerVariable* elt = *it;
             CallattrFlags flags = {.cls_only = true, .null_on_nonexistent = false, .argspec = ArgPassSpec(1) };
             CompilerVariable* r
                 = rtn->callattr(emitter, getOpInfoForNode(node, unw_info), add_str, flags, { elt }, NULL);

src/runtime/set.cpp

@@ -24,6 +24,14 @@ extern "C" Box* createSet() {
     return new BoxedSet();
 }
 
+void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val) {
+    auto&& p = self->s.insert(val);
+    if (!p.second /* already exists */) {
+        // keep the original key
+        Py_DECREF(val.value);
+    }
+}
+
 namespace set {
 
 class BoxedSetIterator : public Box {
@@ -89,14 +97,6 @@ Box* setiteratorIter(BoxedSetIterator* self) {
     return incref(self);
 }
 
-static void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val) {
-    auto&& p = self->s.insert(val);
-    if (!p.second /* already exists */) {
-        Py_DECREF(p.first->value);
-        *p.first = val;
-    }
-}
-
 static void _setAdd(BoxedSet* self, BoxAndHash val) {
     Py_INCREF(val.value);
     _setAddStolen(self, val);

src/runtime/set.h

@@ -41,6 +41,8 @@ public:
     static int traverse(Box* self, visitproc visit, void* arg) noexcept;
     static int clear(Box* self) noexcept;
 };
+
+void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val);
 }
 
 #endif

test/tests/dict.py

-d = {2:"should get overwritten", 2:2}
+d = {2:"should get overwritten", 2L:2}
 d[1] = 1
 print d
 print d[1], d[1L], d[1.0], d[True]

test/tests/set.py

@@ -260,3 +260,9 @@ s.remove(1L)
 s = set([1, 2, 3, 4])
 s2 = set([3L, 4L, 5L, 6L])
 s.symmetric_difference_update(s2)
+
+# make sure we are inserting the tuple elements in reverse:
+print {1, 1L}, {1L, 1}, set([1, 1L]), set([1L, 1])
+s = {1}
+s.add(1L)
+print s