Skip to content

Commit 6a52195

Browse files
ppislppisl
committed
[GR-18174] ForComprehansionExpression doesn't create right target for generators. Fix for github issue #101
PullRequest: graalpython/652
2 parents 21f4754 + dbbb641 commit 6a52195

File tree

4 files changed

+154
-4
lines changed

4 files changed

+154
-4
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,4 +300,9 @@ public void genFromPip() throws Exception {
300300
checkScopeFromFile(testFile, true);
301301
checkTreeFromFile(testFile, true);
302302
}
303+
304+
@Test
305+
public void issueGR18174() throws Exception {
306+
checkScopeAndTree("[b for b in [a for a in (1,2)]]");
307+
}
303308
}

‎graalpython/com.oracle.graal.python.test/testData/goldenFiles/GeneratorAndCompForTests/issueGR18174.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: generator13
7+
Kind: Generator
8+
FrameDescriptor: [a, <return_val>]
9+
CellVars: Empty
10+
FreeVars: Empty
11+
Scope: generator1
12+
Kind: Generator
13+
FrameDescriptor: [b, <return_val>]
14+
CellVars: Empty
15+
FreeVars: Empty

‎graalpython/com.oracle.graal.python.test/testData/goldenFiles/GeneratorAndCompForTests/issueGR18174.tast

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
ModuleRootNode Name: <module 'issueGR18174'> SourceSection: [0,31]`[b for b in [a for a...`
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,31]`[b for b in [a for a...`
8+
PythonCallNodeGen SourceSection: [0,31]`[b for b in [a for a...`
9+
CachedCallNodeGen SourceSection: None
10+
CreateArgumentsNodeGen SourceSection: None
11+
CallDispatchNodeGen SourceSection: None
12+
GeneratorExpressionNode SourceSection: [3,30]`for b in [a for a in...`
13+
Name: [].<locals>.<genexp>:issueGR18174:1
14+
FrameDescriptor: 2 slots [b, <return_val>]
15+
Enclosing
16+
FrameDescriptor: Empty
17+
Active Flags: 2
18+
For Nodes: 1
19+
Block Nodes: 0
20+
Is Enclosing Frame Generator: false
21+
FunctionRootNode SourceSection: [3,30]`for b in [a for a in...`
22+
Name: [].<locals>.<genexp>:issueGR18174:1
23+
Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
24+
CelVars: None
25+
FreeVars: None
26+
NeedsCellFrame: False
27+
FrameDescriptor: 2 slots [b, <return_val>]
28+
ExecutionSlots:
29+
FreeVarsSlots: None
30+
CellVarsSlots: None
31+
InnerRootNode SourceSection: [3,30]`for b in [a for a in...`
32+
GeneratorReturnTargetNode SourceSection: [3,30]`for b in [a for a in...`
33+
flagSlot: 1
34+
GeneratorForNode SourceSection: [3,30]`for b in [a for a in...`
35+
ExpressionStatementNode SourceSection: [1,2]`b`
36+
YieldNode SourceSection: [1,2]`b`
37+
flagSlot: 0
38+
ReadGeneratorFrameVariableNode SourceSection: [1,2]`b`
39+
Frame: [0,b,Illegal]
40+
ReadVariableFromFrameNodeGen SourceSection: None
41+
GeneratorAccessNode SourceSection: None
42+
WriteGeneratorFrameVariableNodeGen SourceSection: None
43+
Identifier: b
44+
WriteGeneraterFrameSlotNodeGen SourceSection: None
45+
Frame: [0,b,Illegal]
46+
ArgumentExpressionNode SourceSection: None
47+
ReadIndexedArgumentNodeGen SourceSection: None
48+
Index: 0
49+
GetNextNode SourceSection: None
50+
LookupAndCallUnaryNodeGen SourceSection: None
51+
Op: __next__
52+
GeneratorAccessNode SourceSection: None
53+
ReadGeneratorFrameVariableNode SourceSection: None
54+
Frame: [1,<return_val>,Illegal]
55+
ReadVariableFromFrameNodeGen SourceSection: None
56+
BlockNode SourceSection: None
57+
GeneratorAccessNode SourceSection: None
58+
PRaiseNodeGen SourceSection: None
59+
GetIteratorExpressionNodeGen SourceSection: [13,29]`a for a in (1,2)`
60+
GetIteratorNodeGen SourceSection: None
61+
PythonCallNodeGen SourceSection: [13,29]`a for a in (1,2)`
62+
CachedCallNodeGen SourceSection: None
63+
CreateArgumentsNodeGen SourceSection: None
64+
CallDispatchNodeGen SourceSection: None
65+
GeneratorExpressionNode SourceSection: [15,29]`for a in (1,2)`
66+
Name: [].<locals>.<genexp>:issueGR18174:1
67+
FrameDescriptor: 2 slots [a, <return_val>]
68+
Enclosing
69+
FrameDescriptor: Empty
70+
Active Flags: 2
71+
For Nodes: 1
72+
Block Nodes: 0
73+
Is Enclosing Frame Generator: true
74+
FunctionRootNode SourceSection: [15,29]`for a in (1,2)`
75+
Name: [].<locals>.<genexp>:issueGR18174:1
76+
Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
77+
CelVars: None
78+
FreeVars: None
79+
NeedsCellFrame: False
80+
FrameDescriptor: 2 slots [a, <return_val>]
81+
ExecutionSlots:
82+
FreeVarsSlots: None
83+
CellVarsSlots: None
84+
InnerRootNode SourceSection: [15,29]`for a in (1,2)`
85+
GeneratorReturnTargetNode SourceSection: [15,29]`for a in (1,2)`
86+
flagSlot: 1
87+
GeneratorForNode SourceSection: [15,29]`for a in (1,2)`
88+
ExpressionStatementNode SourceSection: [13,14]`a`
89+
YieldNode SourceSection: [13,14]`a`
90+
flagSlot: 0
91+
ReadGeneratorFrameVariableNode SourceSection: [13,14]`a`
92+
Frame: [0,a,Illegal]
93+
ReadVariableFromFrameNodeGen SourceSection: None
94+
GeneratorAccessNode SourceSection: None
95+
WriteGeneratorFrameVariableNodeGen SourceSection: None
96+
Identifier: a
97+
WriteGeneraterFrameSlotNodeGen SourceSection: None
98+
Frame: [0,a,Illegal]
99+
ArgumentExpressionNode SourceSection: None
100+
ReadIndexedArgumentNodeGen SourceSection: None
101+
Index: 0
102+
GetNextNode SourceSection: None
103+
LookupAndCallUnaryNodeGen SourceSection: None
104+
Op: __next__
105+
GeneratorAccessNode SourceSection: None
106+
ReadGeneratorFrameVariableNode SourceSection: None
107+
Frame: [1,<return_val>,Illegal]
108+
ReadVariableFromFrameNodeGen SourceSection: None
109+
BlockNode SourceSection: None
110+
GeneratorAccessNode SourceSection: None
111+
PRaiseNodeGen SourceSection: None
112+
GetIteratorExpressionNodeGen SourceSection: [24,29]`(1,2)`
113+
GetIteratorNodeGen SourceSection: None
114+
TupleLiteralNode SourceSection: [24,29]`(1,2)`
115+
PythonObjectFactoryNodeGen SourceSection: None
116+
IntegerLiteralNode SourceSection: [25,26]`1`
117+
Value: 1
118+
IntegerLiteralNode SourceSection: [27,28]`2`
119+
Value: 2
120+
PythonObjectFactoryNodeGen SourceSection: None
121+
ReadGlobalOrBuiltinNodeGen SourceSection: None
122+
Identifier: list
123+
ReadAttributeFromObjectNotTypeNodeGen SourceSection: None
124+
PythonObjectFactoryNodeGen SourceSection: None
125+
ReadGlobalOrBuiltinNodeGen SourceSection: None
126+
Identifier: list
127+
ReadAttributeFromObjectNotTypeNodeGen SourceSection: None

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/GeneratorFactorySSTVisitor.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,16 +167,19 @@ public PNode visit(ForComprehensionSSTNode node) {
167167
int oldNumOfGeneratorBlockNode = numOfGeneratorBlockNode;
168168
int oldNumOfGeneratorForNode = numOfGeneratorForNode;
169169
init();
170-
SSTNode sstIterator = node.iterator instanceof ForComprehensionSSTNode ? ((ForComprehensionSSTNode) node.iterator).target : node.iterator;
170+
SSTNode sstIterator = node.iterator;
171+
if (sstIterator instanceof ForComprehensionSSTNode && ((ForComprehensionSSTNode) sstIterator).resultType == PythonBuiltinClassType.PGenerator) {
172+
sstIterator = ((ForComprehensionSSTNode) node.iterator).target;
173+
}
171174
ScopeInfo originScope = scopeEnvironment.getCurrentScope();
172175
scopeEnvironment.setCurrentScope(node.scope.getParent());
176+
parentVisitor.comprLevel++;
173177
ExpressionNode iterator = (ExpressionNode) sstIterator.accept(this);
174178
GetIteratorExpressionNode getIterator = nodeFactory.createGetIterator(iterator);
175179
getIterator.assignSourceSection(iterator.getSourceSection());
176180
scopeEnvironment.setCurrentScope(node.scope);
177181

178182
ExpressionNode targetExpression;
179-
parentVisitor.comprLevel++;
180183
if (node.resultType == PythonBuiltinClassType.PDict) {
181184
targetExpression = nodeFactory.createTupleLiteral((ExpressionNode) node.name.accept(this), (ExpressionNode) node.target.accept(parentVisitor));
182185
} else {
@@ -202,7 +205,7 @@ public PNode visit(ForComprehensionSSTNode node) {
202205
returnTarget.assignSourceSection(body.getSourceSection());
203206
// int lineNum = ctx.getStart().getLine();
204207

205-
// createing generator expression
208+
// creating generator expression
206209
FrameDescriptor fd = node.scope.getFrameDescriptor();
207210
String name = node.scope.getParent().getScopeId() + ".<locals>.<genexp>:" + source.getName() + ":" + node.line;
208211
FunctionRootNode funcRoot = nodeFactory.createFunctionRoot(returnTarget.getSourceSection(), name, true, fd, returnTarget, scopeEnvironment.getExecutionCellSlots(), Signature.EMPTY);
@@ -250,7 +253,7 @@ private StatementNode createGeneratorExpressionBody(ForComprehensionSSTNode node
250253
if (condition != null) {
251254
// TODO: Do we have to create empty block in the else branch?
252255
body = GeneratorIfNode.create(nodeFactory.createYesNode(condition), body, nodeFactory.createBlock(), numOfActiveFlags++, numOfActiveFlags++);
253-
} else if (node.iterator instanceof ForComprehensionSSTNode) {
256+
} else if (node.iterator instanceof ForComprehensionSSTNode && ((ForComprehensionSSTNode) node.iterator).resultType == PythonBuiltinClassType.PGenerator) {
254257
ForComprehensionSSTNode forComp = (ForComprehensionSSTNode) node.iterator;
255258
SSTNode sstIterator = forComp.iterator instanceof ForComprehensionSSTNode ? ((ForComprehensionSSTNode) forComp.iterator).target : forComp.iterator;
256259
ExpressionNode exprIterator = (ExpressionNode) sstIterator.accept(this);

0 commit comments

Comments
 (0)