8000 [3.13] gh-120906: Support arbitrary hashable keys in FrameLocalsProxy… · python/cpython@8c8c43e · GitHub
[go: up one dir, main page]
Skip to content

Commit 8c8c43e

Browse files
encukouncoghlan
encukou
and
ncoghlan
authored
[3.13] gh-120906: Support arbitrary hashable keys in FrameLocalsProxy (GH-122309) (#122488)
[3.13] gh-120906: Support arbitrary hashable keys in FrameLocalsProxy (GH-122309) Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com> (cherry picked from commit 5912487)
1 parent 927bd9e commit 8c8c43e

File tree

3 files changed

+204
-56
lines changed

3 files changed

+204
-56
lines changed

‎Lib/test/test_frame.py

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from test import support
1717
from test.support import import_helper, threading_helper, Py_GIL_DISABLED
1818
from test.support.script_helper import assert_python_ok
19+
from test import mapping_tests
1920

2021

2122
class ClearTest(unittest.TestCase):
@@ -421,6 +422,132 @@ def test_unsupport(self):
421422
copy.deepcopy(d)
422423

423424

425+
def _x_stringlikes(self):
426+
class StringSubclass(str):
427+
pass
428+
429+
class ImpostorX:
430+
def __hash__(self):
431+
return hash('x')
432+
433+
def __eq__(self, other):
434+
return other == 'x'
435+
436+
return StringSubclass('x'), ImpostorX(), 'x'
437+
438+
def test_proxy_key_stringlikes_overwrite(self):
439+
def f(obj):
440+
x = 1
441+
proxy = sys._getframe().f_locals
442+
proxy[obj] = 2
443+
return (
444+
list(proxy.keys()),
445+
dict(proxy),
446+
proxy
447+
)
448+
449+
for obj in self._x_stringlikes():
450+
with self.subTest(cls=type(obj).__name__):
451+
452+
keys_snapshot, proxy_snapshot, proxy = f(obj)
453+
expected_keys = ['obj', 'x', 'proxy']
454+
expected_dict = {'obj': 'x', 'x': 2, 'proxy': proxy}
455+
self.assertEqual(proxy.keys(), expected_keys)
456+
self.assertEqual(proxy, expected_dict)
457+
self.assertEqual(keys_snapshot, expected_keys)
458+
self.assertEqual(proxy_snapshot, expected_dict)
459+
460+
def test_proxy_key_stringlikes_ftrst_write(self):
461+
def f(obj):
462+
proxy = sys._getframe().f_locals
463+
proxy[obj] = 2
464+
self.assertEqual(x, 2)
465+
x = 1
466+
467+
for obj in self._x_stringlikes():
468+
with self.subTest(cls=type(obj).__name__):
469+
f(obj)
470+
471+
def test_proxy_key_unhashables(self):
472+
class StringSubclass(str):
473+
__hash__ = None
474+
475+
class ObjectSubclass:
476+
__hash__ = None
477+
478+
proxy = sys._getframe().f_locals
479+
480+
for obj in StringSubclass('x'), ObjectSubclass():
481+
with self.subTest(cls=type(obj).__name__):
482+
with self.assertRaises(TypeError):
483+
proxy[obj]
484+
with self.assertRaises(TypeError):
485+
proxy[obj] = 0
486+
487+
488+
class FrameLocalsProxyMappingTests(mapping_tests.TestHashMappingProtocol):
489+
"""Test that FrameLocalsProxy behaves like a Mapping (with exceptions)"""
490+
491+
def _f(*args, **kwargs):
492+
def _f():
493+
return sys._getframe().f_locals
494+
return _f()
495+
type2test = _f
496+
497+
@unittest.skipIf(True, 'Locals proxies for different frames never compare as equal')
498+
def test_constructor(self):
499+
pass
500+
501+
@unittest.skipIf(True, 'Unlike a mapping: del proxy[key] fails')
502+
def test_write(self):
503+
pass
504+
505+
@unittest.skipIf(True, 'Unlike a mapping: no proxy.popitem')
506+
def test_popitem(self):
507+
pass
508+
509+
@unittest.skipIf(True, 'Unlike a mapping: no proxy.pop')
510+
def test_pop(self):
511+
pass
512+
513+
@unittest.skipIf(True, 'Unlike a mapping: no proxy.clear')
514+
def test_clear(self):
515+
pass
516+
517+
@unittest.skipIf(True, 'Unlike a mapping: no proxy.fromkeys')
518+
def test_fromkeys(self):
519+
pass
520+
521+
# no del
522+
def test_getitem(self):
523+
mapping_tests.BasicTestMappingProtocol.test_getitem(self)
524+
d = self._full_mapping({'a': 1, 'b': 2})
525+
self.assertEqual(d['a'], 1)
526+
self.assertEqual(d['b'], 2)
527+
d['c'] = 3
528+
d['a'] = 4
529+
self.assertEqual(d['c'], 3)
530+
self.assertEqual(d['a'], 4)
531+
532+
@unittest.skipIf(True, 'Unlike a mapping: no proxy.update')
533+
def test_update(self):
534+
pass
535+
536+
# proxy.copy returns a regular dict
537+
def test_copy(self):
538+
d = self._full_mapping({1:1, 2:2, 3:3})
539+
self.assertEqual(d.copy(), {1:1, 2:2, 3:3})
540+
d = self._empty_mapping()
541+
self.assertEqual(d.copy(), d)
542+
self.assertRaises(TypeError, d.copy, None)
543+
544+
self.assertIsInstance(d.copy(), dict)
545+
546+
@unittest.skipIf(True, 'Locals proxies for different frames never compare as equal')
547+
def test_eq(self):
548+
pass
549+
550+
424551
class TestFrameCApi(unittest.TestCase):
425552
def test_basic(self):
426553
x = 1

‎Misc/NEWS.d/next/Core_and_Builtins/2024-07-26-13-56-32.gh-issue-120906.qBh2I9.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:attr:`frame.f_locals` now supports arbitrary hashable objects as keys.

‎Objects/frameobject.c

Lines changed: 76 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,27 @@ static int
5353
framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject* key, bool read)
5454
{
5555
/*
56-
* Returns the fast locals index of the key
56+
* Returns -2 (!) if an error occurred; exception will be set.
57+
* Returns the fast locals index of the key on success:
5758
* - if read == true, returns the index if the value is not NULL
5859
* - if read == false, returns the index if the value is not hidden
60+
* Otherwise returns -1.
5961
*/
6062

61-
assert(PyUnicode_CheckExact(key));
62-
6363
PyCodeObject *co = _PyFrame_GetCode(frame->f_frame);
64-
int found_key = false;
64+
65+
// Ensure that the key is hashable.
66+
Py_hash_t key_hash = PyObject_Hash(key);
67+
if (key_hash == -1) {
68+
return -2;
69+
}
70+
bool found = false;
6571

6672
// We do 2 loops here because it's highly possible the key is interned
6773
// and we can do a pointer comparison.
6874
for (int i = 0; i < co->co_nlocalsplus; i++) {
6975
PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
7076
if (name == key) {
71-
found_key = true;
7277
if (read) {
7378
if (framelocalsproxy_getval(frame->f_frame, co, i) != NULL) {
7479
return i;
@@ -78,23 +83,35 @@ framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject* key, bool read)
7883
return i;
7984
}
8085
}
86+
found = true;
8187
}
8288
}
83-
84-
if (!found_key) {
85-
// This is unlikely, but we need to make sure. This means the key
86-
// is not interned.
87-
for (int i = 0; i < co->co_nlocalsplus; i++) {
88-
PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
89-
if (_PyUnicode_EQ(name, key)) {
90-
if (read) {
91-
if (framelocalsproxy_getval(frame->f_frame, co, i) != NULL) {
92-
return i;
93-
}
94-
} else {
95-
if (!(_PyLocals_GetKind(co->co_localspluskinds, i) & CO_FAST_HIDDEN)) {
96-
return i;
97-
}
89+
if (found) {
90+
// This is an attempt to read an unset local variable or
91+
// write to a variable that is hidden from regular write operations
92+
return -1;
93+
}
94+
// This is unlikely, but we need to make sure. This means the key
95+
// is not interned.
96+
for (int i = 0; i < co->co_nlocalsplus; i++) {
97+
PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
98+
Py_hash_t name_hash = PyObject_Hash(name);
99+
assert(name_hash != -1); // keys are exact unicode
100+
if (name_hash != key_hash) {
101+
continue;
102+
}
103+
int same = PyObject_RichCompareBool(name, key, Py_EQ);
104+
if (same < 0) {
105+
return -2;
106+
}
107+
if (same) {
108+
if (read) {
109+
if (framelocalsproxy_getval(frame->f_frame, co, i) != NULL) {
110+
return i;
111+
}
112+
} else {
113+
if (!(_PyLocals_GetKind(co->co_localspluskinds, i) & CO_FAST_HIDDEN)) {
114+
return i;
98115
}
99116
}
100117
}
@@ -109,13 +126,14 @@ framelocalsproxy_getitem(PyObject *self, PyObject *key)
109126
PyFrameObject* frame = ((PyFrameLocalsProxyObject*)self)->frame;
110127
PyCodeObject* co = _PyFrame_GetCode(frame->f_frame);
111128

112-
if (PyUnicode_CheckExact(key)) {
113-
int i = framelocalsproxy_getkeyindex(frame, key, true);
114-
if (i >= 0) {
115-
PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
116-
assert(value != NULL);
117-
return Py_NewRef(value);
118-
}
129+
int i = framelocalsproxy_getkeyindex(frame, key, true);
130+
if (i == -2) {
131+
return NULL;
132+
}
133+
if (i >= 0) {
134+
PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
135+
assert(value != NULL);
136+
return Py_NewRef(value);
119137
}
120138

121139
// Okay not in the fast locals, try extra locals
@@ -145,35 +163,36 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value)
145163
return -1;
146164
}
147165

148-
if (PyUnicode_CheckExact(key)) {
149-
int i = framelocalsproxy_getkeyindex(frame, key, false);
150-
if (i >= 0) {
151-
_Py_Executors_InvalidateDependency(PyInterpreterState_Get(), co, 1);
166+
int i = framelocalsproxy_getkeyindex(frame, key, false);
167+
if (i == -2) {
168+
return -1;
169+
}
170+
if (i >= 0) {
171+
_Py_Executors_InvalidateDependency(PyInterpreterState_Get(), co, 1);
152172

153-
_PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
154-
PyObject *oldvalue = fast[i];
155-
PyObject *cell = NULL;
156-
if (kind == CO_FAST_FREE) {
157-
// The cell was set when the frame was created from
158-
// the function's closure.
159-
assert(oldvalue != NULL && PyCell_Check(oldvalue));
173+
_PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
174+
PyObject *oldvalue = fast[i];
175+
PyObject *cell = NULL;
176+
if (kind == CO_FAST_FREE) {
177+
// The cell was set when the frame was created from
178+
// the function's closure.
179+
assert(oldvalue != NULL && PyCell_Check(oldvalue));
180+
cell = oldvalue;
181+
} else if (kind & CO_FAST_CELL && oldvalue != NULL) {
182+
if (PyCell_Check(oldvalue)) {
160183
cell = oldvalue;
161-
} else if (kind & CO_FAST_CELL && oldvalue != NULL) {
162-
if (PyCell_Check(oldvalue)) {
163-
cell = oldvalue;
164-
}
165184
}
166-
if (cell != NULL) {
167-
oldvalue = PyCell_GET(cell);
168-
if (value != oldvalue) {
169-
PyCell_SET(cell, Py_XNewRef(value));
170-
Py_XDECREF(oldvalue);
171-
}
172-
} else if (value != oldvalue) {
173-
Py_XSETREF(fast[i], Py_NewRef(value));
185+
}
186+
if (cell != NULL) {
187+
oldvalue = PyCell_GET(cell);
188+
if (value != oldvalue) {
189+
PyCell_SET(cell, Py_XNewRef(value));
190+
Py_XDECREF(oldvalue);
174191
}
175-
return 0;
192+
} else if (value != oldvalue) {
193+
Py_XSETREF(fast[i], Py_NewRef(value));
176194
}
195+
return 0;
177196
}
178197

179198
// Okay not in the fast locals, try extra locals
@@ -543,11 +562,12 @@ framelocalsproxy_contains(PyObject *self, PyObject *key)
543562
{
544563
PyFrameObject *frame = ((PyFrameLocalsProxyObject*)self)->frame;
545564

546-
if (PyUnicode_CheckExact(key)) {
547-
int i = framelocalsproxy_getkeyindex(frame, key, true);
548-
if (i >= 0) {
549-
return 1;
550-
}
565+
int i = framelocalsproxy_getkeyindex(frame, key, true);
566+
if (i == -2) {
567+
return -1;
568+
}
569+
if (i >= 0) {
570+
return 1;
551571
}
552572

553573
PyObject *extra = ((PyFrameObject*)frame)->f_extra_locals;

0 commit comments

Comments
 (0)
0