Skip to content
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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add Py3.4 support. #20

wants to merge 13 commits into from

Conversation

darius
Copy link

@darius darius commented Feb 16, 2016

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.

… 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.)
@nedbat
Copy link
Owner

nedbat commented Feb 16, 2016

@darius: I'm unlikely to do more work on byterun, so maybe we should find another owner...

@darius
Copy link
Author

darius commented Feb 16, 2016

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.

…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.)
@darius
Copy link
Author

darius commented Feb 18, 2016

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.

@darius
Copy link
Author

darius commented Feb 18, 2016

Oh, and I still need to install 3.3 and make sure that's still passing.

vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 30, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
vrthra added a commit to vrthra-forks/byterun that referenced this pull request Jan 31, 2018
@darius darius mentioned this pull request Feb 5, 2018
@@ -207,6 +207,18 @@ def f4(g):
assert answer == 54
""")

def test_closure_vars_from_static_parent(self):
Copy link

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.

Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants