From e7ffc9db1d2cd8f6be143e0997eff64afe39bc71 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Sat, 31 Aug 2019 15:03:44 +0100 Subject: [PATCH 1/3] load_global shouldn't load outermost locals. --- tests/snippets/global_nonlocal.py | 3 +++ vm/src/scope.rs | 9 --------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/snippets/global_nonlocal.py b/tests/snippets/global_nonlocal.py index ddf5e1ed2..1996f2e33 100644 --- a/tests/snippets/global_nonlocal.py +++ b/tests/snippets/global_nonlocal.py @@ -15,8 +15,11 @@ assert a == 4 def x(): def y(): + global a nonlocal b + assert a == 4, a b = 3 + a = "no!" # a here shouldn't be seen by the global above. b = 2 y() return b diff --git a/vm/src/scope.rs b/vm/src/scope.rs index 3ba068f6e..eda7ddc09 100644 --- a/vm/src/scope.rs +++ b/vm/src/scope.rs @@ -179,15 +179,6 @@ impl NameProtocol for Scope { #[cfg_attr(feature = "flame-it", flame("Scope"))] /// Load a global name. fn load_global(&self, vm: &VirtualMachine, name: &str) -> Option { - // First, take a look in the outmost local scope (the scope at top level) - let last_local_dict = self.locals.iter().last(); - if let Some(local_dict) = last_local_dict { - if let Some(value) = local_dict.get_item_option(name, vm).unwrap() { - return Some(value); - } - } - - // Now, take a look at the globals or builtins. if let Some(value) = self.globals.get_item_option(name, vm).unwrap() { Some(value) } else { From b087325b8db88e24facf342a0a76402ed859bb69 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Sat, 31 Aug 2019 16:39:27 +0100 Subject: [PATCH 2/3] Be extremely conservative about guessing scopes when we don't know. --- compiler/src/symboltable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 3c70d4fc2..ab931ac57 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -252,8 +252,8 @@ impl SymbolTableAnalyzer { // Symbol is in some outer scope. symbol.is_free = true; } else { - // Well, it must be a global then :) - symbol.scope = SymbolScope::Global; + // Don't make assumptions when we don't know. + symbol.scope = SymbolScope::Unknown; } } } From 527f3ff800c1c5a7862d1b4c5cdb4b45857c5d12 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Sat, 31 Aug 2019 18:15:56 +0100 Subject: [PATCH 3/3] Be somewhat less conservative about declaring things global. --- compiler/src/symboltable.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index ab931ac57..c07b5ae40 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -251,9 +251,12 @@ impl SymbolTableAnalyzer { if found_in_outer_scope { // Symbol is in some outer scope. symbol.is_free = true; - } else { + } else if self.tables.is_empty() { // Don't make assumptions when we don't know. symbol.scope = SymbolScope::Unknown; + } else { + // If there are scopes above we can assume global. + symbol.scope = SymbolScope::Global; } } }