Merge pull request #590 from RustPython/exec

Proper construction of scope for exec/eval.

tests/snippets/test_exec.py

@@ -0,0 +1,42 @@
+exec("def square(x):\n return x * x\n")
+assert 16 == square(4)
+
+d = {}
+exec("def square(x):\n return x * x\n", {}, d)
+assert 16 == d['square'](4)
+
+exec("assert 2 == x", {}, {'x': 2})
+exec("assert 2 == x", {'x': 2}, {})
+exec("assert 4 == x", {'x': 2}, {'x': 4})
+
+exec("assert max(1, 2) == 2", {}, {})
+
+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, {})
+#
+# 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()
+
+try:
+    exec("", 1)
+except TypeError:
+    pass
+else:
+    raise TypeError("exec should fail unless globals is a dict or None")

vm/src/builtins.rs

@@ -18,7 +18,6 @@ use crate::frame::{Scope, ScopeRef};
 use crate::pyobject::{
     AttributeProtocol, IdProtocol, PyContext, PyFuncArgs, PyObjectRef, PyResult, TypeProtocol,
 };
-use std::rc::Rc;
 
 #[cfg(not(target_arch = "wasm32"))]
 use crate::stdlib::io::io_open;
@@ -191,12 +190,11 @@ fn builtin_eval(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         vm,
         args,
         required = [(source, None)],
-        optional = [
-            (_globals, Some(vm.ctx.dict_type())),
-            (locals, Some(vm.ctx.dict_type()))
-        ]
+        optional = [(globals, None), (locals, Some(vm.ctx.dict_type()))]
     );
 
+    let scope = make_scope(vm, globals, locals)?;
+
     // Determine code object:
     let code_obj = if objtype::isinstance(source, &vm.ctx.code_type()) {
         source.clone()
@@ -215,8 +213,6 @@ fn builtin_eval(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         return Err(vm.new_type_error("code argument must be str or code object".to_string()));
     };
 
-    let scope = make_scope(vm, locals);
-
     // Run the source:
     vm.run_code_obj(code_obj.clone(), scope)
 }
@@ -228,12 +224,11 @@ fn builtin_exec(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         vm,
         args,
         required = [(source, None)],
-        optional = [
-            (_globals, Some(vm.ctx.dict_type())),
-            (locals, Some(vm.ctx.dict_type()))
-        ]
+        optional = [(globals, None), (locals, Some(vm.ctx.dict_type()))]
     );
 
+    let scope = make_scope(vm, globals, locals)?;
+
     // Determine code object:
     let code_obj = if objtype::isinstance(source, &vm.ctx.str_type()) {
         let mode = compile::Mode::Exec;
@@ -252,26 +247,48 @@ fn builtin_exec(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         return Err(vm.new_type_error("source argument must be str or code object".to_string()));
     };
 
-    let scope = make_scope(vm, locals);
-
     // Run the code:
     vm.run_code_obj(code_obj, scope)
 }
 
-fn make_scope(vm: &mut VirtualMachine, locals: Option<&PyObjectRef>) -> ScopeRef {
-    // handle optional global and locals
-    let locals = if let Some(locals) = locals {
-        locals.clone()
-    } else {
-        vm.new_dict()
+fn make_scope(
+    vm: &mut VirtualMachine,
+    globals: Option<&PyObjectRef>,
+    locals: Option<&PyObjectRef>,
+) -> PyResult<ScopeRef> {
+    let dict_type = vm.ctx.dict_type();
+    let globals = match globals {
+        Some(arg) => {
+            if arg.is(&vm.get_none()) {
+                None
+            } else {
+                if vm.isinstance(arg, &dict_type)? {
+                    Some(arg)
+                } else {
+                    let arg_typ = arg.typ();
+                    let actual_type = vm.to_pystr(&arg_typ)?;
+                    let expected_type_name = vm.to_pystr(&dict_type)?;
+                    return Err(vm.new_type_error(format!(
+                        "globals must be a {}, not {}",
+                        expected_type_name, actual_type
+                    )));
+                }
+            }
+        }
+        None => None,
     };
 
-    // TODO: handle optional globals
-    // Construct new scope:
-    Rc::new(Scope {
-        locals,
-        parent: None,
-    })
+    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 locals = match locals {
+        Some(dict) => dict.clone(),
+        None => current_scope.locals.clone(),
+    };
+
+    Ok(Scope::new(locals, parent))
 }
 
 fn builtin_format(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {

vm/src/frame.rs

@@ -35,6 +35,12 @@ pub struct Scope {
 }
 pub type ScopeRef = Rc<Scope>;
 
+impl Scope {
+    pub fn new(locals: PyObjectRef, parent: Option<ScopeRef>) -> ScopeRef {
+        Rc::new(Scope { locals, parent })
+    }
+}
+
 #[derive(Clone, Debug)]
 struct Block {
     /// The type of block.

vm/src/vm.rs

@@ -90,6 +90,12 @@ impl VirtualMachine {
         result
     }
 
+    pub fn current_scope(&self) -> &ScopeRef {
+        let current_frame = &self.frames[self.frames.len() - 1];
+        let frame = objframe::get_value(current_frame);
+        &frame.scope
+    }
+
     /// Create a new python string object.
     pub fn new_str(&self, s: String) -> PyObjectRef {
         self.ctx.new_str(s)
@@ -218,7 +224,7 @@ impl VirtualMachine {
         &self.ctx
     }
 
-    pub fn get_builtin_scope(&mut self) -> ScopeRef {
+    pub fn get_builtin_scope(&self) -> ScopeRef {
         let a2 = &*self.builtins;
         match a2.payload {
             PyObjectPayload::Module { ref scope, .. } => scope.clone(),