Py36 support #31

vrthra · 2018-01-30T19:35:40Z

Adding python 3.6 support.

Includes the python 3.4 patch from @darius so that the tests pass. If that gets committed, I will remove the last patch from this PR.

llllllllll · 2018-01-30T21:03:51Z

Thanks for working on this! I will try to look at this either tonight or tomorrow after work.

vrthra · 2018-02-04T11:39:18Z

@llllllllll ping -- any thing I can do?

llllllllll

Sorry it took so long, I left some comments. This is pretty awesome, thanks!

llllllllll · 2018-02-05T07:13:19Z

byterun/pyobj.py

            # 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.)


should we check this?

This is another from #20 PR by @darius

llllllllll · 2018-02-05T07:16:52Z

byterun/pyvm2.py

        f = self.frame
        opoffset = f.f_lasti
-        byteCode = byteint(f.f_code.co_code[opoffset])
+        if f.py36_opcodes:


why do we need special processing here? the non-3.6 branch looks like it should handle this fine.

Only until 3.4. I will change the special processing to check for 3.4 onward if that is required.

So, using the opcodes = dis.get_instructions API is kind of hard in versions before Python 3.6 because on jumps, the VM sets the bytecode counter to the offset to be jumped to. In versions before Python 3.6, there is no direct correspondence to between the opcodes index and the offset to be jumped to -- and has to be computed for each jump. For Python 3.6, and above, there is a direct correspondence because each bytecode is now fixed width, and one can compute the jump index directly by dividing target by 2. Given the significant simplification Python 3.6 has provided, along with the API for bytecode iteration, perhaps we should special case from Python 3.6 onward?

@vrthra reports:

opcodes = dis.get_instructions API is kind of hard in versions before Python 3.6 because o[f] jumps

While there are more serious problems mentioned below, by using xdis in my fork, the newer and better APIs in later releases can be applied to bytecode on older versions of Python.

the VM sets the bytecode counter to the offset to be jumped to.

In fact, this is wrong at least as far as emulating CPython semantics because this value is stored in the frame's f_lasti.

While this may be tolerable for reading log traces, if you were to try to hook this up with a debugger it would be intolerable: you can't report a frame's f_lasti as the instruction that would be run if the current instruction had finished and not jumped, returned or raised an exception. Instead f_lasti needs to show the current instruction as it does in CPython.

perhaps we should special case from Python 3.6 onward?

Although the specific problems you mention are addressed above, something like this in x-python goes on with opcodes. Here the advantages are

pedagocial

cleaner separatation of code, which leads to

scalability

llllllllll · 2018-02-05T07:17:21Z

byterun/pyvm2.py

        arg = None
        arguments = []
+        if f.py36_opcodes and byteCode == dis.EXTENDED_ARG:


I don't think extended_arg is new to 3.6, can we remove the first part of this check?

As I mentioned previously, this approach only works for Python 3.6 and above, and only if we are using the get_instructions api.

llllllllll · 2018-02-05T07:19:37Z

byterun/pyvm2.py

-            intArg = byteint(arg[0]) + (byteint(arg[1]) << 8)
+            if f.py36_opcodes:
+                intArg = currentOp.arg
+            else:


also here, I am not sure why we are handling 3.6 in such a special way, the code below looks correct in either case.

(I misunderstood you earlier.) From 3.6 on, there is a subtle difference. All ops have arguments. Which means that we do not need to increment the f_lasti. Given that we have to special case py36 anyway, we might as well rely on the get_instructions API for versions from now on to get the actual argument rather than doing the bit manipulation ourselves.

llllllllll · 2018-02-05T07:23:51Z

byterun/pyvm2.py

+        elts = self.popn(count)
+        self.push(tuple(e for l in elts for e in l))
+
+    def byte_BUILD_TUPLE_UNPACK(self, count):


in the C source, BUILD_TUPLE_UNPACK, BUILD_TUPLE_UNPACK_WITH_CALL, and BUILD_LIST_UNPACK have the same target, should we reflect that by having them share the same target here?

I will update to reflect that that.

llllllllll · 2018-02-05T07:27:49Z

byterun/pyvm2.py

+
+    def byte_BUILD_MAP(self, count):
+        # Pushes a new dictionary on to stack.
+        if not(six.PY3 and sys.version_info.minor >= 5):


this might be more clear as sys.version_info[:2] < (3, 5). at first I thought this was flipped

I will update that

llllllllll · 2018-02-05T07:29:05Z

byterun/pyvm2.py

+        # dictionary holds count entries: {..., TOS3: TOS2, TOS1:TOS}
+        # updated in version 3.5
+        kvs = {}
+        for i in range(0, count):


range(n) is shorthand for range(0, n) and is a more common form

Thanks, I will change that.

llllllllll · 2018-02-05T07:30:44Z

byterun/pyvm2.py

        globs = self.frame.f_globals
-        fn = Function(name, code, globs, defaults, None, self)
+        if PY3 and sys.version_info.minor >= 6:
+            closure = self.pop() if (argc & 0x8) else None


is there a reason to use hex-literals for values under 10?

@llllllllll I took the comparisons from the Python bytecode documentation directly, which uses these hex literals.

llllllllll · 2018-02-05T07:33:15Z

byterun/pyvm2.py


        def byte_STORE_LOCALS(self):
            self.frame.f_locals = self.pop()

    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):


did we need to rewrite this function? I think the scope of this project is confined to the interpreter loop and function calling. I may have missed some new reason why this needs to be in Python now though.

See the comment further down in the function, and my remarks on the pull request #20 -- it might be better to discuss this over there instead. I'm not sure if any of your other comments are about my old submission instead of @vrthra's, since I've forgotten what was in it by now.

I vaguely remember trying alternatives, but couldn't find any better way to get Py3.4 supported.

llllllllll · 2018-02-05T07:35:45Z

byterun/pyobj.py

        self.f_code = f_code
+        self.py36_opcodes = list(dis.get_instructions(self.f_code)) \


Is there a reason to prefer this over just reading the f_code? If anything, 3.6 makes this very simple with the fixed width instructions.

Given that Python is now exposing the API to iterate, and given that the bytecode format is not considered by Python project to be part of their public API, I think we should use their public API rather than read and interpret f_code.

Unfortunately the API is only available from 3.4 so we have to special case this for any code from 3.4 onwards.

vrthra force-pushed the py36 branch 10 times, most recently from 8b42c5a to ebfdc5c Compare January 31, 2018 12:02

vrthra and others added 10 commits January 31, 2018 13:03

Add python 3.6 word code support

2210203

tests/test_basic.py: Fix printing for python3

b7db929

byterun/pyvm2.py: Fix BUILD_MAP and add BUILD_CONST_KEY_MAP

a429e8f

byterun/pyvm2.py: Add BUILD_TUPLE_UNPACK and ..WITH_CALL

4011ef7

added GET_YIELD_FROM_ITER

ac20f46

byterun/pyvm2.py: add WITH_CLEANUP_START .. FINISH

0c4d8fc

byterun/pyvm2.py: Fix CALL_FUNCTION_KW and add _EX (need nedbat#20 PR)

d3b9088

byterun/pyvm2.py: MAKE_FUNCTION

9c386c7

authors

fe1ba4f

PR nedbat#20 from Darius

a07642f

vrthra force-pushed the py36 branch from ebfdc5c to a07642f Compare January 31, 2018 12:04

vrthra mentioned this pull request Jan 31, 2018

Ensure that import goes through byterun (WIP) #32

Open

llllllllll reviewed Feb 5, 2018

View reviewed changes

vrthra added 3 commits February 12, 2018 14:47

byte_CALL_FUNCTION_KW for py34

1765ce0

Updated with changes suggested by the reviewer

5fce7b9

Add a few more opcodes

e39fec9

vrthra force-pushed the py36 branch from 0707033 to e39fec9 Compare February 13, 2018 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Py36 support #31

Py36 support #31

vrthra commented Jan 30, 2018 •

edited

Loading

llllllllll commented Jan 30, 2018

vrthra commented Feb 4, 2018

llllllllll left a comment

llllllllll Feb 5, 2018

vrthra Feb 6, 2018

llllllllll Feb 5, 2018

vrthra Feb 6, 2018

vrthra Feb 6, 2018

rocky Jun 2, 2020 •

edited

Loading

llllllllll Feb 5, 2018

vrthra Feb 6, 2018

llllllllll Feb 5, 2018

vrthra Feb 6, 2018 •

edited

Loading

llllllllll Feb 5, 2018

vrthra Feb 6, 2018

llllllllll Feb 5, 2018

vrthra Feb 6, 2018

llllllllll Feb 5, 2018

vrthra Feb 6, 2018

llllllllll Feb 5, 2018

vrthra Feb 6, 2018 •

edited

Loading

llllllllll Feb 5, 2018

darius Feb 5, 2018

llllllllll Feb 5, 2018

vrthra Feb 6, 2018 •

edited

Loading

		self.f_code = f_code
		self.py36_opcodes = list(dis.get_instructions(self.f_code)) \

Py36 support #31

Are you sure you want to change the base?

Py36 support #31

Conversation

vrthra commented Jan 30, 2018 • edited Loading

llllllllll commented Jan 30, 2018

vrthra commented Feb 4, 2018

llllllllll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rocky Jun 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrthra Feb 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrthra Feb 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrthra Feb 6, 2018 • edited Loading

Choose a reason for hiding this comment

vrthra commented Jan 30, 2018 •

edited

Loading

rocky Jun 2, 2020 •

edited

Loading

vrthra Feb 6, 2018 •

edited

Loading

vrthra Feb 6, 2018 •

edited

Loading

vrthra Feb 6, 2018 •

edited

Loading