Skip to content

Commit 30c9491

Browse files
ppislppisl
committed
[GR-21351] add unittest for testing for cases where nonlocal variables do not end up in the freevars of the function
PullRequest: graalpython/825
2 parents 9f4eede + 4aa036b commit 30c9491

File tree

7 files changed

+199
-14
lines changed

7 files changed

+199
-14
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/FuncDefTests.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,12 @@ public void positionalOnlyArg47() throws Exception {
475475
checkSyntaxError("async def f(a, *, c, /, d, e): pass");
476476
}
477477

478+
@Test
479+
public void issue21351() throws Exception {
480+
// parser test for GR-21351
481+
checkScopeAndTree();
482+
}
483+
478484
private void checkScopeAndTree() throws Exception {
479485
File testFile = getTestFileFromTestAndTestMethod();
480486
checkScopeFromFile(testFile, true);

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
1+
# Copyright (c) 2018, 2020, 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
@@ -930,3 +930,18 @@ class D(B, C):
930930
locs = list(A.locs.keys())
931931
for k in ["__module__", "__qualname__", "ranges", "B", "C", "locs", "D"]:
932932
assert k in locs, locs
933+
934+
935+
def test_free_var_with_nonlocals():
936+
def outer():
937+
var = None
938+
939+
def toInternal(obj):
940+
nonlocal var
941+
x = 10
942+
943+
return toInternal
944+
fnc = outer()
945+
c = fnc.__code__
946+
assert c.co_freevars == ('var',)
947+
assert c.co_cellvars == tuple()

‎graalpython/com.oracle.graal.python.test/testData/testFiles/FuncDefTests/issue21351.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
def outer():
2+
var = None
3+
def toInternal(obj):
4+
nonlocal var
5+
x = 10
6+
return toInternal
7+
8+
fnc = outer()
9+
c = fnc.__code__
10+
11+
print("free > ", c.co_freevars)
12+
print("cell > ", c.co_cellvars)

‎graalpython/com.oracle.graal.python.test/testData/testFiles/FuncDefTests/issue21351.scope

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Scope: []
2+
Kind: Module
3+
FrameDescriptor: Empty
4+
CellVars: Empty
5+
FreeVars: Empty
6+
Scope: outer
7+
Kind: Function
8+
FrameDescriptor: [toInternal, var, <return_val>]
9+
CellVars: [var]
10+
FreeVars: Empty
11+
Scope: toInternal
12+
Kind: Function
13+
FrameDescriptor: [obj, var, x, <return_val>]
14+
CellVars: Empty
15+
FreeVars: [var]

‎graalpython/com.oracle.graal.python.test/testData/testFiles/FuncDefTests/issue21351.tast

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
ModuleRootNode Name: <module 'issue21351'> SourceSection: [0,208]`def outer():↵ var...`
2+
Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
3+
FreeVars: None
4+
NeedsCellFrame: False
5+
FrameDescriptor: Empty
6+
Documentation: None
7+
InnerRootNode SourceSection: [0,208]`def outer():↵ var...`
8+
ExpressionWithSideEffects SourceSection: [0,207]`def outer():↵ var...`
9+
WriteNameNodeGen SourceSection: [0,112]`def outer():↵ var...`
10+
Identifier: outer
11+
FunctionDefinitionNode Name: outer SourceSection: None
12+
Arguments: None
13+
KwArguments: None
14+
Documentation: StringLiteralNode: Empty
15+
FreeVarSlots: None
16+
ExecutionSlots:
17+
FreeVarsSlots: None
18+
CellVarsSlots: var,
19+
FunctionRootNode SourceSection: [0,112]`def outer():↵ var...`
20+
Name: outer
21+
Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
22+
CelVars: var
23+
FreeVars: None
24+
NeedsCellFrame: False
25+
FrameDescriptor: 3 slots [var, toInternal, <return_val>]
26+
ExecutionSlots:
27+
FreeVarsSlots: None
28+
CellVarsSlots: var,
29+
InnerRootNode SourceSection: [0,112]`def outer():↵ var...`
30+
ReturnTargetNode SourceSection: [0,112]`def outer():↵ var...`
31+
Body: BlockNode SourceSection: None
32+
BlockNode SourceSection: None
33+
FunctionBodyNode SourceSection: [17,110]`var = None↵ def t...`
34+
WriteLocalCellNodeGen SourceSection: [17,27]`var = None`
35+
Identifier: var
36+
ReadLocalVariableNode SourceSection: None
37+
Frame: [0,var,Illegal]
38+
ReadVariableFromFrameNodeGen SourceSection: None
39+
ObjectLiteralNode SourceSection: [23,27]`None`
40+
WriteLocalVariableNodeGen SourceSection: [32,93]`def toInternal(obj):...`
41+
Identifier: toInternal
42+
WriteLocalFrameSlotNodeGen SourceSection: None
43+
Frame: [1,toInternal,Illegal]
44+
FunctionDefinitionNode Name: toInternal SourceSection: None
45+
Arguments: None
46+
KwArguments: None
47+
Documentation: StringLiteralNode: Empty
48+
FreeVarSlots: var,
49+
ExecutionSlots:
50+
FreeVarsSlots: var,
51+
CellVarsSlots: None
52+
FunctionRootNode SourceSection: [32,93]`def toInternal(obj):...`
53+
Name: toInternal
54+
Signature: varArgs=False, varKeywordArgs=False, noArguments=False, positionalOnly=True, requiresKeywordArgs=False
55+
Param Names: obj
56+
CelVars: None
57+
FreeVars: var
58+
NeedsCellFrame: False
59+
FrameDescriptor: 4 slots [obj, x, var, <return_val>]
60+
ExecutionSlots:
61+
FreeVarsSlots: var,
62+
CellVarsSlots: None
63+
InnerRootNode SourceSection: [32,93]`def toInternal(obj):...`
64+
ReturnTargetNode SourceSection: [32,93]`def toInternal(obj):...`
65+
Body: BlockNode SourceSection: None
66+
WriteLocalVariableNodeGen SourceSection: None
67+
Identifier: obj
68+
WriteLocalFrameSlotNodeGen SourceSection: None
69+
Frame: [0,obj,Illegal]
70+
ArgumentExpressionNode SourceSection: None
71+
ReadIndexedArgumentNodeGen SourceSection: None
72+
Index: 0
73+
FunctionBodyNode SourceSection: [61,88]`nonlocal var↵ ...`
74+
ExpressionStatementNode SourceSection: [61,73]`nonlocal var`
75+
EmptyNode SourceSection: [61,73]`nonlocal var`
76+
WriteLocalVariableNodeGen SourceSection: [82,88]`x = 10`
77+
Identifier: x
78+
WriteLocalFrameSlotNodeGen SourceSection: None
79+
Frame: [1,x,Illegal]
80+
IntegerLiteralNode SourceSection: [86,88]`10`
81+
Value: 10
82+
Return Expresssion: ReadLocalVariableNode SourceSection: None
83+
Frame: [3,<return_val>,Illegal]
84+
ReadVariableFromFrameNodeGen SourceSection: None
85+
FrameReturnNode SourceSection: [93,110]`return toInternal`
86+
WriteLocalVariableNodeGen SourceSection: None
87+
Identifier: <return_val>
88+
WriteLocalFrameSlotNodeGen SourceSection: None
89+
Frame: [2,<return_val>,Illegal]
90+
ReadLocalVariableNode SourceSection: [100,110]`toInternal`
91+
Frame: [1,toInternal,Illegal]
92+
ReadVariableFromFrameNodeGen SourceSection: None
93+
Return Expresssion: ReadLocalVariableNode SourceSection: None
94+
Frame: [2,<return_val>,Illegal]
95+
ReadVariableFromFrameNodeGen SourceSection: None
96+
WriteNameNodeGen SourceSection: [112,125]`fnc = outer()`
97+
Identifier: fnc
98+
PythonCallNodeGen SourceSection: [118,125]`outer()`
99+
CallNodeGen SourceSection: None
100+
ReadNameNodeGen SourceSection: [118,123]`outer`
101+
Identifier: outer
102+
WriteNameNodeGen SourceSection: [126,142]`c = fnc.__code__`
103+
Identifier: c
104+
GetAttributeNodeGen SourceSection: [130,142]`fnc.__code__`
105+
GetFixedAttributeNodeGen SourceSection: None
106+
Key: __code__
107+
LookupAndCallBinaryNodeGen SourceSection: None
108+
Op: __getattribute__
109+
ReadNameNodeGen SourceSection: [130,133]`fnc`
110+
Identifier: fnc
111+
ExpressionStatementNode SourceSection: [144,175]`print("free > ", c.c...`
112+
PythonCallNodeGen SourceSection: [144,175]`print("free > ", c.c...`
113+
CallNodeGen SourceSection: None
114+
StringLiteralNode SourceSection: [150,159]`"free > "`
115+
GetAttributeNodeGen SourceSection: [161,174]`c.co_freevars`
116+
GetFixedAttributeNodeGen SourceSection: None
117+
Key: co_freevars
118+
LookupAndCallBinaryNodeGen SourceSection: None
119+
Op: __getattribute__
120+
ReadNameNodeGen SourceSection: [161,162]`c`
121+
Identifier: c
122+
ReadNameNodeGen SourceSection: [144,149]`print`
123+
Identifier: print
124+
PythonCallNodeGen SourceSection: [176,207]`print("cell > ", c.c...`
125+
CallNodeGen SourceSection: None
126+
StringLiteralNode SourceSection: [182,191]`"cell > "`
127+
GetAttributeNodeGen SourceSection: [193,206]`c.co_cellvars`
128+
GetFixedAttributeNodeGen SourceSection: None
129+
Key: co_cellvars
130+
LookupAndCallBinaryNodeGen SourceSection: None
131+
Op: __getattribute__
132+
ReadNameNodeGen SourceSection: [193,194]`c`
133+
Identifier: c
134+
ReadNameNodeGen SourceSection: [176,181]`print`
135+
Identifier: print

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/ScopeEnvironment.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -72,6 +72,7 @@ public class ScopeEnvironment implements CellFrameSlotSupplier {
7272

7373
public static String CLASS_VAR_PREFIX = "<>class";
7474
public static String LAMBDA_NAME = "lambda";
75+
public static int CLASS_VAR_PREFIX_IDX = CLASS_VAR_PREFIX.length();
7576

7677
private final NodeFactory factory;
7778

@@ -116,21 +117,21 @@ public ScopeInfo popScope() {
116117
String name = identifier instanceof String ? (String) identifier : identifier.toString();
117118

118119
if (localySeenVars != null) {
119-
// remove the localy declared variable
120+
// remove the locally declared variable
120121
if (definingScopeKind == ScopeInfo.ScopeKind.Class && name.startsWith(CLASS_VAR_PREFIX)) {
121-
localySeenVars.remove(name.substring(7));
122+
localySeenVars.remove(name.substring(CLASS_VAR_PREFIX_IDX));
122123
} else {
123124
localySeenVars.remove(name);
124125
}
125126
}
126127

127128
List<ScopeInfo> usedInScopes = unresolvedVars.get(name);
128-
// was the declared varibale seen before?
129+
// was the declared variable seen before?
129130
if (usedInScopes != null && !(definingScopeKind == ScopeInfo.ScopeKind.Module && definingScope.findFrameSlot(name) != null)) {
130-
// make the varible freevar and cellvar in scopes, where is used
131+
// make the variable freevar and cellvar in scopes, where is used
131132
List<ScopeInfo> copy = new ArrayList<>(usedInScopes);
132133
for (ScopeInfo scope : copy) {
133-
// we need to find out, whether the scope is a under the defing scope
134+
// we need to find out, whether the scope is a under the defining scope
134135
ScopeInfo tmpScope = scope;
135136
ScopeInfo parentDefiningScope = definingScope.getParent();
136137
while (tmpScope != null && tmpScope != definingScope && tmpScope != parentDefiningScope) {
@@ -177,13 +178,13 @@ public ScopeInfo popScope() {
177178
String name = (String) identifier;
178179
if (name.startsWith(CLASS_VAR_PREFIX)) {
179180
definingScope.getFrameDescriptor().removeFrameSlot(identifier);
180-
name = name.substring(7);
181+
name = name.substring(CLASS_VAR_PREFIX_IDX);
181182
definingScope.createSlotIfNotPresent(name);
182183
copy = true;
183184
}
184185
}
185186
if (copy) {
186-
// we copy it because the idexes are now wrong due the issue GR-17984
187+
// we copy it because the indexes are now wrong due the issue GR-17984
187188
definingScope.setFrameDescriptor(definingScope.getFrameDescriptor().copy());
188189
}
189190
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/ScopeInfo.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2019, Oracle and/or its affiliates.
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates.
33
* Copyright (c) 2013, Regents of the University of California
44
*
55
* All rights reserved.
@@ -193,6 +193,7 @@ public void addExplicitNonlocalVariable(String identifier) {
193193
if (explicitNonlocalVariables == null) {
194194
explicitNonlocalVariables = new HashSet<>();
195195
}
196+
addSeenVar(identifier);
196197
explicitNonlocalVariables.add(identifier);
197198
}
198199

@@ -204,6 +205,10 @@ public boolean isExplicitNonlocalVariable(String identifier) {
204205
return explicitNonlocalVariables != null && explicitNonlocalVariables.contains(identifier);
205206
}
206207

208+
public Set<String> getExplicitNonlocalVariables() {
209+
return explicitNonlocalVariables;
210+
}
211+
207212
public void addCellVar(String identifier) {
208213
addCellVar(identifier, false);
209214
}
@@ -218,10 +223,6 @@ public void addCellVar(String identifier, boolean createFrameSlot) {
218223
}
219224
}
220225

221-
public void addFreeVar(String identifier) {
222-
addFreeVar(identifier, false);
223-
}
224-
225226
public void addFreeVar(String identifier, boolean createFrameSlot) {
226227
if (freeVars == null) {
227228
freeVars = new TreeSet<>();

0 commit comments

Comments
 (0)