Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 5912487

Browse filesBrowse files
encukouncoghlan
andauthored
gh-120906: Support arbitrary hashable keys in FrameLocalsProxy (GH-122309)
Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
1 parent 1cac090 commit 5912487
Copy full SHA for 5912487

File tree

3 files changed

+208
-60
lines changed
Filter options

3 files changed

+208
-60
lines changed

‎Lib/test/test_frame.py

Copy file name to clipboardExpand all lines: Lib/test/test_frame.py
+127Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from test import support
1616
from test.support import import_helper, threading_helper
1717
from test.support.script_helper import assert_python_ok
18+
from test import mapping_tests
1819

1920

2021
class ClearTest(unittest.TestCase):
@@ -431,6 +432,132 @@ def test_is_mapping(self):
431432
kind = "other"
432433
self.assertEqual(kind, "mapping")
433434

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

Copy file name to clipboardExpand all lines: Objects/frameobject.c
+80-60Lines changed: 80 additions & 60 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,37 +163,38 @@ 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);
152-
153-
_PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
154-
_PyStackRef 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.bits != 0 && PyCell_Check(PyStackRef_AsPyObjectBorrow(oldvalue)));
160-
cell = PyStackRef_AsPyObjectBorrow(oldvalue);
161-
} else if (kind & CO_FAST_CELL && oldvalue.bits != 0) {
162-
PyObject *as_obj = PyStackRef_AsPyObjectBorrow(oldvalue);
163-
if (PyCell_Check(as_obj)) {
164-
cell = as_obj;
165-
}
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);
172+
173+
_PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
174+
_PyStackRef 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.bits != 0 && PyCell_Check(PyStackRef_AsPyObjectBorrow(oldvalue)));
180+
cell = PyStackRef_AsPyObjectBorrow(oldvalue);
181+
} else if (kind & CO_FAST_CELL && oldvalue.bits != 0) {
182+
PyObject *as_obj = PyStackRef_AsPyObjectBorrow(oldvalue);
183+
if (PyCell_Check(as_obj)) {
184+
cell = as_obj;
166185
}
167-
if (cell != NULL) {
168-
PyObject *oldvalue_o = PyCell_GET(cell);
169-
if (value != oldvalue_o) {
170-
PyCell_SET(cell, Py_XNewRef(value));
171-
Py_XDECREF(oldvalue_o);
172-
}
173-
} else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) {
174-
PyStackRef_XCLOSE(fast[i]);
175-
fast[i] = PyStackRef_FromPyObjectNew(value);
186+
}
187+
if (cell != NULL) {
188+
PyObject *oldvalue_o = PyCell_GET(cell);
189+
if (value != oldvalue_o) {
190+
PyCell_SET(cell, Py_XNewRef(value));
191+
Py_XDECREF(oldvalue_o);
176192
}
177-
return 0;
193+
} else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) {
194+
PyStackRef_XCLOSE(fast[i]);
195+
fast[i] = PyStackRef_FromPyObjectNew(value);
178196
}
197+
return 0;
179198
}
180199

181200
// Okay not in the fast locals, try extra locals
@@ -545,11 +564,12 @@ framelocalsproxy_contains(PyObject *self, PyObject *key)
545564
{
546565
PyFrameObject *frame = ((PyFrameLocalsProxyObject*)self)->frame;
547566

548-
if (PyUnicode_CheckExact(key)) {
549-
int i = framelocalsproxy_getkeyindex(frame, key, true);
550-
if (i >= 0) {
551-
return 1;
552-
}
567+
int i = framelocalsproxy_getkeyindex(frame, key, true);
568+
if (i == -2) {
569+
return -1;
570+
}
571+
if (i >= 0) {
572+
return 1;
553573
}
554574

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.