diff --git a/Include/cpython/code.h b/Include/cpython/code.h index 5238447070ee54..c958f4d05a6cc5 100644 --- a/Include/cpython/code.h +++ b/Include/cpython/code.h @@ -107,6 +107,10 @@ typedef struct { struct _PyCoMonitoringData *_co_monitoring; /* Monitoring data */ \ _PyCode_DEF_UNIQUE_ID() \ int _co_firsttraceable; /* index of first traceable instruction */ \ + /* 0 = not yet known, 1 = co_localsplusnames are unique, 2 = a name is \ + repeated. Learned from bound locals; deduplication is skipped when \ + this is 1. */ \ + char _co_localsplus_dedup; \ /* Scratch space for extra data relating to the code object. \ Type is a void* to keep the format private in codeobject.c to force \ people to go through the proper APIs. */ \ diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index aeb2ad06ad07cb..16e2ccf513bc7f 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -273,6 +273,12 @@ extern PyObject* _PyCode_GetCellvars(PyCodeObject *); extern PyObject* _PyCode_GetFreevars(PyCodeObject *); extern PyObject* _PyCode_GetCode(PyCodeObject *); +/* co->_co_localsplus_dedup. Learned from bound locals. UNIQUE is stored + only after a walk sees every slot bound without a repeated name. */ +#define _PYCODE_LOCALSPLUS_DEDUP_UNKNOWN 0 +#define _PYCODE_LOCALSPLUS_DEDUP_UNIQUE 1 +#define _PYCODE_LOCALSPLUS_DEDUP_NEEDED 2 + /** API for initializing the line number tables. */ PyAPI_FUNC(int) _PyCode_InitAddressRange(PyCodeObject* co, PyCodeAddressRange *bounds); diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py index f02f3223a513d2..047e3a3cef6fef 100644 --- a/Lib/test/test_listcomps.py +++ b/Lib/test/test_listcomps.py @@ -944,6 +944,35 @@ def inner(): {"snaps": [1, 2], "vals": [2, 2], "consistent": [True, True]}, ns={"sys": sys}, scopes=["module", "function"]) + def test_frame_locals_dedup_learned_before_slot_is_bound(self): + # Reading f_locals before the comprehension slot is bound must not + # cache "names are unique": the repeated name is still on the code + # object, and dict(**f_locals) inside the comprehension has to work. + # The enclosing x is already bound, so the first read shows that + # value. Each API runs on its own code object. + probes = { + "keys": ("list(sys._getframe().f_locals.keys())", ["x"]), + "values": ("list(sys._getframe().f_locals.values())", [[7, 8]]), + "items": ("list(sys._getframe().f_locals.items())", + [("x", [7, 8])]), + "len": ("len(sys._getframe().f_locals)", 1), + } + import sys + for name, (probe, expected) in probes.items(): + with self.subTest(probe=name): + code = f""" + def outer(x): + def inner(): + probe = {probe} + return probe, [dict(**sys._getframe().f_locals)['x'] + for x in x] + return inner() + probe, result = outer([7, 8]) + """ + self._check_in_scopes( + code, {"probe": expected, "result": [7, 8]}, + ns={"sys": sys}, scopes=["module", "function"]) + def test_frame_locals_nested_comp_cell_and_enclosing_free(self): # Stress a nested inlined shape where a comp cell and enclosing free # share a name; all f_locals views must stay consistent. diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 724243d2b8cfae..0dad125ccffe20 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -561,6 +561,7 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con) /* not set */ co->co_weakreflist = NULL; co->co_extra = NULL; + co->_co_localsplus_dedup = _PYCODE_LOCALSPLUS_DEDUP_UNKNOWN; co->_co_cached = NULL; co->co_executors = NULL; diff --git a/Objects/frameobject.c b/Objects/frameobject.c index a4cc14a6eaad45..7187465a175214 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -94,14 +94,71 @@ framelocalsproxy_hasval(_PyInterpreterFrame *frame, PyCodeObject *co, int i) return true; } +/* *seen is NULL when dedup is unnecessary. Otherwise it is the set of bound + names already kept. Returns -1 if the set cannot be created. */ static int -framelocalsproxy_is_first_occurrence(PyObject *seen, PyObject *name) +framelocalsproxy_prepare_dedup(PyCodeObject *co, PyObject **seen) { + *seen = NULL; + char state = FT_ATOMIC_LOAD_CHAR_RELAXED(co->_co_localsplus_dedup); + if (state == _PYCODE_LOCALSPLUS_DEDUP_UNIQUE) { + return 0; + } + /* A single name cannot be repeated. More names stay UNKNOWN until a + walk sees every slot bound without a repeat. */ + if (state == _PYCODE_LOCALSPLUS_DEDUP_UNKNOWN && co->co_nlocalsplus <= 1) { + FT_ATOMIC_STORE_CHAR_RELAXED(co->_co_localsplus_dedup, + _PYCODE_LOCALSPLUS_DEDUP_UNIQUE); + return 0; + } + *seen = PySet_New(NULL); + if (*seen == NULL) { + return -1; + } + return 0; +} + +/* A walk that saw every slot bound, and no repeated bound name, has shown + that co_localsplusnames are unique. A sparse walk leaves the cache + UNKNOWN: an unbound slot may still repeat a name. */ +static void +framelocalsproxy_finish_dedup(PyCodeObject *co, int all_bound) +{ + if (all_bound && + FT_ATOMIC_LOAD_CHAR_RELAXED(co->_co_localsplus_dedup) == + _PYCODE_LOCALSPLUS_DEDUP_UNKNOWN) + { + FT_ATOMIC_STORE_CHAR_RELAXED(co->_co_localsplus_dedup, + _PYCODE_LOCALSPLUS_DEDUP_UNIQUE); + } +} + +/* Returns 1 if this slot's value should be kept, 0 if not, -1 on error. + Empty slots are not kept. The first bound value for a name wins. + *all_bound is cleared when a slot is empty. A repeated bound name + stores NEEDED. */ +static int +framelocalsproxy_visit_name(PyCodeObject *co, int i, PyObject *seen, + int has_value, int *all_bound) +{ + if (!has_value) { + *all_bound = 0; + return 0; + } + if (seen == NULL) { + return 1; + } + PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); int found = PySet_Contains(seen, name); if (found < 0) { return -1; } if (found) { + if (FT_ATOMIC_LOAD_CHAR_RELAXED(co->_co_localsplus_dedup) == + _PYCODE_LOCALSPLUS_DEDUP_UNKNOWN) { + FT_ATOMIC_STORE_CHAR_RELAXED(co->_co_localsplus_dedup, + _PYCODE_LOCALSPLUS_DEDUP_NEEDED); + } return 0; } if (PySet_Add(seen, name) < 0) { @@ -396,28 +453,31 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored)) if (names == NULL) { return NULL; } - // An inlined comprehension cell can share a name with a free var. - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { + // An inlined comprehension local can share a name with a free var. + PyObject *seen = NULL; + if (framelocalsproxy_prepare_dedup(co, &seen) < 0) { Py_DECREF(names); return NULL; } + int all_bound = 1; for (int i = 0; i < co->co_nlocalsplus; i++) { - if (framelocalsproxy_hasval(frame->f_frame, co, i)) { + int has = framelocalsproxy_hasval(frame->f_frame, co, i); + int emit = framelocalsproxy_visit_name(co, i, seen, has, &all_bound); + if (emit < 0) { + goto error; + } + if (emit) { PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); - int first = framelocalsproxy_is_first_occurrence(seen, name); - if (first < 0) { + if (PyList_Append(names, name) < 0) { goto error; } - if (first) { - if (PyList_Append(names, name) < 0) { - goto error; - } - } } } - Py_DECREF(seen); + if (seen != NULL) { + framelocalsproxy_finish_dedup(co, all_bound); + Py_DECREF(seen); + } // Iterate through the extra locals if (frame->f_extra_locals) { @@ -438,7 +498,7 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored)) return names; error: - Py_DECREF(seen); + Py_XDECREF(seen); Py_DECREF(names); return NULL; } @@ -622,30 +682,31 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored)) if (values == NULL) { return NULL; } - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { + PyObject *seen = NULL; + if (framelocalsproxy_prepare_dedup(co, &seen) < 0) { Py_DECREF(values); return NULL; } + int all_bound = 1; for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i); - if (value) { - 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; - } - } + int has = value != NULL; + int emit = framelocalsproxy_visit_name(co, i, seen, has, &all_bound); + if (emit < 0) { + Py_XDECREF(value); + goto error; + } + if (emit && PyList_Append(values, value) < 0) { Py_DECREF(value); - if (first < 0) { - goto error; - } + goto error; } + Py_XDECREF(value); + } + if (seen != NULL) { + framelocalsproxy_finish_dedup(co, all_bound); + Py_DECREF(seen); } - Py_DECREF(seen); // Iterate through the extra locals if (frame->f_extra_locals) { @@ -663,7 +724,7 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored)) return values; error: - Py_DECREF(seen); + Py_XDECREF(seen); Py_DECREF(values); return NULL; } @@ -677,37 +738,39 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored)) if (items == NULL) { return NULL; } - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { + PyObject *seen = NULL; + if (framelocalsproxy_prepare_dedup(co, &seen) < 0) { Py_DECREF(items); return NULL; } + int all_bound = 1; 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) { - 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; - } - } - else { - Py_DECREF(value); - if (first < 0) { - goto error; - } - } + int has = value != NULL; + int emit = framelocalsproxy_visit_name(co, i, seen, has, &all_bound); + if (emit < 0) { + Py_XDECREF(value); + goto error; + } + if (!emit) { + Py_XDECREF(value); + continue; } + PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); + PyObject *pair = _PyTuple_FromPairSteal(Py_NewRef(name), value); + if (pair == NULL) { + goto error; + } + if (_PyList_AppendTakeRef((PyListObject *)items, pair) < 0) { + goto error; + } + } + if (seen != NULL) { + framelocalsproxy_finish_dedup(co, all_bound); + Py_DECREF(seen); + seen = NULL; } - Py_DECREF(seen); - seen = NULL; // Iterate through the extra locals if (frame->f_extra_locals) { @@ -746,24 +809,26 @@ framelocalsproxy_length(PyObject *self) size += PyDict_Size(frame->f_extra_locals); } - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { + PyObject *seen = NULL; + if (framelocalsproxy_prepare_dedup(co, &seen) < 0) { return -1; } + int all_bound = 1; 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 first = framelocalsproxy_is_first_occurrence(seen, name); - if (first < 0) { - Py_DECREF(seen); - return -1; - } - else if (first) { - size++; - } + int has = framelocalsproxy_hasval(frame->f_frame, co, i); + int emit = framelocalsproxy_visit_name(co, i, seen, has, &all_bound); + if (emit < 0) { + Py_XDECREF(seen); + return -1; + } + if (emit) { + size++; } } - Py_DECREF(seen); + if (seen != NULL) { + framelocalsproxy_finish_dedup(co, all_bound); + Py_DECREF(seen); + } return size; }