dedup keys(), values(), items(), and len() on frame

Author: iritkatriel

Lib/test/test_listcomps.py

@@ -811,19 +811,35 @@ def test_frame_locals(self):
     def test_frame_locals_comp_cell_and_enclosing_free(self):
         # The inlined listcomp cell and the enclosing free share a name.
         # f_locals keys must still be unique so dict(**f_locals) works.
+        # keys(), values(), items(), and len() must agree (first slot wins).
         code = """
             def outer(x):
                 def inner():
-                    return [(lambda: x, dict(**sys._getframe().f_locals))
+                    return [(lambda: x,
+                             dict(**sys._getframe().f_locals),
+                             len(sys._getframe().f_locals),
+                             list(sys._getframe().f_locals.keys()),
+                             list(sys._getframe().f_locals.values()),
+                             list(sys._getframe().f_locals.items()),
+                             dict(sys._getframe().f_locals.items()))
                             for x in x]
                 return inner()
             result = outer([1, 2])
-            snaps = [d['x'] for _, d in result]
-            vals = [fn() for fn, _ in result]
+            snaps = [d['x'] for _, d, *_ in result]
+            vals = [fn() for fn, *_ in result]
+            consistent = []
+            for _, d, n, ks, vs, it, d_items in result:
+                consistent.append(
+                    n == len(ks) == len(vs) == len(it)
+                    and ks.count('x') == 1
+                    and d == d_items == dict(zip(ks, vs))
+                )
         """
         import sys
-        self._check_in_scopes(code, {"snaps": [1, 2], "vals": [2, 2]},
-                              ns={"sys": sys}, scopes=["module", "function"])
+        self._check_in_scopes(
+            code,
+            {"snaps": [1, 2], "vals": [2, 2], "consistent": [True, True]},
+            ns={"sys": sys}, scopes=["module", "function"])
 
     def _recursive_replace(self, maybe_code):
         if not isinstance(maybe_code, types.CodeType):

Objects/frameobject.c

@@ -94,6 +94,22 @@ framelocalsproxy_hasval(_PyInterpreterFrame *frame, PyCodeObject *co, int i)
     return true;
 }
 
+static int
+framelocalsproxy_is_first_occurrence(PyObject *seen, PyObject *name)
+{
+    int found = PySet_Contains(seen, name);
+    if (found < 0) {
+        return -1;
+    }
+    if (found) {
+        return 0;
+    }
+    if (PySet_Add(seen, name) < 0) {
+        return -1;
+    }
+    return 1;
+}
+
 static int
 framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject *key, bool read, PyObject **value_ptr)
 {
@@ -390,18 +406,14 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored))
     for (int i = 0; i < co->co_nlocalsplus; i++) {
         if (framelocalsproxy_hasval(frame->f_frame, co, i)) {
             PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
-            int found = PySet_Contains(seen, name);
-            if (found < 0) {
+            int first = framelocalsproxy_is_first_occurrence(seen, name);
+            if (first < 0) {
                 goto error;
             }
-            if (found) {
-                continue;
-            }
-            if (PySet_Add(seen, name) < 0) {
-                goto error;
-            }
-            if (PyList_Append(names, name) < 0) {
-                goto error;
+            if (first) {
+                if (PyList_Append(names, name) < 0) {
+                    goto error;
+                }
             }
         }
     }
@@ -610,18 +622,30 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored))
     if (values == NULL) {
         return NULL;
     }
+    PyObject *seen = PySet_New(NULL);
+    if (seen == NULL) {
+        Py_DECREF(values);
+        return NULL;
+    }
 
     for (int i = 0; i < co->co_nlocalsplus; i++) {
         PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
         if (value) {
-            if (PyList_Append(values, value) < 0) {
-                Py_DECREF(values);
-                Py_DECREF(value);
-                return NULL;
+            PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
+            int first = framelocalsproxy_is_first_occurrence(seen, name);
+            if (first == 1) {
+                if (PyList_Append(values, value) < 0) {
+                    Py_DECREF(value);
+                    goto error;
+                }
             }
             Py_DECREF(value);
+            if (first < 0) {
+                goto error;
+            }
         }
     }
+    Py_DECREF(seen);
 
     // Iterate through the extra locals
     if (frame->f_extra_locals) {
@@ -637,6 +661,11 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored))
     }
 
     return values;
+
+error:
+    Py_DECREF(seen);
+    Py_DECREF(values);
+    return NULL;
 }
 
 static PyObject *
@@ -648,22 +677,37 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored))
     if (items == NULL) {
         return NULL;
     }
+    PyObject *seen = PySet_New(NULL);
+    if (seen == NULL) {
+        Py_DECREF(items);
+        return NULL;
+    }
 
     for (int i = 0; i < co->co_nlocalsplus; i++) {
         PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
         PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
 
         if (value) {
-            PyObject *pair = _PyTuple_FromPairSteal(Py_NewRef(name), value);
-            if (pair == NULL) {
-                goto error;
+            int first = framelocalsproxy_is_first_occurrence(seen, name);
+            if (first == 1) {
+                PyObject *pair = _PyTuple_FromPairSteal(Py_NewRef(name), value);
+                if (pair == NULL) {
+                    goto error;
+                }
+                if (_PyList_AppendTakeRef((PyListObject *)items, pair) < 0) {
+                    goto error;
+                }
             }
-
-            if (_PyList_AppendTakeRef((PyListObject *)items, pair) < 0) {
-                goto error;
+            else {
+                Py_DECREF(value);
+                if (first < 0) {
+                    goto error;
+                }
             }
         }
     }
+    Py_DECREF(seen);
+    seen = NULL;
 
     // Iterate through the extra locals
     if (frame->f_extra_locals) {
@@ -685,6 +729,7 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored))
     return items;
 
 error:
+    Py_XDECREF(seen);
     Py_DECREF(items);
     return NULL;
 }
@@ -701,11 +746,24 @@ framelocalsproxy_length(PyObject *self)
         size += PyDict_Size(frame->f_extra_locals);
     }
 
+    PyObject *seen = PySet_New(NULL);
+    if (seen == NULL) {
+        return -1;
+    }
     for (int i = 0; i < co->co_nlocalsplus; i++) {
         if (framelocalsproxy_hasval(frame->f_frame, co, i)) {
-            size++;
+            PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
+            int first = framelocalsproxy_is_first_occurrence(seen, name);
+            if (first < 0) {
+                Py_DECREF(seen);
+                return -1;
+            }
+            else if (first) {
+                size++;
+            }
         }
     }
+    Py_DECREF(seen);
     return size;
 }