-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Py3.4 support. #20
base: master
Are you sure you want to change the base?
Conversation
… have a scope prefix, as in Thing.boom.<locals>.<listcomp>, we look at the end of the name, not the whole name.
…tely broken; now it works if there's no func_closure or metaclass. (For now, just abort if those cases come up.)
@darius: I'm unlikely to do more work on byterun, so maybe we should find another owner... |
I guess I could volunteer, though I'd rather someone else owned it, like you or @akaptur. My intent for this work here was to make a cut-down version of byterun for just py3.4 and the bytecodes that my compiler uses, so people could more easily read the whole thing. I just figured it's more sensible/helpful to do the porting work first on the real byterun. |
…e part of standard Python.
…ixes nedbat#17. (I haven't checked this code against the CPython code, but at least we're more correct than before, by the tests.
…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.)
I also ported a half-dozen tests for metaclass logic over from CPython 3.4's test_metaclass.py (they're all passing). They're not checked in yet because I guess we'd need to add the CPython license terms to LICENSE and I don't really understand those legalities. Besides that, and making sure my fix for #17 follows the CPython logic, I think this is done. I'm going to get on with my subset, but let me know if it'd help to clean this up somehow, check in the new tests, etc. |
Oh, and I still need to install 3.3 and make sure that's still passing. |
@@ -207,6 +207,18 @@ def f4(g): | |||
assert answer == 54 | |||
""") | |||
|
|||
def test_closure_vars_from_static_parent(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darius I like this test. I've added it in my fork here rocky@e4782c0
What I like about this kind of test is that it simplifies testing. Instead of comparing output in the test runner, all the test runner has to do is run the program and the execution of the program tests itself.
And that way you can simply run the interpeter without any test framework to see if the issue is resoved. Real simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gotten around to studying why this test fails and it uncovers a fundamental flaw with how opcode arguments are passed in the current design, at least for "freeops" I think. Lacking a better place, I'll note it here.
It may be a while before I fix it in x-python.
The "deref" opcodes like LOAD_DEREF
, also known as opcodes in the hasfree[]
opcodes list, are passed a string name, but in the Python reference library of course, the operand is really an integer index.
In this inerpreter, the frame cells are a dictionary whereas in CPython frame cells are an array.
Normally, everything is fine because names are distinct. For example, you can't have two local variables called a
. But in the cells array, names don't have to be distinct.
In particular in this test, the variable xs
appears in two scopes. So when a LOAD_DEREF
does its lookup into a dictionary, it finds the wrong value since the dictonary can only have one key with value xs
.
With these patches the tests pass in Py3.4. This isn't done yet because I had to reimplement
__build_class__
, and that reimplementation doesn't yet support metaclasses or closures. It looks like the closure support will need to share a fix with issue #17, so I'm going to take a look at that next. I'm just opening this pull request now to let you know what's up -- maybe I should've opened an issue instead?Another idea could be to wrap the built-in
__build_class__
primitive, passing it a Python function object for which it'd do the right thing, but__build_class__
is magical enough that that seems difficult.There are also fixes for a couple of other problems that came up in running the tests in 3.4.
(I haven't tested 3.5 or 3.6, or 3.3 yet either. It does still pass the tests in 2.7.)
My motivation is to get my Python bytecode compiler https://github.com/darius/500lines/blob/master/bytecode-compiler/bytecompiler.rewrite.md to run on top of a bytecode interpreter also in Python. My compiler's for Py3.4 only.