From 06f0f94a865354aa5ed86c1da75a9ea0da17bb00 Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Mon, 15 Feb 2016 18:59:32 -0500 Subject: [PATCH 01/13] Get comprehensions to work in Python 3.4. Since the function name may have a scope prefix, as in Thing.boom.., we look at the end of the name, not the whole name. --- byterun/pyobj.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/byterun/pyobj.py b/byterun/pyobj.py index f2924305..b8d24cf7 100644 --- a/byterun/pyobj.py +++ b/byterun/pyobj.py @@ -2,6 +2,7 @@ import collections import inspect +import re import types import six @@ -61,11 +62,12 @@ def __get__(self, instance, owner): return self def __call__(self, *args, **kwargs): - if PY2 and self.func_name in ["", "", ""]: + if re.search(r'<(listcomp|setcomp|dictcomp|genexpr)>$', self.func_name): # D'oh! http://bugs.python.org/issue19611 Py2 doesn't know how to # inspect set comprehensions, dict comprehensions, or generator # expressions properly. They are always functions of one argument, - # so just do the right thing. + # so just do the right thing. Py3.4 also would fail without this + # hack, for list comprehensions too. (Haven't checked for other 3.x.) assert len(args) == 1 and not kwargs, "Surprising comprehension!" callargs = {".0": args[0]} else: From 28a816551f0227e043bf09cc0011c6a2fe0c663c Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Mon, 15 Feb 2016 19:20:47 -0500 Subject: [PATCH 02/13] Make the LOAD_BUILD_CLASS opcode roughly work in Py3.4. It was completely broken; now it works if there's no func_closure or metaclass. (For now, just abort if those cases come up.) --- byterun/pyvm2.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/byterun/pyvm2.py b/byterun/pyvm2.py index 3e1a7277..239703d1 100644 --- a/byterun/pyvm2.py +++ b/byterun/pyvm2.py @@ -1029,7 +1029,7 @@ def byte_BUILD_CLASS(self): elif PY3: def byte_LOAD_BUILD_CLASS(self): # New in py3 - self.push(__build_class__) + self.push(build_class) def byte_STORE_LOCALS(self): self.frame.f_locals = self.pop() @@ -1037,3 +1037,26 @@ def byte_STORE_LOCALS(self): if 0: # Not in py2.7 def byte_SET_LINENO(self, lineno): self.frame.f_lineno = lineno + +if PY3: + def build_class(func, name, *bases, **kwds): + "Simplified (i.e., wrong) version of __build_class__." + assert isinstance(func, Function) + assert isinstance(name, str) + # This is simplified in that we don't yet handle metaclasses. So we do + # make sure there is no metaclass, before proceeding. + metaclass = kwds.get('metaclass') # (We don't just write 'metaclass=None' + # in the signature above because that's a syntax error in Py2.) + assert metaclass is None + assert not kwds + for base in bases: + assert type(base) == type + # OK, no metaclass; we may proceed. + # XXX What about func.func_closure? vm.make_frame() gives us no way to pass it in. + # We'll come back to fix this; for now, just make sure this case doesn't come up. + assert not func.func_closure + ns = {} + frame = func._vm.make_frame(func.func_code, f_globals=func.func_globals, f_locals=ns) + + func_result = func._vm.run_frame(frame) + return type(name, bases, ns) From a2214f6d75e9b421032608919337dc2e429bea0f Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Mon, 15 Feb 2016 19:28:05 -0500 Subject: [PATCH 03/13] Simpler code. --- byterun/pyvm2.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/byterun/pyvm2.py b/byterun/pyvm2.py index 239703d1..c3bdb550 100644 --- a/byterun/pyvm2.py +++ b/byterun/pyvm2.py @@ -1045,18 +1045,14 @@ def build_class(func, name, *bases, **kwds): assert isinstance(name, str) # This is simplified in that we don't yet handle metaclasses. So we do # make sure there is no metaclass, before proceeding. - metaclass = kwds.get('metaclass') # (We don't just write 'metaclass=None' - # in the signature above because that's a syntax error in Py2.) - assert metaclass is None - assert not kwds + assert not kwds # No explicit metaclass or keyword arguments to the metaclass. for base in bases: - assert type(base) == type + assert type(base) == type # No implicit metaclass from the bases. # OK, no metaclass; we may proceed. # XXX What about func.func_closure? vm.make_frame() gives us no way to pass it in. # We'll come back to fix this; for now, just make sure this case doesn't come up. assert not func.func_closure ns = {} frame = func._vm.make_frame(func.func_code, f_globals=func.func_globals, f_locals=ns) - - func_result = func._vm.run_frame(frame) + func._vm.run_frame(frame) return type(name, bases, ns) From 2190231c06622d042c75ff9d4088951581c07841 Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Mon, 15 Feb 2016 19:31:49 -0500 Subject: [PATCH 04/13] Make the NameError message match Py3's if PY3. --- byterun/pyvm2.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/byterun/pyvm2.py b/byterun/pyvm2.py index c3bdb550..f7781f82 100644 --- a/byterun/pyvm2.py +++ b/byterun/pyvm2.py @@ -421,7 +421,10 @@ def byte_LOAD_GLOBAL(self, name): elif name in f.f_builtins: val = f.f_builtins[name] else: - raise NameError("global name '%s' is not defined" % name) + if PY2: + raise NameError("global name '%s' is not defined" % name) + elif PY3: + raise NameError("name '%s' is not defined" % name) self.push(val) def byte_LOAD_DEREF(self, name): From fcea67e17d544e7dc1c38613c876a8ed4b365beb Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Mon, 15 Feb 2016 19:33:01 -0500 Subject: [PATCH 05/13] add py34 to tox.ini --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 4544cbc0..c7a5c858 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ # and then run "tox" from this directory. [tox] -envlist = py27, py33 +envlist = py27, py33, py34 [testenv] commands = From 6be106433615c517dd361a5e5de361784d41c33d Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Tue, 16 Feb 2016 14:59:29 -0500 Subject: [PATCH 06/13] Use a noncapturing regex since we don't need to capture. --- byterun/pyobj.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/byterun/pyobj.py b/byterun/pyobj.py index b8d24cf7..47f04da3 100644 --- a/byterun/pyobj.py +++ b/byterun/pyobj.py @@ -62,7 +62,7 @@ def __get__(self, instance, owner): return self def __call__(self, *args, **kwargs): - if re.search(r'<(listcomp|setcomp|dictcomp|genexpr)>$', self.func_name): + if re.search(r'<(?:listcomp|setcomp|dictcomp|genexpr)>$', self.func_name): # D'oh! http://bugs.python.org/issue19611 Py2 doesn't know how to # inspect set comprehensions, dict comprehensions, or generator # expressions properly. They are always functions of one argument, From 4c90233a069f15d6f593bac8cf671c126a11fd73 Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Tue, 16 Feb 2016 22:30:56 -0500 Subject: [PATCH 07/13] Remove func_locals from Function: it's not used and doesn't seem to be part of standard Python. --- byterun/pyobj.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/byterun/pyobj.py b/byterun/pyobj.py index 47f04da3..12bea900 100644 --- a/byterun/pyobj.py +++ b/byterun/pyobj.py @@ -24,7 +24,7 @@ def make_cell(value): class Function(object): __slots__ = [ 'func_code', 'func_name', 'func_defaults', 'func_globals', - 'func_locals', 'func_dict', 'func_closure', + 'func_dict', 'func_closure', '__name__', '__dict__', '__doc__', '_vm', '_func', ] @@ -35,7 +35,6 @@ def __init__(self, name, code, globs, defaults, closure, vm): self.func_name = self.__name__ = name or code.co_name self.func_defaults = tuple(defaults) self.func_globals = globs - self.func_locals = self._vm.frame.f_locals self.__dict__ = {} self.func_closure = closure self.__doc__ = code.co_consts[0] if code.co_consts else None From 70727da1e5c6f98075b2eeecc597cb81b43e7c4f Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Tue, 16 Feb 2016 23:25:20 -0500 Subject: [PATCH 08/13] Add a failing test for issue #17. --- tests/test_functions.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_functions.py b/tests/test_functions.py index f86fd131..7df69df7 100644 --- a/tests/test_functions.py +++ b/tests/test_functions.py @@ -207,6 +207,18 @@ def f4(g): assert answer == 54 """) + def test_closure_vars_from_static_parent(self): + self.assert_ok("""\ + def f(xs): + return lambda: xs[0] + + def g(h): + xs = 5 + lambda: xs + return h() + + assert g(f([42])) == 42 + """) class TestGenerators(vmtest.VmTestCase): def test_first(self): From 83699af4b902011a9962be2cbc8b9f828d7344e7 Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Tue, 16 Feb 2016 23:40:13 -0500 Subject: [PATCH 09/13] Pass fn.func_closure into new frames, and initialize cells using it. Fixes #17. (I haven't checked this code against the CPython code, but at least we're more correct than before, by the tests. --- byterun/pyobj.py | 27 ++++++++------------------- byterun/pyvm2.py | 12 ++++++------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/byterun/pyobj.py b/byterun/pyobj.py index 12bea900..ef6995ef 100644 --- a/byterun/pyobj.py +++ b/byterun/pyobj.py @@ -72,7 +72,7 @@ def __call__(self, *args, **kwargs): else: callargs = inspect.getcallargs(self._func, *args, **kwargs) frame = self._vm.make_frame( - self.func_code, callargs, self.func_globals, {} + self.func_code, callargs, self.func_globals, {}, self.func_closure ) CO_GENERATOR = 32 # flag for "this code uses yield" if self.func_code.co_flags & CO_GENERATOR: @@ -136,7 +136,7 @@ def set(self, value): class Frame(object): - def __init__(self, f_code, f_globals, f_locals, f_back): + def __init__(self, f_code, f_globals, f_locals, f_closure, f_back): self.f_code = f_code self.f_globals = f_globals self.f_locals = f_locals @@ -152,24 +152,13 @@ def __init__(self, f_code, f_globals, f_locals, f_back): self.f_lineno = f_code.co_firstlineno self.f_lasti = 0 - if f_code.co_cellvars: - self.cells = {} - if not f_back.cells: - f_back.cells = {} - for var in f_code.co_cellvars: - # Make a cell for the variable in our locals, or None. - cell = Cell(self.f_locals.get(var)) - f_back.cells[var] = self.cells[var] = cell - else: - self.cells = None - + self.cells = {} if f_code.co_cellvars or f_code.co_freevars else None + for var in f_code.co_cellvars: + # Make a cell for the variable in our locals, or None. + self.cells[var] = Cell(self.f_locals.get(var)) if f_code.co_freevars: - if not self.cells: - self.cells = {} - for var in f_code.co_freevars: - assert self.cells is not None - assert f_back.cells, "f_back.cells: %r" % (f_back.cells,) - self.cells[var] = f_back.cells[var] + assert len(f_code.co_freevars) == len(f_closure) + self.cells.update(zip(f_code.co_freevars, f_closure)) self.block_stack = [] self.generator = None diff --git a/byterun/pyvm2.py b/byterun/pyvm2.py index f7781f82..eda4913d 100644 --- a/byterun/pyvm2.py +++ b/byterun/pyvm2.py @@ -90,7 +90,7 @@ def push_block(self, type, handler=None, level=None): def pop_block(self): return self.frame.block_stack.pop() - def make_frame(self, code, callargs={}, f_globals=None, f_locals=None): + def make_frame(self, code, callargs={}, f_globals=None, f_locals=None, f_closure=None): log.info("make_frame: code=%r, callargs=%s" % (code, repper(callargs))) if f_globals is not None: f_globals = f_globals @@ -107,7 +107,7 @@ def make_frame(self, code, callargs={}, f_globals=None, f_locals=None): '__package__': None, } f_locals.update(callargs) - frame = Frame(code, f_globals, f_locals, self.frame) + frame = Frame(code, f_globals, f_locals, f_closure, self.frame) return frame def push_frame(self, frame): @@ -1052,10 +1052,10 @@ def build_class(func, name, *bases, **kwds): for base in bases: assert type(base) == type # No implicit metaclass from the bases. # OK, no metaclass; we may proceed. - # XXX What about func.func_closure? vm.make_frame() gives us no way to pass it in. - # We'll come back to fix this; for now, just make sure this case doesn't come up. - assert not func.func_closure ns = {} - frame = func._vm.make_frame(func.func_code, f_globals=func.func_globals, f_locals=ns) + frame = func._vm.make_frame(func.func_code, + f_globals=func.func_globals, + f_locals=ns, + f_closure=func.func_closure) func._vm.run_frame(frame) return type(name, bases, ns) From 90f028e952a5ea62ddc4bd9efd2b02bd6bea311b Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Wed, 17 Feb 2016 18:51:21 -0500 Subject: [PATCH 10/13] Populate the global environment for running tests in real Python for comparison; before, with an empty dict, eval would fill in __name__ as 'builtins', making results differ from running in the VM. (Also, extracted run_in_byterun and run_in_real_python methods, to help me figure out the problem.) --- tests/vmtest.py | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/vmtest.py b/tests/vmtest.py index 9e071838..763aa174 100644 --- a/tests/vmtest.py +++ b/tests/vmtest.py @@ -40,6 +40,19 @@ def assert_ok(self, code, raises=None): # Print the disassembly so we'll see it if the test fails. dis_code(code) + # Run the code through our VM and the real Python interpreter, for comparison. + vm_value, vm_exc, vm_stdout = self.run_in_byterun(code) + py_value, py_exc, py_stdout = self.run_in_real_python(code) + + self.assert_same_exception(vm_exc, py_exc) + self.assertEqual(vm_stdout.getvalue(), py_stdout.getvalue()) + self.assertEqual(vm_value, py_value) + if raises: + self.assertIsInstance(vm_exc, raises) + else: + self.assertIsNone(vm_exc) + + def run_in_byterun(self, code): real_stdout = sys.stdout # Run the code through our VM. @@ -64,32 +77,36 @@ def assert_ok(self, code, raises=None): raise vm_exc = e finally: + sys.stdout = real_stdout real_stdout.write("-- stdout ----------\n") real_stdout.write(vm_stdout.getvalue()) - # Run the code through the real Python interpreter, for comparison. + return vm_value, vm_exc, vm_stdout + + def run_in_real_python(self, code): + real_stdout = sys.stdout py_stdout = six.StringIO() sys.stdout = py_stdout py_value = py_exc = None - globs = {} + globs = { + '__builtins__': __builtins__, + '__name__': '__main__', + '__doc__': None, + '__package__': None, + } + try: py_value = eval(code, globs, globs) except AssertionError: # pragma: no cover raise except Exception as e: py_exc = e + finally: + sys.stdout = real_stdout - sys.stdout = real_stdout - - self.assert_same_exception(vm_exc, py_exc) - self.assertEqual(vm_stdout.getvalue(), py_stdout.getvalue()) - self.assertEqual(vm_value, py_value) - if raises: - self.assertIsInstance(vm_exc, raises) - else: - self.assertIsNone(vm_exc) + return py_value, py_exc, py_stdout def assert_same_exception(self, e1, e2): """Exceptions don't implement __eq__, check it ourselves.""" From 9541a714baa14cc943f58ab8f7eae951f1aaa31b Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Wed, 17 Feb 2016 19:38:51 -0500 Subject: [PATCH 11/13] Fill in build_class with the metaclass logic (following the CPython impl). --- byterun/pyvm2.py | 58 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/byterun/pyvm2.py b/byterun/pyvm2.py index eda4913d..80d43c3b 100644 --- a/byterun/pyvm2.py +++ b/byterun/pyvm2.py @@ -15,7 +15,7 @@ PY3, PY2 = six.PY3, not six.PY3 -from .pyobj import Frame, Block, Method, Function, Generator +from .pyobj import Frame, Block, Method, Function, Generator, Cell log = logging.getLogger(__name__) @@ -1043,19 +1043,49 @@ def byte_SET_LINENO(self, lineno): if PY3: def build_class(func, name, *bases, **kwds): - "Simplified (i.e., wrong) version of __build_class__." - assert isinstance(func, Function) - assert isinstance(name, str) - # This is simplified in that we don't yet handle metaclasses. So we do - # make sure there is no metaclass, before proceeding. - assert not kwds # No explicit metaclass or keyword arguments to the metaclass. - for base in bases: - assert type(base) == type # No implicit metaclass from the bases. - # OK, no metaclass; we may proceed. - ns = {} + "Like __build_class__ in bltinmodule.c, but running in the byterun VM." + if not isinstance(func, Function): + raise TypeError("func must be a function") + if not isinstance(name, str): + raise TypeError("name is not a string") + metaclass = kwds.get('metaclass') + if 'metaclass' in kwds: del kwds['metaclass'] + # (We don't just write 'metaclass=None' in the signature above + # because that's a syntax error in Py2.) + if metaclass is None: + metaclass = type(bases[0]) if bases else type + if isinstance(metaclass, type): + metaclass = calculate_metaclass(metaclass, bases) + + try: + prepare = metaclass.__prepare__ + except AttributeError: + namespace = {} + else: + namespace = prepare(name, bases, **kwds) + + # Execute the body of func. This is the step that would go wrong if + # we tried to use the built-in __build_class__, because __build_class__ + # does not call func, it magically executes its body directly, as we + # do here (except we invoke our VirtualMachine instead of CPython's). frame = func._vm.make_frame(func.func_code, f_globals=func.func_globals, - f_locals=ns, + f_locals=namespace, f_closure=func.func_closure) - func._vm.run_frame(frame) - return type(name, bases, ns) + cell = func._vm.run_frame(frame) + + cls = metaclass(name, bases, namespace) + if isinstance(cell, Cell): + cell.set(cls) + return cls + + def calculate_metaclass(metaclass, bases): + "Determine the most derived metatype." + winner = metaclass + for base in bases: + t = type(base) + if issubclass(t, winner): + winner = t + elif not issubclass(winner, t): + raise TypeError("metaclass conflict", winner, t) + return winner From 86c702b305895552c5fec12974b431ebe94e38a7 Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Wed, 17 Feb 2016 19:45:19 -0500 Subject: [PATCH 12/13] add to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 4680226d..9a042123 100644 --- a/AUTHORS +++ b/AUTHORS @@ -2,3 +2,4 @@ Paul Swartz Ned Batchelder Allison Kaptur Laura Lindzey +Darius Bacon From 31d4e46e07e9b8c12d7bf6f0983e4fb43dd85be6 Mon Sep 17 00:00:00 2001 From: Darius Bacon Date: Tue, 23 Feb 2016 15:45:45 -0500 Subject: [PATCH 13/13] simplify popping from kwds --- byterun/pyvm2.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/byterun/pyvm2.py b/byterun/pyvm2.py index 80d43c3b..b2fec891 100644 --- a/byterun/pyvm2.py +++ b/byterun/pyvm2.py @@ -1048,8 +1048,7 @@ def build_class(func, name, *bases, **kwds): raise TypeError("func must be a function") if not isinstance(name, str): raise TypeError("name is not a string") - metaclass = kwds.get('metaclass') - if 'metaclass' in kwds: del kwds['metaclass'] + metaclass = kwds.pop('metaclass', None) # (We don't just write 'metaclass=None' in the signature above # because that's a syntax error in Py2.) if metaclass is None: