Skip to content

Commit 1b7d6a5

Browse files
authored
Properly bind references in comprehension scopes (#1142)
* Properly bind references in comprehension scopes * Add tests * Make Comprehension.Iterators a read-only list
1 parent d644871 commit 1b7d6a5

File tree

3 files changed

+40
-14
lines changed

3 files changed

+40
-14
lines changed

Src/IronPython/Compiler/Ast/Comprehension.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public abstract class ComprehensionIterator : Node {
2525
}
2626

2727
public abstract class Comprehension : Expression {
28-
public abstract IList<ComprehensionIterator> Iterators { get; }
28+
public abstract IReadOnlyList<ComprehensionIterator> Iterators { get; }
2929
public abstract override string NodeName { get; }
3030

3131
protected abstract MSAst.ParameterExpression MakeParameter();
@@ -85,7 +85,7 @@ public ListComprehension(Expression item, ComprehensionIterator[] iterators) {
8585

8686
public Expression Item { get; }
8787

88-
public override IList<ComprehensionIterator> Iterators => _iterators;
88+
public override IReadOnlyList<ComprehensionIterator> Iterators => _iterators;
8989

9090
protected override MSAst.ParameterExpression MakeParameter()
9191
=> Ast.Parameter(typeof(PythonList), "list_comprehension_list");
@@ -134,7 +134,7 @@ public SetComprehension(Expression item, ComprehensionIterator[] iterators) {
134134

135135
public Expression Item { get; }
136136

137-
public override IList<ComprehensionIterator> Iterators => _iterators;
137+
public override IReadOnlyList<ComprehensionIterator> Iterators => _iterators;
138138

139139
protected override MSAst.ParameterExpression MakeParameter()
140140
=> Ast.Parameter(typeof(SetCollection), "set_comprehension_set");
@@ -186,7 +186,7 @@ public DictionaryComprehension(Expression key, Expression value, ComprehensionIt
186186

187187
public Expression Value { get; }
188188

189-
public override IList<ComprehensionIterator> Iterators => _iterators;
189+
public override IReadOnlyList<ComprehensionIterator> Iterators => _iterators;
190190

191191
protected override MSAst.ParameterExpression MakeParameter()
192192
=> Ast.Parameter(typeof(PythonDictionary), "dict_comprehension_dict");
@@ -280,6 +280,7 @@ internal override bool TryBindOuter(ScopeStatement from, PythonReference referen
280280
}
281281

282282
internal override PythonVariable BindReference(PythonNameBinder binder, PythonReference reference) {
283+
// First try variables local to this scope
283284
if (TryGetVariable(reference.Name, out PythonVariable variable)) {
284285
if (variable.Kind == VariableKind.Global) {
285286
AddReferencedGlobal(reference.Name);
@@ -288,8 +289,14 @@ internal override PythonVariable BindReference(PythonNameBinder binder, PythonRe
288289
return variable;
289290
}
290291

291-
// then bind in our parent scope
292-
return Parent.BindReference(binder, reference);
292+
// then try to bind in outer scopes
293+
for (ScopeStatement parent = Parent; parent != null; parent = parent.Parent) {
294+
if (parent.TryBindOuter(this, reference, out variable)) {
295+
return variable;
296+
}
297+
}
298+
299+
return null;
293300
}
294301

295302
internal override Ast GetVariableExpression(PythonVariable variable) {

Src/IronPython/Modules/_ast.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ internal static AST Convert(Node node) {
353353
return ast;
354354
}
355355

356-
internal static PythonList Convert(IList<ComprehensionIterator> iters) {
356+
internal static PythonList Convert(IReadOnlyList<ComprehensionIterator> iters) {
357357
var cfCollector = new List<ComprehensionFor>();
358358
var cifCollector = new List<List<ComprehensionIf>>();
359359
List<ComprehensionIf> cif = null;

Tests/test_listcomp.py

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import unittest
66

7-
from iptest import run_test, is_cli
7+
from iptest import run_test, is_cpython
88

99
class ListCompTest(unittest.TestCase):
1010
def test_positive(self):
@@ -79,12 +79,9 @@ def test_ipy3_gh809(self):
7979
# first iterable is captured and subsequent assignments do not change it
8080
self.maxDiff = None
8181
x, y, z = range(1), range(2), range(3)
82-
if is_cli:
83-
_dir = ['x', 'y']
84-
# TODO: should be: _dir = ['x', 'y', 'z']
85-
# See: https://github.com/IronLanguages/ironpython3/issues/1132
86-
else:
87-
_dir = ['.0', 'x', 'y', 'z'] # adds implementation-level variable '.0'
82+
_dir = ['x', 'y', 'z']
83+
if is_cpython:
84+
_dir.insert(0, '.0') # adds implementation-level variable '.0'
8885
# below, x and first/third y is local, second y and z is from the outer scope
8986
self.assertEqual([(x, y, z, dir()) for x in y for y in z],
9087
[(0, 0, range(3), _dir),
@@ -106,4 +103,26 @@ def apply(f, i): return f(i)
106103
res = [x for x in [y for y in apply(lambda i: range(i+x), x)]]
107104
self.assertEqual(res, [0, 1, 2, 3])
108105

106+
def test_ipy3_gh1132(self):
107+
"""https://github.com/IronLanguages/ironpython3/issues/1132"""
108+
109+
global global_test_list
110+
global_test_list = [1, 2]
111+
local_list = [3, 4]
112+
113+
res = [dir() for x in global_test_list for y in local_list]
114+
assert 'global_test_list' not in res[0]
115+
assert 'local_list' in res[0]
116+
117+
def test_ipy3_gh1082(self):
118+
"""https://github.com/IronLanguages/ironpython3/issues/1082"""
119+
120+
def foo():
121+
[x for y in range(2)]
122+
x = 2
123+
with self.assertRaises(NameError) as cm:
124+
foo()
125+
self.assertNotIsInstance(cm.exception, UnboundLocalError)
126+
self.assertEqual(cm.exception.args, ("free variable 'x' referenced before assignment in enclosing scope",))
127+
109128
run_test(__name__)

0 commit comments

Comments
 (0)