Skip to content
This repository has been archived by the owner. It is now read-only.

Added closure related test cases #873

Merged
merged 3 commits into from Aug 11, 2018
Merged

Conversation

@BPYap
Copy link
Contributor

@BPYap BPYap commented Jul 22, 2018

While all but one test cases failed because voc currently doesn't support closure in generator, class and method context yet, I want to highlight 1 particular test case which isn't addressed in voc's current closure creation logic. As pointed out by @freakboy3742 , consider the following:

def func():
    closure_var = 'before nested is defined'
    def nested():
        print(closure_var)
    closure_var = 'after nested is defined'
    nested() 
func()

CPython3 will print after nested is defined in console while voc prints before nested is defined.
This is because voc takes the value of closure variable at the moment the closure function is defined and that value is stored in closure_vars field of that closure. Thus any rebinding of closure variables in enclosing scope after closure definition is not known to the closure.

As @freakboy3742 mentioned, current implementation of closure in voc may require some modifications /rework to allow closure to acquire the 'latest' value of closure variable.
Perhaps we could make use of code.co_cellvars in enclosing scope's context and store free variables somewhere else instead of local register of enclosing context so both enclosing scope and closure scope can be directed to that storage space when they want to access free variables. Plus it would also make nonlocal implementation easier because closure can directly modify nonlocal value in that same storage space.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

👍

@freakboy3742 freakboy3742 merged commit 97878b7 into beeware:master Aug 11, 2018
5 checks passed
@BPYap BPYap deleted the closure-test-case branch Aug 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants