byterun icon indicating copy to clipboard operation
byterun copied to clipboard

Fix a bug in closures. Lexcical scoping for closures.

Open sheisc opened this issue 3 years ago • 4 comments

Fix a bug in closures. It seems that lexical scoping was not implemented correctly in byterun. Please refer to the new test case (i.e., test_lexical_scope()) in tests/test_functions.py.

def f(x, z):
	def g(y):
		return x + y + z
	return g

g1 = f(3, 4)
g2 = f(5, 6)
# BUG: the result of g1(100) is not 107. 
assert g1(100) == 107
assert g2(100) == 111

sheisc avatar Feb 02 '22 14:02 sheisc

I am not seeing a problem In the fork of this over at https://github.com/rocky/x-python which handles Python bytecode from version 2.5 or so to 3.10 (with some gaps around 3.0):

For example:

$ xpython -v test_lexical_scope-3.4.pyc 
INFO:xpython.vm:L. 1   @  0: LOAD_CONST <Code3 code object f at 0x7fbd39604fa0, file test_lexical_scope.py>, line 1
INFO:xpython.vm:       @  3: LOAD_CONST f
INFO:xpython.vm:       @  6: MAKE_FUNCTION (f) [0 positional, 0 name and default] 0
INFO:xpython.vm:       @  9: STORE_NAME (<Function f at 0x7fbd395de480>) f
INFO:xpython.vm:L. 5   @ 12: LOAD_NAME f
INFO:xpython.vm:       @ 15: LOAD_CONST 3
INFO:xpython.vm:       @ 18: LOAD_CONST 4
INFO:xpython.vm:       @ 21: CALL_FUNCTION (f) [2 positional, 0 named] 2
INFO:xpython.vm:    L. 2   @  0: LOAD_CLOSURE x
INFO:xpython.vm:           @  3: LOAD_CLOSURE z
INFO:xpython.vm:           @  6: BUILD_TUPLE 2
INFO:xpython.vm:           @  9: LOAD_CONST <Code3 code object g at 0x7fbd39605120, file test_lexical_scope.py>, line 2
INFO:xpython.vm:           @ 12: LOAD_CONST f.<locals>.g
INFO:xpython.vm:           @ 15: MAKE_CLOSURE 0
INFO:xpython.vm:           @ 18: STORE_FAST (<Function f.<locals>.g at 0x7fbd395de840>) g
INFO:xpython.vm:    L. 4   @ 21: LOAD_FAST g
INFO:xpython.vm:           @ 24: RETURN_VALUE (<Function f.<locals>.g at 0x7fbd395de840>) 
INFO:xpython.vm:       @ 24: STORE_NAME (<Function f.<locals>.g at 0x7fbd395de840>) g1
INFO:xpython.vm:L. 6   @ 27: LOAD_NAME f
INFO:xpython.vm:       @ 30: LOAD_CONST 5
INFO:xpython.vm:       @ 33: LOAD_CONST 6
INFO:xpython.vm:       @ 36: CALL_FUNCTION (f) [2 positional, 0 named] 2
INFO:xpython.vm:    L. 2   @  0: LOAD_CLOSURE x
INFO:xpython.vm:           @  3: LOAD_CLOSURE z
INFO:xpython.vm:           @  6: BUILD_TUPLE 2
INFO:xpython.vm:           @  9: LOAD_CONST <Code3 code object g at 0x7fbd39605120, file test_lexical_scope.py>, line 2
INFO:xpython.vm:           @ 12: LOAD_CONST f.<locals>.g
INFO:xpython.vm:           @ 15: MAKE_CLOSURE 0
INFO:xpython.vm:           @ 18: STORE_FAST (<Function f.<locals>.g at 0x7fbd395de8e0>) g
INFO:xpython.vm:    L. 4   @ 21: LOAD_FAST g
INFO:xpython.vm:           @ 24: RETURN_VALUE (<Function f.<locals>.g at 0x7fbd395de8e0>) 
INFO:xpython.vm:       @ 39: STORE_NAME (<Function f.<locals>.g at 0x7fbd395de8e0>) g2
INFO:xpython.vm:L. 7   @ 42: LOAD_NAME g1
INFO:xpython.vm:       @ 45: LOAD_CONST 100
INFO:xpython.vm:       @ 48: CALL_FUNCTION (f.<locals>.g) [1 positional, 0 named] 1
INFO:xpython.vm:    L. 3   @  0: LOAD_DEREF (3) x
INFO:xpython.vm:           @  3: LOAD_FAST y
INFO:xpython.vm:           @  6: BINARY_ADD (3, 100) 
INFO:xpython.vm:           @  7: LOAD_DEREF (4) z
INFO:xpython.vm:           @ 10: BINARY_ADD (103, 4) 
INFO:xpython.vm:           @ 11: RETURN_VALUE (107) 
INFO:xpython.vm:       @ 51: LOAD_CONST 107
INFO:xpython.vm:       @ 54: COMPARE_OP (107, 107) ==
INFO:xpython.vm:       @ 57: POP_JUMP_IF_TRUE 66
INFO:xpython.vm:L. 8   @ 66: LOAD_NAME g2
INFO:xpython.vm:       @ 69: LOAD_CONST 100
INFO:xpython.vm:       @ 72: CALL_FUNCTION (f.<locals>.g) [1 positional, 0 named] 1
INFO:xpython.vm:    L. 3   @  0: LOAD_DEREF (5) x
INFO:xpython.vm:           @  3: LOAD_FAST y
INFO:xpython.vm:           @  6: BINARY_ADD (5, 100) 
INFO:xpython.vm:           @  7: LOAD_DEREF (6) z
INFO:xpython.vm:           @ 10: BINARY_ADD (105, 6) 
INFO:xpython.vm:           @ 11: RETURN_VALUE (111) 
INFO:xpython.vm:       @ 75: LOAD_CONST 111
INFO:xpython.vm:       @ 78: COMPARE_OP (111, 111) ==
INFO:xpython.vm:       @ 81: POP_JUMP_IF_TRUE 90
INFO:xpython.vm:       @ 90: LOAD_CONST None
INFO:xpython.vm:       @ 93: RETURN_VALUE (None) 

(And use -vv if you want more information or remove -v to remove the tracing.

rocky avatar Feb 02 '22 17:02 rocky

Hi, Rocky, thank you for your amazing work on x-python. I am very interested in it. Thanks a lot.

sheisc avatar Feb 03 '22 02:02 sheisc

Hi, everyone. I think I have found an interesting test case which might show that "a pure-Python implementation like byterun is hard to be fully equivalent to cpython".

   # add the following function into class TestPrinting in tests/test_basic.py
        def test_locals(self):
            self.assert_ok("""\
                x = locals()
                print(len(x))
                """)

As for byterun, the output is always the length of the locals in the call_function(self, arg, args, kwargs) in pyvm2.py, rather than the result needed (i.e., locals in the test case). So this test case fails in a pure-Python implementation. But I really enjoy reading the code of byterun, as it does give me an overview of cpython. Thanks.

sheisc avatar Feb 03 '22 07:02 sheisc

A big surprise to find that Rocky's x-python has already considered locals(), globals() and so on in the function call_function_with_args_resolved() in byteop.py.

sheisc avatar Feb 03 '22 14:02 sheisc