Remove Scope variant from PyObject.

Author: cthulahoops

src/main.rs

@@ -15,7 +15,7 @@ use rustpython_vm::{
     import,
     obj::objstr,
     print_exception,
-    pyobject::{AttributeProtocol, PyObjectRef, PyResult},
+    pyobject::{AttributeProtocol, PyResult, ScopeRef},
     util, VirtualMachine,
 };
 use rustyline::{error::ReadlineError, Editor};
@@ -120,11 +120,7 @@ fn run_script(vm: &mut VirtualMachine, script_file: &str) -> PyResult {
     }
 }
 
-fn shell_exec(
-    vm: &mut VirtualMachine,
-    source: &str,
-    scope: PyObjectRef,
-) -> Result<(), CompileError> {
+fn shell_exec(vm: &mut VirtualMachine, source: &str, scope: ScopeRef) -> Result<(), CompileError> {
     match compile::compile(
         source,
         &compile::Mode::Single,

vm/src/builtins.rs

@@ -3,7 +3,6 @@
 //! Implements functions listed here: https://docs.python.org/3/library/builtins.html
 
 // use std::ops::Deref;
-use std::cell::RefCell;
 use std::char;
 use std::io::{self, Write};
 
@@ -16,9 +15,10 @@ use crate::obj::objstr;
 use crate::obj::objtype;
 
 use crate::pyobject::{
-    AttributeProtocol, IdProtocol, PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef,
-    PyResult, Scope, TypeProtocol,
+    AttributeProtocol, IdProtocol, PyContext, PyFuncArgs, PyObjectRef, PyResult, Scope, ScopeRef,
+    TypeProtocol,
 };
+use std::rc::Rc;
 
 #[cfg(not(target_arch = "wasm32"))]
 use crate::stdlib::io::io_open;
@@ -258,7 +258,7 @@ fn builtin_exec(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     vm.run_code_obj(code_obj, scope)
 }
 
-fn make_scope(vm: &mut VirtualMachine, locals: Option<&PyObjectRef>) -> PyObjectRef {
+fn make_scope(vm: &mut VirtualMachine, locals: Option<&PyObjectRef>) -> ScopeRef {
     // handle optional global and locals
     let locals = if let Some(locals) = locals {
         locals.clone()
@@ -268,18 +268,10 @@ fn make_scope(vm: &mut VirtualMachine, locals: Option<&PyObjectRef>) -> PyObject
 
     // TODO: handle optional globals
     // Construct new scope:
-    let scope_inner = Scope {
+    Rc::new(Scope {
         locals,
         parent: None,
-    };
-
-    PyObject {
-        payload: PyObjectPayload::Scope {
-            scope: RefCell::new(scope_inner),
-        },
-        typ: None,
-    }
-    .into_ref()
+    })
 }
 
 fn builtin_format(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -842,13 +834,7 @@ pub fn builtin_build_class_(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> Py
         },
     )?;
 
-    vm.invoke(
-        function,
-        PyFuncArgs {
-            args: vec![namespace.clone()],
-            kwargs: vec![],
-        },
-    )?;
+    vm.invoke_with_locals(function, namespace.clone())?;
 
     vm.call_method(&metaclass, "__call__", vec![name_arg, bases, namespace])
 }

vm/src/bytecode.rs

@@ -159,7 +159,6 @@ pub enum Instruction {
     },
     PrintExpr,
     LoadBuildClass,
-    StoreLocals,
     UnpackSequence {
         size: usize,
     },
@@ -358,7 +357,6 @@ impl Instruction {
             MapAdd { i } => w!(MapAdd, i),
             PrintExpr => w!(PrintExpr),
             LoadBuildClass => w!(LoadBuildClass),
-            StoreLocals => w!(StoreLocals),
             UnpackSequence { size } => w!(UnpackSequence, size),
             UnpackEx { before, after } => w!(UnpackEx, before, after),
             Unpack => w!(Unpack),

vm/src/compile.rs

@@ -482,18 +482,14 @@ impl Compiler {
                 self.emit(Instruction::LoadBuildClass);
                 let line_number = self.get_source_line_number();
                 self.code_object_stack.push(CodeObject::new(
-                    vec![String::from("__locals__")],
+                    vec![],
                     None,
                     vec![],
                     None,
                     self.source_path.clone().unwrap(),
                     line_number,
                     name.clone(),
                 ));
-                self.emit(Instruction::LoadName {
-                    name: String::from("__locals__"),
-                });
-                self.emit(Instruction::StoreLocals);
                 self.compile_statements(body)?;
                 self.emit(Instruction::LoadConst {
                     value: bytecode::Constant::None,

vm/src/eval.rs

@@ -3,15 +3,10 @@ extern crate rustpython_parser;
 use std::error::Error;
 
 use crate::compile;
-use crate::pyobject::{PyObjectRef, PyResult};
+use crate::pyobject::{PyResult, ScopeRef};
 use crate::vm::VirtualMachine;
 
-pub fn eval(
-    vm: &mut VirtualMachine,
-    source: &str,
-    scope: PyObjectRef,
-    source_path: &str,
-) -> PyResult {
+pub fn eval(vm: &mut VirtualMachine, source: &str, scope: ScopeRef, source_path: &str) -> PyResult {
     match compile::compile(
         source,
         &compile::Mode::Eval,

vm/src/frame.rs

@@ -16,8 +16,8 @@ use crate::obj::objlist;
 use crate::obj::objstr;
 use crate::obj::objtype;
 use crate::pyobject::{
-    DictProtocol, IdProtocol, ParentProtocol, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef,
-    PyResult, TypeProtocol,
+    DictProtocol, IdProtocol, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult,
+    ScopeRef, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 use num_bigint::BigInt;
@@ -50,7 +50,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 locals: PyObjectRef,          // Variables
+    pub scope: ScopeRef,              // Variables
     pub lasti: RefCell<usize>,        // index of last instruction ran
 }
 
@@ -64,7 +64,7 @@ pub enum ExecutionResult {
 pub type FrameResult = Result<Option<ExecutionResult>, PyObjectRef>;
 
 impl Frame {
-    pub fn new(code: PyObjectRef, globals: PyObjectRef) -> Frame {
+    pub fn new(code: PyObjectRef, scope: ScopeRef) -> Frame {
         //populate the globals and locals
         //TODO: This is wrong, check https://github.com/nedbat/byterun/blob/31e6c4a8212c35b5157919abff43a7daa0f377c6/byterun/pyvm2.py#L95
         /*
@@ -73,7 +73,7 @@ impl Frame {
             None => HashMap::new(),
         };
         */
-        let locals = globals;
+        // let locals = globals;
         // locals.extend(callargs);
 
         Frame {
@@ -82,7 +82,7 @@ impl Frame {
             blocks: RefCell::new(vec![]),
             // save the callargs as locals
             // globals: locals.clone(),
-            locals,
+            scope,
             lasti: RefCell::new(0),
         }
     }
@@ -436,7 +436,7 @@ impl Frame {
                 };
                 // pop argc arguments
                 // argument: name, args, globals
-                let scope = self.locals.clone();
+                let scope = self.scope.clone();
                 let obj = vm.ctx.new_function(code_obj, scope, defaults);
                 self.push_value(obj);
                 Ok(None)
@@ -582,16 +582,6 @@ impl Frame {
                 self.push_value(rustfunc);
                 Ok(None)
             }
-            bytecode::Instruction::StoreLocals => {
-                let locals = self.pop_value();
-                match self.locals.payload {
-                    PyObjectPayload::Scope { ref scope } => {
-                        (*scope.borrow_mut()).locals = locals;
-                    }
-                    _ => panic!("We really expect our scope to be a scope!"),
-                }
-                Ok(None)
-            }
             bytecode::Instruction::UnpackSequence { size } => {
                 let value = self.pop_value();
                 let elements = vm.extract_elements(&value)?;
@@ -715,8 +705,10 @@ impl Frame {
         let obj = import_module(vm, current_path, module)?;
 
         for (k, v) in obj.get_key_value_pairs().iter() {
-            vm.ctx
-                .set_attr(&self.locals, &objstr::get_value(k), v.clone());
+            &self
+                .scope
+                .locals
+                .set_item(&vm.ctx, &objstr::get_value(k), v.clone());
         }
         Ok(None)
     }
@@ -838,37 +830,35 @@ impl Frame {
 
     fn store_name(&self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
         let obj = self.pop_value();
-        vm.ctx.set_attr(&self.locals, name, obj);
+        self.scope.locals.set_item(&vm.ctx, name, obj);
         Ok(None)
     }
 
     fn delete_name(&self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
-        let locals = match self.locals.payload {
-            PyObjectPayload::Scope { ref scope } => scope.borrow().locals.clone(),
-            _ => panic!("We really expect our scope to be a scope!"),
-        };
-
-        // Assume here that locals is a dict
         let name = vm.ctx.new_str(name.to_string());
-        vm.call_method(&locals, "__delitem__", vec![name])?;
+        vm.call_method(&self.scope.locals, "__delitem__", vec![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.locals.clone();
+        let mut scope = self.scope.clone();
         loop {
-            if scope.contains_key(name) {
-                let obj = scope.get_item(name).unwrap();
+            if scope.locals.contains_key(name) {
+                let obj = scope.locals.get_item(name).unwrap();
                 self.push_value(obj);
-                break Ok(None);
-            } else if scope.has_parent() {
-                scope = scope.get_parent();
-            } else {
-                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);
-                break Err(name_error);
+                return 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);
+                }
             }
         }
     }
@@ -1119,21 +1109,18 @@ impl fmt::Debug for Frame {
             .map(|elem| format!("\n  > {:?}", elem))
             .collect::<Vec<_>>()
             .join("");
-        let local_str = match self.locals.payload {
-            PyObjectPayload::Scope { ref scope } => match scope.borrow().locals.payload {
-                PyObjectPayload::Dict { ref elements } => {
-                    objdict::get_key_value_pairs_from_content(&elements.borrow())
-                        .iter()
-                        .map(|elem| format!("\n  {:?} = {:?}", elem.0, elem.1))
-                        .collect::<Vec<_>>()
-                        .join("")
-                }
-                ref unexpected => panic!(
-                    "locals unexpectedly not wrapping a dict! instead: {:?}",
-                    unexpected
-                ),
-            },
-            ref unexpected => panic!("locals unexpectedly not a scope! instead: {:?}", unexpected),
+        let local_str = match self.scope.locals.payload {
+            PyObjectPayload::Dict { ref elements } => {
+                objdict::get_key_value_pairs_from_content(&elements.borrow())
+                    .iter()
+                    .map(|elem| format!("\n  {:?} = {:?}", elem.0, elem.1))
+                    .collect::<Vec<_>>()
+                    .join("")
+            }
+            ref unexpected => panic!(
+                "locals unexpectedly not wrapping a dict! instead: {:?}",
+                unexpected
+            ),
         };
         write!(
             f,

vm/src/import.rs

@@ -43,8 +43,9 @@ fn import_uncached_module(
 
     let builtins = vm.get_builtin_scope();
     let scope = vm.ctx.new_scope(Some(builtins));
-    vm.ctx
-        .set_attr(&scope, "__name__", vm.new_str(module.to_string()));
+    scope
+        .locals
+        .set_item(&vm.ctx, "__name__", vm.new_str(module.to_string()));
     vm.run_code_obj(code_obj, scope.clone())?;
     Ok(vm.ctx.new_module(module, scope))
 }

vm/src/obj/objframe.rs

@@ -30,13 +30,7 @@ fn frame_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 fn frame_flocals(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(vm, args, required = [(frame, Some(vm.ctx.frame_type()))]);
     let frame = get_value(frame);
-    let py_scope = frame.locals.clone();
-
-    if let PyObjectPayload::Scope { scope } = &py_scope.payload {
-        Ok(scope.borrow().locals.clone())
-    } else {
-        panic!("The scope isn't a scope!");
-    }
+    Ok(frame.scope.locals.clone())
 }
 
 fn frame_fcode(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {