Be more careful about how we remove elements from sets

ie if we do set.remove(k), we don't decrement k -- we decrement
whatever was in the set, which can be different.

src/runtime/set.cpp

@@ -102,10 +102,14 @@ static void _setAdd(BoxedSet* self, BoxAndHash val) {
     _setAddStolen(self, val);
 }
 
-static void _setRemove(BoxedSet* self, BoxAndHash val) {
-    bool erased = self->s.erase(val);
-    if (erased)
-        Py_DECREF(val.value);
+static bool _setRemove(BoxedSet* self, BoxAndHash val) {
+    auto it = self->s.find(val);
+    if (it == self->s.end())
+        return false;
+    Box* to_decref = it->value;
+    self->s.erase(it);
+    Py_DECREF(to_decref);
+    return true;
 }
 
 // Creates a set of type 'cls' from 'container' (NULL to get an empty set).
@@ -288,8 +292,7 @@ static void _setSymmetricDifferenceUpdate(BoxedSet* self, Box* other) {
     for (auto elt : other_set->s) {
         auto&& p = self->s.insert(elt);
         if (!p.second /* already exists */) {
-            self->s.erase(p.first);
-            Py_DECREF(elt.value);
+            _setRemove(self, elt);
         } else {
             Py_INCREF(elt.value);
         }
@@ -364,9 +367,7 @@ Box* setISub(BoxedSet* lhs, BoxedSet* rhs) {
 
     // TODO: write and call setDifferenceUpdate2
     for (auto&& elt : rhs->s) {
-        bool found = lhs->s.erase(elt);
-        if (found)
-            Py_DECREF(elt.value);
+        _setRemove(lhs, elt);
     }
     return incref(lhs);
 }
@@ -734,12 +735,9 @@ Box* setRemove(BoxedSet* self, Box* key) {
 
     if (PySet_Check(key)) {
         try {
-            BoxAndHash k_hash(key);
-            if (self->s.find(k_hash) != self->s.end()) {
-                bool existed = self->s.erase(k_hash);
-                if (existed)
-                    return incref(None);
-            }
+            bool existed = _setRemove(self, key);
+            if (existed)
+                return incref(None);
         } catch (ExcInfo e) {
             if (!e.matches(TypeError))
                 throw e;
@@ -748,23 +746,17 @@ Box* setRemove(BoxedSet* self, Box* key) {
 
             BoxedSet* tmpKey = makeNewSet(frozenset_cls, key);
             AUTO_DECREF(tmpKey);
-            if (self->s.find(tmpKey) != self->s.end()) {
-                bool existed = self->s.erase(tmpKey);
-                if (existed)
-                    return incref(None);
-            }
+            bool existed = _setRemove(self, tmpKey);
+            if (existed)
+                return incref(None);
         }
         raiseExcHelper(KeyError, key);
     }
 
-    auto it = self->s.find(key);
-    if (it == self->s.end()) {
-        raiseExcHelper(KeyError, key);
-    }
-
-    self->s.erase(it);
-    Py_DECREF(key);
-    return incref(None);
+    bool existed = _setRemove(self, key);
+    if (existed)
+        return incref(None);
+    raiseExcHelper(KeyError, key);
 }
 
 Box* setDiscard(BoxedSet* self, Box* key) {
@@ -772,10 +764,7 @@ Box* setDiscard(BoxedSet* self, Box* key) {
 
     if (PySet_Check(key)) {
         try {
-            BoxAndHash k_hash(key);
-            if (self->s.find(k_hash) != self->s.end()) {
-                self->s.erase(k_hash);
-            }
+            _setRemove(self, key);
         } catch (ExcInfo e) {
             if (!e.matches(TypeError))
                 throw e;
@@ -784,18 +773,12 @@ Box* setDiscard(BoxedSet* self, Box* key) {
 
             BoxedSet* tmpKey = makeNewSet(frozenset_cls, key);
             AUTO_DECREF(tmpKey);
-            if (self->s.find(tmpKey) != self->s.end()) {
-                self->s.erase(tmpKey);
-            }
+            _setRemove(self, tmpKey);
         }
         return incref(None);
     }
 
-    auto it = self->s.find(key);
-    if (it != self->s.end()) {
-        self->s.erase(it);
-        Py_DECREF(key);
-    }
+    _setRemove(self, key);
 
     return incref(None);
 }

test/tests/dict.py

@@ -235,3 +235,8 @@ print sorted(l)
 d = dict()
 d['two'] = d
 print d
+
+
+# Remove an item using a different key:
+d = {1:1}
+d.pop(1L)

test/tests/set.py

-# expected: reffail
 s1 = {1}
 
 def sorted(s):
@@ -251,3 +250,13 @@ a = set()
 a.add(foo)
 print(a.remove(foo))
 print(foo in set())
+
+
+# Remove an item using a different key:
+s = set()
+s.add(1)
+s.remove(1L)
+
+s = set([1, 2, 3, 4])
+s2 = set([3L, 4L, 5L, 6L])
+s.symmetric_difference_update(s2)

test/tests/urlparse_test.py

-# expected: reffail
 import urlparse
 print urlparse.urlparse("http://www.dropbox.com")