RustPython · windelbouwman · Mar 10, 2019 · Mar 9, 2019 · Mar 9, 2019 · Mar 9, 2019
diff --git a/src/main.rs b/src/main.rs
@@ -12,7 +12,7 @@ use rustpython_parser::error::ParseError;
 use rustpython_vm::{
     compile,
     error::CompileError,
-    frame::ScopeRef,
+    frame::Scope,
     import,
     obj::objstr,
     print_exception,
@@ -82,8 +82,7 @@ fn _run_string(vm: &mut VirtualMachine, source: &str, source_path: String) -> Py
         vm.new_exception(syntax_error, err.to_string())
     })?;
     // trace!("Code object: {:?}", code_obj.borrow());
-    let builtins = vm.get_builtin_scope();
-    let vars = vm.context().new_scope(Some(builtins)); // Keep track of local variables
+    let vars = vm.ctx.new_scope(); // Keep track of local variables
     vm.run_code_obj(code_obj, vars)
 }
 
@@ -121,7 +120,7 @@ fn run_script(vm: &mut VirtualMachine, script_file: &str) -> PyResult {
     }
 }
 
-fn shell_exec(vm: &mut VirtualMachine, source: &str, scope: ScopeRef) -> Result<(), CompileError> {
+fn shell_exec(vm: &mut VirtualMachine, source: &str, scope: Scope) -> Result<(), CompileError> {
     match compile::compile(
         source,
         &compile::Mode::Single,
@@ -165,8 +164,7 @@ fn run_shell(vm: &mut VirtualMachine) -> PyResult {
         "Welcome to the magnificent Rust Python {} interpreter",
         crate_version!()
     );
-    let builtins = vm.get_builtin_scope();
-    let vars = vm.context().new_scope(Some(builtins)); // Keep track of local variables
+    let vars = vm.ctx.new_scope();
 
     // Read a single line:
     let mut input = String::new();

diff --git a/tests/snippets/test_exec.py b/tests/snippets/test_exec.py
@@ -13,30 +13,34 @@
 
 exec("assert max(1, 5, square(5)) == 25", None)
 
-#
-# These doesn't work yet:
-#
 # Local environment shouldn't replace global environment:
-#
-# exec("assert max(1, 5, square(5)) == 25", None, {})
-#
+exec("assert max(1, 5, square(5)) == 25", None, {})
+
 # Closures aren't available if local scope is replaced:
-#
-# def g():
-#     seven = "seven"
-#     def f():
-#         try:
-#             exec("seven", None, {})
-#         except NameError:
-#             pass
-#         else:
-#             raise NameError("seven shouldn't be in scope")
-#     f()
-# g()
+def g():
+    seven = "seven"
+    def f():
+        try:
+            exec("seven", None, {})
+        except NameError:
+            pass
+        else:
+            raise NameError("seven shouldn't be in scope")
+    f()
+g()
 
 try:
     exec("", 1)
 except TypeError:
     pass
 else:
     raise TypeError("exec should fail unless globals is a dict or None")
+
+g = globals()
+g['x'] = 2
+exec('x += 2')
+assert x == 4
+assert g['x'] == x
+
+exec("del x")
+assert 'x' not in g
diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs
@@ -14,7 +14,7 @@ use crate::obj::objiter;
 use crate::obj::objstr;
 use crate::obj::objtype;
 
-use crate::frame::{Scope, ScopeRef};
+use crate::frame::Scope;
 use crate::pyobject::{
     AttributeProtocol, IdProtocol, PyContext, PyFuncArgs, PyObjectRef, PyResult, TypeProtocol,
 };
@@ -245,7 +245,7 @@ fn make_scope(
     vm: &mut VirtualMachine,
     globals: Option<&PyObjectRef>,
     locals: Option<&PyObjectRef>,
-) -> PyResult<ScopeRef> {
+) -> PyResult<Scope> {
     let dict_type = vm.ctx.dict_type();
     let globals = match globals {
         Some(arg) => {
@@ -269,16 +269,16 @@ fn make_scope(
     };
 
     let current_scope = vm.current_scope();
-    let parent = match globals {
-        Some(dict) => Some(Scope::new(dict.clone(), Some(vm.get_builtin_scope()))),
-        None => current_scope.parent.clone(),
+    let globals = match globals {
+        Some(dict) => dict.clone(),
+        None => current_scope.globals.clone(),
     };
     let locals = match locals {
-        Some(dict) => dict.clone(),
-        None => current_scope.locals.clone(),
+        Some(dict) => Some(dict.clone()),
+        None => current_scope.get_only_locals(),
     };
 
-    Ok(Scope::new(locals, parent))
+    Ok(Scope::new(locals, globals))
 }
 
 fn builtin_format(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -303,7 +303,9 @@ fn builtin_getattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     vm.get_attribute(obj.clone(), attr.clone())
 }
 
-// builtin_globals
+fn builtin_globals(vm: &mut VirtualMachine, _args: PyFuncArgs) -> PyResult {
+    Ok(vm.current_scope().globals.clone())
+}
 
 fn builtin_hasattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(
@@ -743,6 +745,7 @@ pub fn make_module(ctx: &PyContext) -> PyObjectRef {
         "filter" => ctx.filter_type(),
         "format" => ctx.new_rustfunc(builtin_format),
         "getattr" => ctx.new_rustfunc(builtin_getattr),
+        "globals" => ctx.new_rustfunc(builtin_globals),
         "hasattr" => ctx.new_rustfunc(builtin_hasattr),
         "hash" => ctx.new_rustfunc(builtin_hash),
         "hex" => ctx.new_rustfunc(builtin_hex),

diff --git a/vm/src/eval.rs b/vm/src/eval.rs
@@ -3,11 +3,11 @@ extern crate rustpython_parser;
 use std::error::Error;
 
 use crate::compile;
-use crate::frame::ScopeRef;
+use crate::frame::Scope;
 use crate::pyobject::PyResult;
 use crate::vm::VirtualMachine;
 
-pub fn eval(vm: &mut VirtualMachine, source: &str, scope: ScopeRef, source_path: &str) -> PyResult {
+pub fn eval(vm: &mut VirtualMachine, source: &str, scope: Scope, source_path: &str) -> PyResult {
     match compile::compile(
         source,
         &compile::Mode::Eval,
@@ -34,7 +34,7 @@ mod tests {
     fn test_print_42() {
         let source = String::from("print('Hello world')\n");
         let mut vm = VirtualMachine::new();
-        let vars = vm.context().new_scope(None);
+        let vars = vm.ctx.new_scope();
         let _result = eval(&mut vm, &source, vars, "<unittest>");
 
         // TODO: check result?

diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -32,16 +32,120 @@ use crate::vm::VirtualMachine;
  * When a name is looked up, it is check in its scope.
  */
 #[derive(Debug)]
+struct RcListNode<T> {
+    elem: T,
+    next: Option<Rc<RcListNode<T>>>,
+}
+
+#[derive(Debug, Clone)]
+struct RcList<T> {
+    head: Option<Rc<RcListNode<T>>>,
+}
+
+struct Iter<'a, T: 'a> {
+    next: Option<&'a RcListNode<T>>,
+}
+
+impl<T> RcList<T> {
+    pub fn new() -> Self {
+        RcList { head: None }
+    }
+
+    pub fn insert(self, elem: T) -> Self {
+        RcList {
+            head: Some(Rc::new(RcListNode {
+                elem,
+                next: self.head,
+            })),
+        }
+    }
+
+    pub fn iter(&self) -> Iter<T> {
+        Iter {
+            next: self.head.as_ref().map(|node| &**node),
+        }
+    }
+}
+
+impl<'a, T> Iterator for Iter<'a, T> {
+    type Item = &'a T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.next.map(|node| {
+            self.next = node.next.as_ref().map(|node| &**node);
+            &node.elem
+        })
+    }
+}
+
+#[derive(Debug, Clone)]
 pub struct Scope {
-    pub locals: PyObjectRef, // Variables
-    // TODO: pub locals: RefCell<PyAttributes>,         // Variables
-    pub parent: Option<Rc<Scope>>, // Parent scope
+    locals: RcList<PyObjectRef>,
+    pub globals: PyObjectRef,
 }
-pub type ScopeRef = Rc<Scope>;
 
 impl Scope {
-    pub fn new(locals: PyObjectRef, parent: Option<ScopeRef>) -> ScopeRef {
-        Rc::new(Scope { locals, parent })
+    pub fn new(locals: Option<PyObjectRef>, globals: PyObjectRef) -> Scope {
+        let locals = match locals {
+            Some(dict) => RcList::new().insert(dict),
+            None => RcList::new(),
+        };
+        Scope { locals, globals }
+    }
+
+    pub fn get_locals(&self) -> PyObjectRef {
+        match self.locals.iter().next() {
+            Some(dict) => dict.clone(),
+            None => self.globals.clone(),
+        }
+    }
+
+    pub fn get_only_locals(&self) -> Option<PyObjectRef> {
+        match self.locals.iter().next() {
+            Some(dict) => Some(dict.clone()),
+            None => None,
+        }
+    }
+
+    pub fn child_scope_with_locals(&self, locals: PyObjectRef) -> Scope {
+        Scope {
+            locals: self.locals.clone().insert(locals),
+            globals: self.globals.clone(),
+        }
+    }
+
+    pub fn child_scope(&self, ctx: &PyContext) -> Scope {
+        self.child_scope_with_locals(ctx.new_dict())
+    }
+}
+
+pub trait NameProtocol {
+    fn load_name(&self, vm: &VirtualMachine, name: &str) -> Option<PyObjectRef>;
+    fn store_name(&self, vm: &VirtualMachine, name: &str, value: PyObjectRef);
+    fn delete_name(&self, vm: &VirtualMachine, name: &str);
+}
+
+impl NameProtocol for Scope {
+    fn load_name(&self, vm: &VirtualMachine, name: &str) -> Option<PyObjectRef> {
+        for dict in self.locals.iter() {
+            if let Some(value) = dict.get_item(name) {
+                return Some(value);
+            }
+        }
+
+        if let Some(value) = self.globals.get_item(name) {
+            return Some(value);
+        }
+
+        vm.builtins.get_item(name)
+    }
+
+    fn store_name(&self, vm: &VirtualMachine, key: &str, value: PyObjectRef) {
+        self.get_locals().set_item(&vm.ctx, key, value)
+    }
+
+    fn delete_name(&self, _vm: &VirtualMachine, key: &str) {
+        self.get_locals().del_item(key)
     }
 }
 
@@ -73,7 +177,7 @@ pub struct Frame {
     // We need 1 stack per frame
     stack: RefCell<Vec<PyObjectRef>>, // The main data frame of the stack machine
     blocks: RefCell<Vec<Block>>,      // Block frames, for controlling loops and exceptions
-    pub scope: ScopeRef,              // Variables
+    pub scope: Scope,                 // Variables
     pub lasti: RefCell<usize>,        // index of last instruction ran
 }
 
@@ -93,7 +197,7 @@ pub enum ExecutionResult {
 pub type FrameResult = Result<Option<ExecutionResult>, PyObjectRef>;
 
 impl Frame {
-    pub fn new(code: PyObjectRef, scope: ScopeRef) -> Frame {
+    pub fn new(code: PyObjectRef, scope: Scope) -> Frame {
         //populate the globals and locals
         //TODO: This is wrong, check https://github.com/nedbat/byterun/blob/31e6c4a8212c35b5157919abff43a7daa0f377c6/byterun/pyvm2.py#L95
         /*
@@ -735,9 +839,7 @@ impl Frame {
         let obj = import_module(vm, current_path, module)?;
 
         for (k, v) in obj.get_key_value_pairs().iter() {
-            self.scope
-                .locals
-                .set_item(&vm.ctx, &objstr::get_value(k), v.clone());
+            self.scope.store_name(&vm, &objstr::get_value(k), v.clone());
         }
         Ok(None)
     }
@@ -859,35 +961,26 @@ impl Frame {
 
     fn store_name(&self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
         let obj = self.pop_value();
-        self.scope.locals.set_item(&vm.ctx, name, obj);
+        self.scope.store_name(&vm, name, obj);
         Ok(None)
     }
 
     fn delete_name(&self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
-        let name = vm.ctx.new_str(name.to_string());
-        vm.call_method(&self.scope.locals, "__delitem__", vec![name])?;
+        self.scope.delete_name(vm, name);
         Ok(None)
     }
 
     fn load_name(&self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
-        // Lookup name in scope and put it onto the stack!
-        let mut scope = self.scope.clone();
-        loop {
-            if scope.locals.contains_key(name) {
-                let obj = scope.locals.get_item(name).unwrap();
-                self.push_value(obj);
-                return Ok(None);
+        match self.scope.load_name(&vm, name) {
+            Some(value) => {
+                self.push_value(value);
+                Ok(None)
             }
-            match &scope.parent {
-                Some(parent_scope) => {
-                    scope = parent_scope.clone();
-                }
-                None => {
-                    let name_error_type = vm.ctx.exceptions.name_error.clone();
-                    let msg = format!("name '{}' is not defined", name);
-                    let name_error = vm.new_exception(name_error_type, msg);
-                    return Err(name_error);
-                }
+            None => {
+                let name_error_type = vm.ctx.exceptions.name_error.clone();
+                let msg = format!("name '{}' is not defined", name);
+                let name_error = vm.new_exception(name_error_type, msg);
+                Err(name_error)
             }
         }
     }
@@ -1138,7 +1231,7 @@ impl fmt::Debug for Frame {
             .map(|elem| format!("\n  > {:?}", elem))
             .collect::<Vec<_>>()
             .join("");
-        let local_str = match self.scope.locals.payload::<PyDict>() {
+        let local_str = match self.scope.get_locals().payload::<PyDict>() {
             Some(dict) => objdict::get_key_value_pairs_from_content(&dict.entries.borrow())
                 .iter()
                 .map(|elem| format!("\n  {:?} = {:?}", elem.0, elem.1))