Skip to content

Commit a9dd444

Browse files
msimacekmsimacek
committed
[GR-44038] Fix locals sync
PullRequest: graalpython/2626
2 parents ae06baa + 73e154c commit a9dd444

File tree

21 files changed

+600
-885
lines changed

21 files changed

+600
-885
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_frame_tests.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
1+
# Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
22
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
33
#
44
# The Universal Permissive License (UPL), Version 1.0
@@ -126,6 +126,19 @@ def test_builtins():
126126
assert print == sys._getframe().f_builtins["print"]
127127

128128

129+
def test_locals_sync():
130+
a = 1
131+
l = locals()
132+
assert l == {'a': 1}
133+
b = 2
134+
# Forces caller frame materialization, this used to erroneously cause the locals dict to update
135+
globals()
136+
assert l == {'a': 1}
137+
# Now this should really cause the locals dict to update
138+
locals()
139+
assert l == {'a': 1, 'b': 2, 'l': l}
140+
141+
129142
# GR-22089
130143
# def test_backref_from_traceback():
131144
# def bar():

‎graalpython/com.oracle.graal.python.test/src/tests/test_pdb.py

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
# Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
2+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
3+
#
4+
# The Universal Permissive License (UPL), Version 1.0
5+
#
6+
# Subject to the condition set forth below, permission is hereby granted to any
7+
# person obtaining a copy of this software, associated documentation and/or
8+
# data (collectively the "Software"), free of charge and under any and all
9+
# copyright rights in the Software, and any and all patent rights owned or
10+
# freely licensable by each licensor hereunder covering either (i) the
11+
# unmodified Software as contributed to or provided by such licensor, or (ii)
12+
# the Larger Works (as defined below), to deal in both
13+
#
14+
# (a) the Software, and
15+
#
16+
# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
17+
# one is included with the Software each a "Larger Work" to which the Software
18+
# is contributed by such licensors),
19+
#
20+
# without restriction, including without limitation the rights to copy, create
21+
# derivative works of, display, perform, and distribute the Software and make,
22+
# use, sell, offer for sale, import, export, have made, and have sold the
23+
# Software and the Larger Work(s), and to sublicense the foregoing rights on
24+
# either these or other terms.
25+
#
26+
# This license is subject to the following condition:
27+
#
28+
# The above copyright notice and either this complete permission notice or at a
29+
# minimum a reference to the UPL must be included in all copies or substantial
30+
# portions of the Software.
31+
#
32+
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
33+
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
34+
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
35+
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
36+
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
37+
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
38+
# SOFTWARE.
39+
40+
import doctest
41+
import sys
42+
43+
44+
# Copied from test_doctest
45+
class _FakeInput:
46+
def __init__(self, lines):
47+
self.lines = lines
48+
49+
def readline(self):
50+
line = self.lines.pop(0)
51+
print(line)
52+
return line + '\n'
53+
54+
55+
# Copied from test_pdb
56+
class PdbTestInput(object):
57+
def __init__(self, input):
58+
self.input = input
59+
60+
def __enter__(self):
61+
self.real_stdin = sys.stdin
62+
sys.stdin = _FakeInput(self.input)
63+
self.orig_trace = sys.gettrace() if hasattr(sys, 'gettrace') else None
64+
65+
def __exit__(self, *exc):
66+
sys.stdin = self.real_stdin
67+
if self.orig_trace:
68+
sys.settrace(self.orig_trace)
69+
70+
71+
def doctest_pdb_locals():
72+
"""
73+
Test that locals get synced after breakpoint
74+
75+
>>> def test_function():
76+
... a = 1
77+
... import pdb; pdb.Pdb(nosigint=True, readrc=False).set_trace()
78+
... a = 2
79+
80+
>>> with PdbTestInput([
81+
... 'p a',
82+
... 'next',
83+
... 'p a',
84+
... 'continue',
85+
... ]):
86+
... test_function()
87+
> <doctest tests.test_pdb.doctest_pdb_locals[0]>(4)test_function()
88+
-> a = 2
89+
(Pdb) p a
90+
1
91+
(Pdb) next
92+
--Return--
93+
> <doctest tests.test_pdb.doctest_pdb_locals[0]>(4)test_function()->None
94+
-> a = 2
95+
(Pdb) p a
96+
2
97+
(Pdb) continue
98+
"""
99+
100+
101+
def doctest_pdb_locals_generator():
102+
"""
103+
Test that locals get synced after breakpoint in a generator
104+
105+
>>> def test_function():
106+
... a = 1
107+
... import pdb; pdb.Pdb(nosigint=True, readrc=False).set_trace()
108+
... a = 2
109+
... yield
110+
111+
>>> with PdbTestInput([
112+
... 'p a',
113+
... 'next',
114+
... 'p a',
115+
... 'continue',
116+
... ]):
117+
... next(test_function())
118+
> <doctest tests.test_pdb.doctest_pdb_locals_generator[0]>(4)test_function()
119+
-> a = 2
120+
(Pdb) p a
121+
1
122+
(Pdb) next
123+
> <doctest tests.test_pdb.doctest_pdb_locals_generator[0]>(5)test_function()
124+
-> yield
125+
(Pdb) p a
126+
2
127+
(Pdb) continue
128+
"""
129+
130+
131+
def doctest_pdb_locals_sync_back():
132+
"""
133+
Test that locals set by debugger get propagated back into the frame.
134+
135+
>>> def test_function():
136+
... foo = 1
137+
... import pdb; pdb.Pdb(nosigint=True, readrc=False).set_trace()
138+
... return foo
139+
140+
>>> with PdbTestInput([
141+
... 'p foo',
142+
... 'foo = 5',
143+
... 'continue',
144+
... ]):
145+
... print(test_function())
146+
> <doctest tests.test_pdb.doctest_pdb_locals_sync_back[0]>(4)test_function()
147+
-> return foo
148+
(Pdb) p foo
149+
1
150+
(Pdb) foo = 5
151+
(Pdb) continue
152+
5
153+
"""
154+
155+
156+
def test_run_doctests():
157+
doctest.testmod(sys.modules[__name__], raise_on_error=True)

‎graalpython/com.oracle.graal.python.test/src/tests/test_tagged_unittests.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@
3636
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
3737
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3838
# SOFTWARE.
39+
import test
3940

4041
import glob
4142
import os
43+
import shlex
4244
import subprocess
4345
import sys
44-
import test
4546
from itertools import zip_longest
4647

4748
if os.environ.get("ENABLE_CPYTHON_TAGGED_UNITTESTS") == "true" or __name__ == "__main__":
@@ -106,7 +107,7 @@ def test_tagged(max_patterns=100):
106107
else:
107108
rcode = run_with_timeout(cmd).returncode
108109
if rcode:
109-
raise CalledProcessError(rcode, cmd)
110+
raise subprocess.CalledProcessError(rcode, cmd)
110111
print(working_test[0], "was finished.")
111112

112113
def run_serialize_out(cmd):
@@ -175,7 +176,7 @@ def parse_unittest_output(output):
175176
# The whole reason for this function's complexity is that we want to consume arbitrary
176177
# warnings after the '...' part without accidentally consuming the next test result
177178
import re
178-
re_test_result = re.compile(r"""\b(test\S+) \(([^\s]+)\)(?:\n.*?)?? \.\.\. """, re.MULTILINE | re.DOTALL)
179+
re_test_result = re.compile(r"""\b(test\S+) \((\S+)\)(?:\n.*?)?? \.\.\. """, re.MULTILINE | re.DOTALL)
179180
re_test_status = re.compile(r"""\b(ok|skipped (?:'[^']*'|"[^"]*")|FAIL|ERROR)$""", re.MULTILINE | re.DOTALL)
180181
pos = 0
181182
current_result = None
@@ -241,7 +242,7 @@ def main():
241242
else:
242243
testfile_stem = os.path.splitext(os.path.basename(testfile))[0]
243244
testmod = "test." + testfile_stem
244-
cmd = executable
245+
cmd = list(executable)
245246
if repeat == 0:
246247
# Allow catching Java exceptions in the first iteration only, so that subsequent iterations
247248
# (there will be one even if everything succeeds) filter out possible false-passes caused by
@@ -264,7 +265,7 @@ def main():
264265
cmd += ["-k", selector]
265266
cmd.append(testfile)
266267

267-
print(" ".join(cmd))
268+
print(shlex.join(cmd))
268269
p = run_with_timeout(cmd, errors='backslashreplace', **kwargs)
269270
print("*stdout*")
270271
print(p.stdout)
@@ -273,7 +274,7 @@ def main():
273274

274275
if p.returncode == -9:
275276
print(
276-
f"\nTimeout (return code -9)\nyou can try to increase the current timeout {tout}s by using --timeout=NNN")
277+
f"\nTimeout (return code -9)\nyou can try to increase the current timeout {TIMEOUT}s by using --timeout=NNN")
277278

278279
passing_tests = set()
279280
failing_tests = set()

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import com.oracle.graal.python.nodes.call.CallNode;
7373
import com.oracle.graal.python.nodes.call.GenericInvokeNode;
7474
import com.oracle.graal.python.nodes.exception.TopLevelExceptionHandler;
75+
import com.oracle.graal.python.nodes.frame.GetFrameLocalsNode;
7576
import com.oracle.graal.python.nodes.frame.MaterializeFrameNode;
7677
import com.oracle.graal.python.pegparser.InputType;
7778
import com.oracle.graal.python.pegparser.NodeFactory;
@@ -589,13 +590,14 @@ public ExecutableNode parse(InlineParsingRequest request) {
589590
@Child private GilNode gilNode = GilNode.create();
590591
@Child private GenericInvokeNode invokeNode = GenericInvokeNode.create();
591592
@Child private MaterializeFrameNode materializeFrameNode = MaterializeFrameNode.create();
593+
@Child private GetFrameLocalsNode getFrameLocalsNode = GetFrameLocalsNode.create();
592594

593595
@Override
594596
public Object execute(VirtualFrame frame) {
595597
Object[] arguments = PArguments.create();
596598
// escape?
597-
PFrame pFrame = materializeFrameNode.execute(frame, this, false, true, frame);
598-
Object locals = pFrame.getLocals();
599+
PFrame pFrame = materializeFrameNode.execute(this, false, true, frame);
600+
Object locals = getFrameLocalsNode.execute(pFrame);
599601
PArguments.setSpecialArgument(arguments, locals);
600602
PArguments.setGlobals(arguments, PArguments.getGlobals(frame));
601603
boolean wasAcquired = gilNode.acquire();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinFunctions.java

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@
113113
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
114114
import com.oracle.graal.python.builtins.PythonBuiltins;
115115
import com.oracle.graal.python.builtins.modules.BuiltinFunctionsFactory.GetAttrNodeFactory;
116-
import com.oracle.graal.python.builtins.modules.BuiltinFunctionsFactory.GlobalsNodeFactory;
117116
import com.oracle.graal.python.builtins.modules.BuiltinFunctionsFactory.HexNodeFactory;
118117
import com.oracle.graal.python.builtins.modules.BuiltinFunctionsFactory.OctNodeFactory;
119118
import com.oracle.graal.python.builtins.modules.WarningsModuleBuiltins.WarnNode;
@@ -164,6 +163,7 @@
164163
import com.oracle.graal.python.lib.GetNextNode;
165164
import com.oracle.graal.python.lib.PyCallableCheckNode;
166165
import com.oracle.graal.python.lib.PyEvalGetGlobals;
166+
import com.oracle.graal.python.lib.PyEvalGetLocals;
167167
import com.oracle.graal.python.lib.PyMappingCheckNode;
168168
import com.oracle.graal.python.lib.PyNumberAsSizeNode;
169169
import com.oracle.graal.python.lib.PyNumberIndexNode;
@@ -215,6 +215,7 @@
215215
import com.oracle.graal.python.nodes.expression.BinaryOpNode;
216216
import com.oracle.graal.python.nodes.expression.CoerceToBooleanNode;
217217
import com.oracle.graal.python.nodes.expression.TernaryArithmetic;
218+
import com.oracle.graal.python.nodes.frame.GetFrameLocalsNode;
218219
import com.oracle.graal.python.nodes.frame.ReadCallerFrameNode;
219220
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
220221
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
@@ -765,12 +766,11 @@ public abstract static class DirNode extends PythonBuiltinNode {
765766
// logic like in 'Objects/object.c: _dir_locals'
766767
@Specialization(guards = "isNoValue(object)")
767768
Object locals(VirtualFrame frame, @SuppressWarnings("unused") Object object,
768-
@Cached ReadCallerFrameNode readCallerFrameNode,
769+
@Cached PyEvalGetLocals getLocals,
769770
@Cached("create(T_KEYS)") LookupAndCallUnaryNode callKeysNode,
770771
@Cached ListBuiltins.ListSortNode sortNode,
771772
@Cached ListNodes.ConstructListNode constructListNode) {
772-
773-
Object localsDict = LocalsNode.getLocalsDict(frame, readCallerFrameNode);
773+
Object localsDict = getLocals.execute(frame);
774774
Object keysObj = callKeysNode.executeObject(frame, localsDict);
775775
PList list = constructListNode.execute(frame, keysObj);
776776
sortNode.execute(frame, list);
@@ -861,8 +861,8 @@ private static void inheritGlobals(PFrame callerFrame, Object[] args) {
861861
PArguments.setGlobals(args, callerFrame.getGlobals());
862862
}
863863

864-
private static void inheritLocals(PFrame callerFrame, Object[] args) {
865-
Object callerLocals = callerFrame.getLocals();
864+
private static void inheritLocals(PFrame callerFrame, Object[] args, GetFrameLocalsNode getFrameLocalsNode) {
865+
Object callerLocals = getFrameLocalsNode.execute(callerFrame);
866866
setCustomLocals(args, callerLocals);
867867
}
868868

@@ -889,12 +889,13 @@ private void setCustomGlobals(VirtualFrame frame, PDict globals, HashingCollecti
889889
@Specialization
890890
Object execInheritGlobalsInheritLocals(VirtualFrame frame, Object source, @SuppressWarnings("unused") PNone globals, @SuppressWarnings("unused") PNone locals,
891891
@Cached ReadCallerFrameNode readCallerFrameNode,
892-
@Shared("getCt") @Cached CodeNodes.GetCodeCallTargetNode getCt) {
892+
@Shared("getCt") @Cached CodeNodes.GetCodeCallTargetNode getCt,
893+
@Cached GetFrameLocalsNode getFrameLocalsNode) {
893894
PCode code = createAndCheckCode(frame, source);
894895
PFrame callerFrame = readCallerFrameNode.executeWith(frame, 0);
895896
Object[] args = PArguments.create();
896897
inheritGlobals(callerFrame, args);
897-
inheritLocals(callerFrame, args);
898+
inheritLocals(callerFrame, args, getFrameLocalsNode);
898899

899900
return invokeNode.execute(frame, getCt.execute(code), args);
900901
}
@@ -2265,9 +2266,9 @@ private Object iterateGeneric(VirtualFrame frame, Object iterator, Object start,
22652266
}
22662267
}
22672268

2268-
@Builtin(name = "globals")
2269+
@Builtin(name = "globals", needsFrame = true, alwaysNeedsCallerFrame = true)
22692270
@GenerateNodeFactory
2270-
public abstract static class GlobalsNode extends PythonBuiltinNode {
2271+
abstract static class GlobalsNode extends PythonBuiltinNode {
22712272
private final ConditionProfile condProfile = ConditionProfile.create();
22722273

22732274
@Specialization
@@ -2281,10 +2282,6 @@ public Object globals(VirtualFrame frame,
22812282
return globals;
22822283
}
22832284
}
2284-
2285-
public static GlobalsNode create() {
2286-
return GlobalsNodeFactory.create(null);
2287-
}
22882285
}
22892286

22902287
@Builtin(name = "locals", needsFrame = true, alwaysNeedsCallerFrame = true)
@@ -2293,18 +2290,8 @@ abstract static class LocalsNode extends PythonBuiltinNode {
22932290

22942291
@Specialization
22952292
Object locals(VirtualFrame frame,
2296-
@Cached ReadCallerFrameNode readCallerFrameNode) {
2297-
return getLocalsDict(frame, readCallerFrameNode);
2298-
}
2299-
2300-
static Object getLocalsDict(VirtualFrame frame, ReadCallerFrameNode readCallerFrameNode) {
2301-
PFrame callerFrame = readCallerFrameNode.executeWith(frame, 0);
2302-
return callerFrame.getLocals();
2303-
}
2304-
2305-
@NeverDefault
2306-
public static LocalsNode create() {
2307-
return BuiltinFunctionsFactory.LocalsNodeFactory.create(null);
2293+
@Cached PyEvalGetLocals getLocals) {
2294+
return getLocals.execute(frame);
23082295
}
23092296
}
23102297

@@ -2314,8 +2301,8 @@ abstract static class VarsNode extends PythonUnaryBuiltinNode {
23142301

23152302
@Specialization(guards = "isNoValue(none)")
23162303
Object vars(VirtualFrame frame, @SuppressWarnings("unused") PNone none,
2317-
@Cached LocalsNode localsNode) {
2318-
return localsNode.execute(frame);
2304+
@Cached PyEvalGetLocals getLocals) {
2305+
return getLocals.execute(frame);
23192306
}
23202307

23212308
@Specialization(guards = "!isNoValue(obj)")

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/SysModuleBuiltins.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ PFrame counted(VirtualFrame frame, int num,
832832
private static PFrame escapeFrame(VirtualFrame frame, int num, ReadCallerFrameNode readCallerNode) {
833833
Reference currentFrameInfo = PArguments.getCurrentFrameInfo(frame);
834834
currentFrameInfo.markAsEscaped();
835-
return readCallerNode.executeWith(frame, currentFrameInfo, num);
835+
return readCallerNode.executeWith(currentFrameInfo, num);
836836
}
837837

838838
private PException raiseCallStackDepth() {

0 commit comments

Comments
 (0)