py/compile: Fix case of eager implicit conversion of local to nonlocal.

This ensures that implicit variables are only converted to implicit
closed-over variables (nonlocals) at the very end of the function scope.
If variables are closed-over when first used (read from, as was done prior
to this commit) then this can be incorrect because the variable may be
assigned to later on in the function which means they are just a plain
local, not closed over.

Fixes issue #4272.
This commit is contained in:
Damien George 2018-10-26 16:48:07 +11:00
parent c2074e7b66
commit 9201f46cc8
6 changed files with 47 additions and 8 deletions

View File

@ -1196,7 +1196,8 @@ STATIC void compile_declare_global(compiler_t *comp, mp_parse_node_t pn, qstr qs
STATIC void compile_declare_nonlocal(compiler_t *comp, mp_parse_node_t pn, qstr qst, bool added, id_info_t *id_info) { STATIC void compile_declare_nonlocal(compiler_t *comp, mp_parse_node_t pn, qstr qst, bool added, id_info_t *id_info) {
if (added) { if (added) {
scope_find_local_and_close_over(comp->scope_cur, id_info, qst); id_info->kind = ID_INFO_KIND_GLOBAL_IMPLICIT;
scope_check_to_close_over(comp->scope_cur, id_info);
if (id_info->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) { if (id_info->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) {
compile_syntax_error(comp, pn, "no binding for nonlocal found"); compile_syntax_error(comp, pn, "no binding for nonlocal found");
} }
@ -3391,6 +3392,14 @@ mp_raw_code_t *mp_compile_to_raw_code(mp_parse_tree_t *parse_tree, qstr source_f
#endif #endif
} else { } else {
compile_scope(comp, s, MP_PASS_SCOPE); compile_scope(comp, s, MP_PASS_SCOPE);
// Check if any implicitly declared variables should be closed over
for (size_t i = 0; i < s->id_info_len; ++i) {
id_info_t *id = &s->id_info[i];
if (id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) {
scope_check_to_close_over(s, id);
}
}
} }
// update maximim number of labels needed // update maximim number of labels needed

View File

@ -35,7 +35,7 @@ void mp_emit_common_get_id_for_load(scope_t *scope, qstr qst) {
bool added; bool added;
id_info_t *id = scope_find_or_add_id(scope, qst, &added); id_info_t *id = scope_find_or_add_id(scope, qst, &added);
if (added) { if (added) {
scope_find_local_and_close_over(scope, id, qst); id->kind = ID_INFO_KIND_GLOBAL_IMPLICIT;
} }
} }

View File

@ -130,21 +130,19 @@ STATIC void scope_close_over_in_parents(scope_t *scope, qstr qst) {
} }
} }
void scope_find_local_and_close_over(scope_t *scope, id_info_t *id, qstr qst) { void scope_check_to_close_over(scope_t *scope, id_info_t *id) {
if (scope->parent != NULL) { if (scope->parent != NULL) {
for (scope_t *s = scope->parent; s->parent != NULL; s = s->parent) { for (scope_t *s = scope->parent; s->parent != NULL; s = s->parent) {
id_info_t *id2 = scope_find(s, qst); id_info_t *id2 = scope_find(s, id->qst);
if (id2 != NULL) { if (id2 != NULL) {
if (id2->kind == ID_INFO_KIND_LOCAL || id2->kind == ID_INFO_KIND_CELL || id2->kind == ID_INFO_KIND_FREE) { if (id2->kind == ID_INFO_KIND_LOCAL || id2->kind == ID_INFO_KIND_CELL || id2->kind == ID_INFO_KIND_FREE) {
id->kind = ID_INFO_KIND_FREE; id->kind = ID_INFO_KIND_FREE;
scope_close_over_in_parents(scope, qst); scope_close_over_in_parents(scope, id->qst);
return;
} }
break; break;
} }
} }
} }
id->kind = ID_INFO_KIND_GLOBAL_IMPLICIT;
} }
#endif // MICROPY_ENABLE_COMPILER #endif // MICROPY_ENABLE_COMPILER

View File

@ -93,6 +93,6 @@ void scope_free(scope_t *scope);
id_info_t *scope_find_or_add_id(scope_t *scope, qstr qstr, bool *added); id_info_t *scope_find_or_add_id(scope_t *scope, qstr qstr, bool *added);
id_info_t *scope_find(scope_t *scope, qstr qstr); id_info_t *scope_find(scope_t *scope, qstr qstr);
id_info_t *scope_find_global(scope_t *scope, qstr qstr); id_info_t *scope_find_global(scope_t *scope, qstr qstr);
void scope_find_local_and_close_over(scope_t *scope, id_info_t *id, qstr qst); void scope_check_to_close_over(scope_t *scope, id_info_t *id);
#endif // MICROPY_INCLUDED_PY_SCOPE_H #endif // MICROPY_INCLUDED_PY_SCOPE_H

View File

@ -0,0 +1,31 @@
# test implicit scoping rules
# implicit nonlocal, with variable defined after closure
def f():
def g():
return x # implicit nonlocal
x = 3 # variable defined after function that closes over it
return g
print(f()())
# implicit nonlocal at inner level, with variable defined after closure
def f():
def g():
def h():
return x # implicit nonlocal
return h
x = 4 # variable defined after function that closes over it
return g
print(f()()())
# local variable which should not be implicitly made nonlocal
def f():
x = 0
def g():
x # local because next statement assigns to it
x = 1
g()
try:
f()
except NameError:
print('NameError')

View File

@ -358,6 +358,7 @@ def run_tests(pyb, tests, args, base_path="."):
skip_tests.add('basics/del_deref.py') # requires checking for unbound local skip_tests.add('basics/del_deref.py') # requires checking for unbound local
skip_tests.add('basics/del_local.py') # requires checking for unbound local skip_tests.add('basics/del_local.py') # requires checking for unbound local
skip_tests.add('basics/exception_chain.py') # raise from is not supported skip_tests.add('basics/exception_chain.py') # raise from is not supported
skip_tests.add('basics/scope_implicit.py') # requires checking for unbound local
skip_tests.add('basics/try_finally_return2.py') # requires raise_varargs skip_tests.add('basics/try_finally_return2.py') # requires raise_varargs
skip_tests.add('basics/unboundlocal.py') # requires checking for unbound local skip_tests.add('basics/unboundlocal.py') # requires checking for unbound local
skip_tests.add('misc/features.py') # requires raise_varargs skip_tests.add('misc/features.py') # requires raise_varargs