Dict views mostly working

Author: xFrednet

Lib/test/test_regions/test_dict.py

@@ -1,8 +1,42 @@
 import unittest
+import sys
 from regions import Region, is_local
 from immutable import freeze, isfrozen
 
-class BaseTestRegionDict(unittest.TestCase):
+class TestRegionDict(unittest.TestCase):
+    def check_view_ref(self, create_view):
+        # Setup
+        r = Region()
+        r.dict = {}
+        base_lrc = r._lrc
+
+        # Pre-condition
+        self.assertGreater(base_lrc, 0)
+
+        # Action: Creating a view
+        view = create_view(r.dict)
+        self.assertEqual(r._lrc, base_lrc + 1)
+
+        # Action: Check mapping of view
+        mapping = view.mapping
+        self.assertEqual(r._lrc, base_lrc + 2)
+
+        # Clearing references could decrease the LRC
+        mapping = None
+        self.assertEqual(r._lrc, base_lrc + 1)
+        view = None
+        self.assertEqual(r._lrc, base_lrc)
+
+    def test_new_dict_refs_from_item_view(self):
+        self.check_view_ref(lambda dict: dict.items())
+
+    def test_new_dict_refs_from_keys_view(self):
+        self.check_view_ref(lambda dict: dict.keys())
+
+    def test_new_dict_refs_from_values_view(self):
+        self.check_view_ref(lambda dict: dict.values())
+
+class BaseTestRegionDictKeys(unittest.TestCase):
     __test__ = False
 
     def check_dict_assign(self, dict, key):
@@ -103,8 +137,39 @@ class SomeObject:
         # Post-condition
         self.assertEqual(r._lrc, lrc + 1)
 
+    def check_loop_lrc_change(self, region, iter_src, loop_lrc_effect, iter_lrc_cost = 1):
+        # Check loop iterations change the LRC
+        lrc = region._lrc
+        i = 0
+        for v in iter_src:
+            self.assertEqual(region._lrc, lrc + iter_lrc_cost + loop_lrc_effect,
+                             f"Fail in iteration: {i} base LRC {lrc} + {iter_lrc_cost} for iter")
+            # Reassigning v should reset the LRC
+            v = None
+            self.assertEqual(region._lrc, lrc + iter_lrc_cost,
+                             f"LRC didn't reset in iteration: {i} base LRC {lrc} + {iter_lrc_cost} for iter")
+            i += 1
+
+        # Check LRC is back to pre-loop levels
+        self.assertEqual(region._lrc, lrc)
+
+    def check_dict_view(self, key1, key2, create_view, loop_lrc_effect, iter_lrc_cost = 1):
+        class SomeObject:
+            pass
+        freeze(SomeObject())
+
+        # Setup
+        r = Region()
+        r.dict = {}
+        r.dict[key1] = SomeObject()
+        r.dict[key2] = SomeObject()
+
+        # Create the view
+        view = create_view(r.dict)
+
+        self.check_loop_lrc_change(r, view, loop_lrc_effect, iter_lrc_cost)
 
-class TestRegionDictUnicodeKeys(BaseTestRegionDict):
+class TestRegionDictUnicodeKeys(BaseTestRegionDictKeys):
     def test_wb_insert_empty(self):
         self.check_dict_assign({}, "some-key")
 
@@ -138,7 +203,24 @@ def test_wb_get_default(self):
     def test_wb_copy(self):
         self.check_dict_item_access("another key", lambda dict, key: dict.copy())
 
-class TestRegionDictObjectKeys(BaseTestRegionDict):
+    def test_wb_keys_view(self):
+        self.check_dict_view("K1", "K2", lambda d: d.keys(), loop_lrc_effect=0)
+
+    def test_wb_values_view(self):
+        self.check_dict_view("K1", "K2", lambda d: d.values(), loop_lrc_effect=1)
+
+    @unittest.expectedFailure # FIXME(regions): xFrednet: Broken until WBs in tuples have been added
+    def test_wb_items_view(self):
+        self.check_dict_view("K1", "K2", lambda d: d.items(), loop_lrc_effect=1)
+
+    def test_wb_iter_dict(self):
+        self.check_dict_view("K1", "K2", lambda d: d, loop_lrc_effect=0)
+
+    def test_wb_iter_dict_reversed(self):
+        # The iterator doesn't effect the LRC, since we create it with the `reversed`
+        self.check_dict_view("K1", "K2", lambda d: reversed(d), loop_lrc_effect=0, iter_lrc_cost=0)
+
+class TestRegionDictObjectKeys(BaseTestRegionDictKeys):
     class Key:
         pass
 
@@ -182,11 +264,26 @@ def test_wb_copy(self):
         # LRC increase of 2: 1x for the key 1x for the item
         self.check_dict_item_access(self.Key(), lambda dict, key: dict.copy(), lrc_offset = 2)
 
-    @unittest.expectedFailure # Broken until WBs in lists have been added
+    @unittest.expectedFailure # FIXME(regions): xFrednet: Broken until WBs in lists have been added
     def test_wb_key_list(self):
         self.check_dict_item_access(self.Key(), lambda dict, key: list(dict))
 
+    def test_wb_keys_view(self):
+        self.check_dict_view(self.Key(), self.Key(), lambda d: d.keys(), 1)
+
+    def test_wb_values_view(self):
+        self.check_dict_view(self.Key(), self.Key(), lambda d: d.values(), 1)
 
+    @unittest.expectedFailure # FIXME(regions): xFrednet: Broken until WBs in tuples have been added
+    def test_wb_items_view(self):
+        self.check_dict_view(self.Key(), self.Key(), lambda d: d.items(), 2)
+
+    def test_wb_iter_dict(self):
+        self.check_dict_view(self.Key(), self.Key(), lambda d: d, loop_lrc_effect=1)
+
+    def test_wb_iter_reversed_dict(self):
+        # The iterator doesn't effect the LRC, since we create it with the `reversed`
+        self.check_dict_view(self.Key(), self.Key(), lambda d: reversed(d), loop_lrc_effect=1, iter_lrc_cost=0)
 
 # TODO: Construction: dict(one=1, two=2, three=3)
 # TODO: Construction: {'one': 1, 'two': 2, 'three': 3}
@@ -195,18 +292,16 @@ def test_wb_key_list(self):
 # TODO: Construction: dict({'three': 3, 'one': 1, 'two': 2})
 # TODO: Construction: dict({'one': 1, 'three': 3}, two=2)
 
-
+# FIXME(regions): xFrednet: Set operations on views, like `&`, `^` and `|`
+#                           are currently not tested and probably don't work.
 
 # TODO: classmethod fromkeys(iterable, value=None, /)
-# TODO: dict.items()
-# TODO: dict.keys()
 # TODO: dict.pop(key)
 # TODO: dict.pop(key, default)
 # TODO: dict.popitem(key)
 # TODO: reversed(dict)
 # TODO: dict.setdefault(key)
 # TODO: dict.setdefault(key, default)
 # TODO: dict.update(???)
-# TODO: dict.values()
 # TODO: dict1 | dict2
 # TODO: dict1 |= dict2

Objects/descrobject.c

@@ -1189,7 +1189,7 @@ mappingproxy_dealloc(PyObject *self)
 {
     mappingproxyobject *pp = (mappingproxyobject *)self;
     _PyObject_GC_UNTRACK(pp);
-    Py_DECREF(pp->mapping);
+    PyRegion_CLEAR(pp, pp->mapping);
     PyObject_GC_Del(pp);
 }
 

Objects/dictobject.c

@@ -5365,6 +5365,10 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
     if (di == NULL) {
         return NULL;
     }
+    if (PyRegion_AddRef(di, dict)) {
+        Py_DECREF(dict);
+        return NULL;
+    }
     di->di_dict = (PyDictObject*)Py_NewRef(dict);
     used = FT_ATOMIC_LOAD_SSIZE_RELAXED(dict->ma_used);
     di->di_used = used;
@@ -5403,8 +5407,8 @@ dictiter_dealloc(PyObject *self)
     dictiterobject *di = (dictiterobject *)self;
     /* bpo-31095: UnTrack is needed before calling any callbacks */
     _PyObject_GC_UNTRACK(di);
-    Py_XDECREF(di->di_dict);
-    Py_XDECREF(di->di_result);
+    PyRegion_CLEAR(di, di->di_dict);
+    PyRegion_CLEAR(di, di->di_result);
     PyObject_GC_Del(di);
 }
 
@@ -5510,10 +5514,11 @@ dictiter_iternextkey_lock_held(PyDictObject *d, PyObject *self)
     }
     di->di_pos = i+1;
     di->len--;
-    return Py_NewRef(key);
+    return PyRegion_NewRef(key);
 
 fail:
     di->di_dict = NULL;
+    PyRegion_RemoveLocalRef(d);
     Py_DECREF(d);
     return NULL;
 }
@@ -5633,10 +5638,11 @@ dictiter_iternextvalue_lock_held(PyDictObject *d, PyObject *self)
     }
     di->di_pos = i+1;
     di->len--;
-    return Py_NewRef(value);
+    return PyRegion_NewRef(value);
 
 fail:
     di->di_dict = NULL;
+    PyRegion_RemoveLocalRef(d);
     Py_DECREF(d);
     return NULL;
 }
@@ -5760,15 +5766,16 @@ dictiter_iternextitem_lock_held(PyDictObject *d, PyObject *self,
     di->di_pos = i+1;
     di->len--;
     if (out_key != NULL) {
-        *out_key = Py_NewRef(key);
+        *out_key = PyRegion_NewRef(key);
     }
     if (out_value != NULL) {
-        *out_value = Py_NewRef(value);
+        *out_value = PyRegion_NewRef(value);
     }
     return 0;
 
 fail:
     di->di_dict = NULL;
+    PyRegion_RemoveLocalRef(d);
     Py_DECREF(d);
     return -1;
 }
@@ -5927,6 +5934,7 @@ static bool
 acquire_iter_result(PyObject *result)
 {
     if (has_unique_reference(result)) {
+        PyRegion_AddLocalRef(result);
         Py_INCREF(result);
         return true;
     }
@@ -5950,11 +5958,13 @@ dictiter_iternextitem(PyObject *self)
 
 #endif
         PyObject *result = di->di_result;
-        if (acquire_iter_result(result)) {
+        if (acquire_iter_result(result) && PyRegion_IsLocal(result)) {
             PyObject *oldkey = PyTuple_GET_ITEM(result, 0);
             PyObject *oldvalue = PyTuple_GET_ITEM(result, 1);
             PyTuple_SET_ITEM(result, 0, key);
             PyTuple_SET_ITEM(result, 1, value);
+            PyRegion_RemoveLocalRef(oldkey);
+            PyRegion_RemoveLocalRef(oldvalue);
             Py_DECREF(oldkey);
             Py_DECREF(oldvalue);
             // bpo-42536: The GC may have untracked this result tuple. Since we're
@@ -6065,19 +6075,25 @@ dictreviter_iter_lock_held(PyDictObject *d, PyObject *self)
     di->len--;
 
     if (Py_IS_TYPE(di, &PyDictRevIterKey_Type)) {
-        return Py_NewRef(key);
+        return PyRegion_NewRef(key);
     }
     else if (Py_IS_TYPE(di, &PyDictRevIterValue_Type)) {
-        return Py_NewRef(value);
+        return PyRegion_NewRef(value);
     }
     else if (Py_IS_TYPE(di, &PyDictRevIterItem_Type)) {
         result = di->di_result;
-        if (Py_REFCNT(result) == 1) {
+        if (Py_REFCNT(result) == 1 && PyRegion_IsLocal(result)) {
             PyObject *oldkey = PyTuple_GET_ITEM(result, 0);
             PyObject *oldvalue = PyTuple_GET_ITEM(result, 1);
+            if (PyRegion_AddRefs(result, key, value)) {
+                Py_DECREF(result);
+                return NULL;
+            }
             PyTuple_SET_ITEM(result, 0, Py_NewRef(key));
             PyTuple_SET_ITEM(result, 1, Py_NewRef(value));
             Py_INCREF(result);
+            PyRegion_RemoveLocalRef(oldkey);
+            PyRegion_RemoveLocalRef(oldvalue);
             Py_DECREF(oldkey);
             Py_DECREF(oldvalue);
             // bpo-42536: The GC may have untracked this result tuple. Since
@@ -6089,6 +6105,10 @@ dictreviter_iter_lock_held(PyDictObject *d, PyObject *self)
             if (result == NULL) {
                 return NULL;
             }
+            if (PyRegion_AddRefs(result, key, value)) {
+                Py_DECREF(result);
+                return NULL;
+            }
             PyTuple_SET_ITEM(result, 0, Py_NewRef(key));
             PyTuple_SET_ITEM(result, 1, Py_NewRef(value));
         }
@@ -6199,7 +6219,7 @@ dictview_dealloc(PyObject *self)
     _PyDictViewObject *dv = (_PyDictViewObject *)self;
     /* bpo-31095: UnTrack is needed before calling any callbacks */
     _PyObject_GC_UNTRACK(dv);
-    Py_XDECREF(dv->dv_dict);
+    PyRegion_CLEAR(dv, dv->dv_dict);
     PyObject_GC_Del(dv);
 }