[Jython-checkins] jython: Dict and set comps no longer leak iteration variables
jim.baker
jython-checkins at python.org
Fri Jan 9 01:08:00 CET 2015
https://hg.python.org/jython/rev/e270e881c84c
changeset: 7521:e270e881c84c
user: Jim Baker <jim.baker at rackspace.com>
date: Thu Jan 08 16:55:51 2015 -0700
summary:
Dict and set comps no longer leak iteration variables
Dict and set comprehensions were acting like list comprehensions, in
that their internal iteration variable was visible. Now they are
hidden away, similar to a generator expression (and using the same
internal logic in the compiler).
files:
Lib/test/test_dictcomps.py | 65 ----
Lib/test/test_setcomps.py | 154 ----------
src/org/python/compiler/CodeCompiler.java | 110 +++++--
src/org/python/compiler/ScopesCompiler.java | 52 +-
4 files changed, 106 insertions(+), 275 deletions(-)
diff --git a/Lib/test/test_dictcomps.py b/Lib/test/test_dictcomps.py
deleted file mode 100644
--- a/Lib/test/test_dictcomps.py
+++ /dev/null
@@ -1,65 +0,0 @@
-
-doctests = """
-
- # FIXME: Jython leaks dictcomp scope.
- #>>> k = "old value"
- #>>> a = {0: None, 1: None, 2: None, 3: None, 4: None, 5: None, 6: None, 7: None, 8: None, 9: None}
- #>>> b = { k: None for k in range(10) }
- #>>> a == b
- #True
- #>>> k
- #'old value'
-
- >>> a = {0: 10, 1: 11, 2: 12, 3: 13, 4: 14, 5: 15, 6: 16, 7: 17, 8: 18, 9: 19}
- >>> b = { k: k+10 for k in range(10) }
- >>> a == b
- True
-
- >>> g = "Global variable"
- >>> a = {0: 'Global variable', 1: 'Global variable', 2: 'Global variable', 3: 'Global variable', 4: 'Global variable', 5: 'Global variable', 6: 'Global variable', 7: 'Global variable', 8: 'Global variable', 9: 'Global variable'}
- >>> b = { k: g for k in range(10) }
- >>> a == b
- True
-
- >>> a = {0: 0, 1: 1, 2: 2, 3: 3, 4: 4, 5: 5, 6: 6, 7: 7, 8: 8, 9: 9}
- >>> b = { k: v for k in range(10) for v in range(10) if k == v }
- >>> a == b
- True
-
- >>> a = {9: 1, 18: 2, 19: 2, 27: 3, 28: 3, 29: 3, 36: 4, 37: 4, 38: 4, 39: 4, 45: 5, 46: 5, 47: 5, 48: 5, 49: 5, 54: 6, 55: 6, 56: 6, 57: 6, 58: 6, 59: 6, 63: 7, 64: 7, 65: 7, 66: 7, 67: 7, 68: 7, 69: 7, 72: 8, 73: 8, 74: 8, 75: 8, 76: 8, 77: 8, 78: 8, 79: 8, 81: 9, 82: 9, 83: 9, 84: 9, 85: 9, 86: 9, 87: 9, 88: 9, 89: 9}
- >>> b = { k: v for v in range(10) for k in range(v*9, v*10) }
- >>> a == b
- True
-
- >>> { x: y for y, x in ((1, 2), (3, 4)) } = 5 # doctest: +IGNORE_EXCEPTION_DETAIL
- Traceback (most recent call last):
- ...
- SyntaxError: ...
-
- >>> { x: y for y, x in ((1, 2), (3, 4)) } += 5 # doctest: +IGNORE_EXCEPTION_DETAIL
- Traceback (most recent call last):
- ...
- SyntaxError: ...
-
-"""
-
-__test__ = {'doctests' : doctests}
-
-def test_main(verbose=None):
- import sys
- from test import test_support
- from test import test_dictcomps
- test_support.run_doctest(test_dictcomps, verbose)
-
- # verify reference counting
- if verbose and hasattr(sys, "gettotalrefcount"):
- import gc
- counts = [None] * 5
- for i in range(len(counts)):
- test_support.run_doctest(test_dictcomps, verbose)
- gc.collect()
- counts[i] = sys.gettotalrefcount()
- print(counts)
-
-if __name__ == "__main__":
- test_main(verbose=True)
diff --git a/Lib/test/test_setcomps.py b/Lib/test/test_setcomps.py
deleted file mode 100644
--- a/Lib/test/test_setcomps.py
+++ /dev/null
@@ -1,154 +0,0 @@
-doctests = """
-########### Tests mostly copied from test_listcomps.py ############
-
-Test simple loop with conditional
-
- >>> sum({i*i for i in range(100) if i&1 == 1})
- 166650
-
-Test simple case
-
- >>> {2*y + x + 1 for x in (0,) for y in (1,)}
- set([3])
-
-Test simple nesting
-
- >>> list(sorted({(i,j) for i in range(3) for j in range(4)}))
- [(0, 0), (0, 1), (0, 2), (0, 3), (1, 0), (1, 1), (1, 2), (1, 3), (2, 0), (2, 1), (2, 2), (2, 3)]
-
-Test nesting with the inner expression dependent on the outer
-
- >>> list(sorted({(i,j) for i in range(4) for j in range(i)}))
- [(1, 0), (2, 0), (2, 1), (3, 0), (3, 1), (3, 2)]
-
-Make sure the induction variable is not exposed
-
- #FIXME: scope leaks in Jython.
- #>>> i = 20
- #>>> sum({i*i for i in range(100)})
- #328350
- #
- #>>> i
- #20
-
-Verify that syntax error's are raised for setcomps used as lvalues
-
- >>> {y for y in (1,2)} = 10 # doctest: +IGNORE_EXCEPTION_DETAIL
- Traceback (most recent call last):
- ...
- SyntaxError: ...
-
- >>> {y for y in (1,2)} += 10 # doctest: +IGNORE_EXCEPTION_DETAIL
- Traceback (most recent call last):
- ...
- SyntaxError: ...
-
-
-Make a nested set comprehension that acts like set(range())
-
- >>> def srange(n):
- ... return {i for i in range(n)}
- >>> list(sorted(srange(10)))
- [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
-
-Same again, only as a lambda expression instead of a function definition
-
- >>> lrange = lambda n: {i for i in range(n)}
- >>> list(sorted(lrange(10)))
- [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
-
-Generators can call other generators:
-
- >>> def grange(n):
- ... for x in {i for i in range(n)}:
- ... yield x
- >>> list(sorted(grange(5)))
- [0, 1, 2, 3, 4]
-
-
-Make sure that None is a valid return value
-
- >>> {None for i in range(10)}
- set([None])
-
-########### Tests for various scoping corner cases ############
-
-Return lambdas that use the iteration variable as a default argument
-
- >>> items = {(lambda i=i: i) for i in range(5)}
- >>> {x() for x in items} == set(range(5))
- True
-
-Same again, only this time as a closure variable
-
- >>> items = {(lambda: i) for i in range(5)}
- >>> {x() for x in items}
- set([4])
-
-Another way to test that the iteration variable is local to the list comp
-
- #FIXME: scope leaks in Jython.
- #>>> items = {(lambda: i) for i in range(5)}
- #>>> i = 20
- #>>> {x() for x in items}
- #set([4])
-
-And confirm that a closure can jump over the list comp scope
-
- >>> items = {(lambda: y) for i in range(5)}
- >>> y = 2
- >>> {x() for x in items}
- set([2])
-
-We also repeat each of the above scoping tests inside a function
-
- >>> def test_func():
- ... items = {(lambda i=i: i) for i in range(5)}
- ... return {x() for x in items}
- >>> test_func() == set(range(5))
- True
-
- >>> def test_func():
- ... items = {(lambda: i) for i in range(5)}
- ... return {x() for x in items}
- >>> test_func()
- set([4])
-
- #FIXME: scope leaks in Jython.
- #>>> def test_func():
- #... items = {(lambda: i) for i in range(5)}
- #... i = 20
- #... return {x() for x in items}
- #>>> test_func()
- #set([4])
-
- >>> def test_func():
- ... items = {(lambda: y) for i in range(5)}
- ... y = 2
- ... return {x() for x in items}
- >>> test_func()
- set([2])
-
-"""
-
-
-__test__ = {'doctests' : doctests}
-
-def test_main(verbose=None):
- import sys
- from test import test_support
- from test import test_setcomps
- test_support.run_doctest(test_setcomps, verbose)
-
- # verify reference counting
- if verbose and hasattr(sys, "gettotalrefcount"):
- import gc
- counts = [None] * 5
- for i in range(len(counts)):
- test_support.run_doctest(test_setcomps, verbose)
- gc.collect()
- counts[i] = sys.gettotalrefcount()
- print(counts)
-
-if __name__ == "__main__":
- test_main(verbose=True)
diff --git a/src/org/python/compiler/CodeCompiler.java b/src/org/python/compiler/CodeCompiler.java
--- a/src/org/python/compiler/CodeCompiler.java
+++ b/src/org/python/compiler/CodeCompiler.java
@@ -2176,53 +2176,30 @@
return null;
}
-
@Override
public Object visitSetComp(SetComp node) throws Exception {
code.new_(p(PySet.class));
-
code.dup();
- code.invokespecial(p(PySet.class), "<init>", sig(Void.TYPE));
-
- code.dup();
-
- code.ldc("add");
-
- code.invokevirtual(p(PyObject.class), "__getattr__", sig(PyObject.class, String.class));
- String tmp_append = "_{" + node.getLine() + "_" + node.getCharPositionInLine() + "}";
-
- java.util.List<expr> args = new ArrayList<expr>();
- args.add(node.getInternalElt());
-
- finishComp(node, args, node.getInternalGenerators(), tmp_append);
-
+ visitInternalGenerators(node, node.getInternalElt(), node.getInternalGenerators());
+ code.invokespecial(p(PySet.class), "<init>", sig(Void.TYPE, PyObject.class));
return null;
}
@Override
public Object visitDictComp(DictComp node) throws Exception {
code.new_(p(PyDictionary.class));
-
code.dup();
code.invokespecial(p(PyDictionary.class), "<init>", sig(Void.TYPE));
-
code.dup();
-
- code.ldc("__setitem__");
-
- code.invokevirtual(p(PyDictionary.class), "__getattr__", sig(PyObject.class, String.class));
- String tmp_append = "_{" + node.getLine() + "_" + node.getCharPositionInLine() + "}";
-
- java.util.List<expr> args = new ArrayList<expr>();
- args.add(node.getInternalKey());
- args.add(node.getInternalValue());
-
- finishComp(node, args, node.getInternalGenerators(), tmp_append);
-
+ java.util.List<expr> kv = Arrays.asList(node.getInternalKey(), node.getInternalValue());
+ visitInternalGenerators(
+ node,
+ new Tuple(node, kv, expr_contextType.UNDEFINED),
+ node.getInternalGenerators());
+ code.invokevirtual(p(PyDictionary.class), "update", sig(Void.TYPE, PyObject.class));
return null;
}
-
private void finishComp(expr node, java.util.List<expr> args, java.util.List<comprehension> generators,
String tmp_append) throws Exception {
set(new Name(node, tmp_append, expr_contextType.Store));
@@ -2586,6 +2563,77 @@
return null;
}
+ private Object visitInternalGenerators(expr node, expr elt, java.util.List<comprehension> generators)
+ throws Exception {
+ String bound_exp = "_(x)";
+
+ setline(node);
+
+ code.new_(p(PyFunction.class));
+ code.dup();
+ loadFrame();
+ code.getfield(p(PyFrame.class), "f_globals", ci(PyObject.class));
+
+ ScopeInfo scope = module.getScopeInfo(node);
+
+ int emptyArray = makeArray(new ArrayList<expr>());
+ code.aload(emptyArray);
+ scope.setup_closure();
+ scope.dump();
+
+ stmt n = new Expr(node, new Yield(node, elt));
+
+ expr iter = null;
+ for (int i = generators.size() - 1; i >= 0; i--) {
+ comprehension comp = generators.get(i);
+ for (int j = comp.getInternalIfs().size() - 1; j >= 0; j--) {
+ java.util.List<stmt> bod = new ArrayList<stmt>();
+ bod.add(n);
+ n = new If(comp.getInternalIfs().get(j), comp.getInternalIfs().get(j), bod,
+ new ArrayList<stmt>());
+ }
+ java.util.List<stmt> bod = new ArrayList<stmt>();
+ bod.add(n);
+ if (i != 0) {
+ n = new For(comp, comp.getInternalTarget(), comp.getInternalIter(), bod,
+ new ArrayList<stmt>());
+ } else {
+ n = new For(comp, comp.getInternalTarget(), new Name(node, bound_exp,
+ expr_contextType.Load), bod, new ArrayList<stmt>());
+ iter = comp.getInternalIter();
+ }
+ }
+
+ java.util.List<stmt> bod = new ArrayList<stmt>();
+ bod.add(n);
+ module.codeConstant(new Suite(node, bod), "<genexpr>", true,
+ className, false, false,
+ node.getLine(), scope, cflags).get(code);
+
+ code.aconst_null();
+ if (!makeClosure(scope)) {
+ code.invokespecial(p(PyFunction.class), "<init>", sig(Void.TYPE, PyObject.class,
+ PyObject[].class, PyCode.class, PyObject.class));
+ } else {
+ code.invokespecial(p(PyFunction.class), "<init>", sig(Void.TYPE, PyObject.class,
+ PyObject[].class, PyCode.class, PyObject.class, PyObject[].class));
+ }
+ int genExp = storeTop();
+
+ visit(iter);
+ code.aload(genExp);
+ code.freeLocal(genExp);
+ code.swap();
+ code.invokevirtual(p(PyObject.class), "__iter__", sig(PyObject.class));
+ loadThreadState();
+ code.swap();
+ code.invokevirtual(p(PyObject.class), "__call__",
+ sig(PyObject.class, ThreadState.class, PyObject.class));
+ freeArray(emptyArray);
+
+ return null;
+ }
+
@Override
public Object visitGeneratorExp(GeneratorExp node) throws Exception {
String bound_exp = "_(x)";
diff --git a/src/org/python/compiler/ScopesCompiler.java b/src/org/python/compiler/ScopesCompiler.java
--- a/src/org/python/compiler/ScopesCompiler.java
+++ b/src/org/python/compiler/ScopesCompiler.java
@@ -19,15 +19,18 @@
import org.python.antlr.ast.Name;
import org.python.antlr.ast.Return;
import org.python.antlr.ast.SetComp;
+import org.python.antlr.ast.Tuple;
import org.python.antlr.ast.With;
import org.python.antlr.ast.Yield;
import org.python.antlr.ast.arguments;
+import org.python.antlr.ast.comprehension;
import org.python.antlr.ast.expr_contextType;
import org.python.antlr.base.expr;
import org.python.antlr.base.stmt;
import org.python.core.ParserFacade;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Hashtable;
import java.util.Stack;
import java.util.List;
@@ -290,21 +293,15 @@
}
@Override
- public Object visitSetComp(SetComp node) throws Exception {
- String tmp = "_{" + node.getLine() + "_" + node.getCharPositionInLine()
- + "}";
- cur.addBound(tmp);
- traverse(node);
- return null;
+ public Object visitDictComp(DictComp node) throws Exception {
+ java.util.List<expr> kv = Arrays.asList(node.getInternalKey(), node.getInternalValue());
+ return visitInternalGenerators(
+ node, new Tuple(node, kv, expr_contextType.UNDEFINED), node.getInternalGenerators());
}
@Override
- public Object visitDictComp(DictComp node) throws Exception {
- String tmp = "_{" + node.getLine() + "_" + node.getCharPositionInLine()
- + "}";
- cur.addBound(tmp);
- traverse(node);
- return null;
+ public Object visitSetComp(SetComp node) throws Exception {
+ return visitInternalGenerators(node, node.getInternalElt(), node.getInternalGenerators());
}
@Override
@@ -324,11 +321,11 @@
return null;
}
- @Override
- public Object visitGeneratorExp(GeneratorExp node) throws Exception {
+ private Object visitInternalGenerators(expr node, expr elt, java.util.List<comprehension> generators)
+ throws Exception {
// The first iterator is evaluated in the outer scope
- if (node.getInternalGenerators() != null && node.getInternalGenerators().size() > 0) {
- visit(node.getInternalGenerators().get(0).getInternalIter());
+ if (generators != null && generators.size() > 0) {
+ visit(generators.get(0).getInternalIter());
}
String bound_exp = "_(x)";
String tmp = "_(" + node.getLine() + "_" + node.getCharPositionInLine()
@@ -345,23 +342,23 @@
cur.defineAsGenerator(node);
cur.yield_count++;
// The reset of the iterators are evaluated in the inner scope
- if (node.getInternalElt() != null) {
- visit(node.getInternalElt());
+ if (elt != null) {
+ visit(elt);
}
- if (node.getInternalGenerators() != null) {
- for (int i = 0; i < node.getInternalGenerators().size(); i++) {
- if (node.getInternalGenerators().get(i) != null) {
+ if (generators != null) {
+ for (int i = 0; i < generators.size(); i++) {
+ if (generators.get(i) != null) {
if (i == 0) {
- visit(node.getInternalGenerators().get(i).getInternalTarget());
- if (node.getInternalGenerators().get(i).getInternalIfs() != null) {
- for (expr cond : node.getInternalGenerators().get(i).getInternalIfs()) {
+ visit(generators.get(i).getInternalTarget());
+ if (generators.get(i).getInternalIfs() != null) {
+ for (expr cond : generators.get(i).getInternalIfs()) {
if (cond != null) {
visit(cond);
}
}
}
} else {
- visit(node.getInternalGenerators().get(i));
+ visit(generators.get(i));
}
}
}
@@ -372,6 +369,11 @@
}
@Override
+ public Object visitGeneratorExp(GeneratorExp node) throws Exception {
+ return visitInternalGenerators(node, node.getInternalElt(), node.getInternalGenerators());
+ }
+
+ @Override
public Object visitWith(With node) throws Exception {
cur.max_with_count++;
traverse(node);
--
Repository URL: https://hg.python.org/jython
More information about the Jython-checkins
mailing list