Use refcells in frames to solve frame mutability problem.

Author: cthulahoops

tests/snippets/getframe.py

@@ -11,7 +11,6 @@ def test_function():
     assert sys._getframe().f_locals is not locals_dict
     assert sys._getframe().f_locals['x'] == 17
     assert sys._getframe(1).f_locals['foo'] == 'bar'
-    print(sys._getframe(1).f_locals)
 
 test_function()
 

vm/src/frame.rs

@@ -45,14 +45,13 @@ enum BlockType {
     },
 }
 
-#[derive(Clone)]
 pub struct Frame {
     pub code: bytecode::CodeObject,
     // We need 1 stack per frame
     stack: RefCell<Vec<PyObjectRef>>, // The main data frame of the stack machine
-    blocks: Vec<Block>,               // Block frames, for controlling loops and exceptions
+    blocks: RefCell<Vec<Block>>,      // Block frames, for controlling loops and exceptions
     pub locals: PyObjectRef,          // Variables
-    pub lasti: usize,                 // index of last instruction ran
+    pub lasti: RefCell<usize>,        // index of last instruction ran
 }
 
 // Running a frame can result in one of the below:
@@ -80,15 +79,15 @@ impl Frame {
         Frame {
             code: objcode::get_value(&code),
             stack: RefCell::new(vec![]),
-            blocks: vec![],
+            blocks: RefCell::new(vec![]),
             // save the callargs as locals
             // globals: locals.clone(),
             locals,
-            lasti: 0,
+            lasti: RefCell::new(0),
         }
     }
 
-    pub fn run(&mut self, vm: &mut VirtualMachine) -> Result<ExecutionResult, PyObjectRef> {
+    pub fn run(&self, vm: &mut VirtualMachine) -> Result<ExecutionResult, PyObjectRef> {
         let filename = &self.code.source_path.to_string();
 
         // This is the name of the object being run:
@@ -142,16 +141,16 @@ impl Frame {
         value
     }
 
-    pub fn fetch_instruction(&mut self) -> bytecode::Instruction {
+    pub fn fetch_instruction(&self) -> bytecode::Instruction {
         // TODO: an immutable reference is enough, we should not
         // clone the instruction.
-        let ins2 = self.code.instructions[self.lasti].clone();
-        self.lasti += 1;
+        let ins2 = self.code.instructions[*self.lasti.borrow()].clone();
+        *self.lasti.borrow_mut() += 1;
         ins2
     }
 
     // Execute a single instruction:
-    fn execute_instruction(&mut self, vm: &mut VirtualMachine) -> FrameResult {
+    fn execute_instruction(&self, vm: &mut VirtualMachine) -> FrameResult {
         let instruction = self.fetch_instruction();
         {
             trace!("=======");
@@ -344,7 +343,7 @@ impl Frame {
                 match next_obj {
                     Some(value) => {
                         // Set back program counter:
-                        self.lasti -= 1;
+                        *self.lasti.borrow_mut() -= 1;
                         Ok(Some(ExecutionResult::Yield(value)))
                     }
                     None => {
@@ -662,7 +661,7 @@ impl Frame {
     }
 
     fn get_elements(
-        &mut self,
+        &self,
         vm: &mut VirtualMachine,
         size: usize,
         unpack: bool,
@@ -683,7 +682,7 @@ impl Frame {
     }
 
     fn import(
-        &mut self,
+        &self,
         vm: &mut VirtualMachine,
         module: &str,
         symbol: &Option<String>,
@@ -701,7 +700,7 @@ impl Frame {
         Ok(None)
     }
 
-    fn import_star(&mut self, vm: &mut VirtualMachine, module: &str) -> FrameResult {
+    fn import_star(&self, vm: &mut VirtualMachine, module: &str) -> FrameResult {
         let current_path = {
             let mut source_pathbuf = PathBuf::from(&self.code.source_path);
             source_pathbuf.pop();
@@ -719,7 +718,7 @@ impl Frame {
     }
 
     // Unwind all blocks:
-    fn unwind_blocks(&mut self, vm: &mut VirtualMachine) -> Option<PyObjectRef> {
+    fn unwind_blocks(&self, vm: &mut VirtualMachine) -> Option<PyObjectRef> {
         while let Some(block) = self.pop_block() {
             match block.typ {
                 BlockType::Loop { .. } => {}
@@ -743,9 +742,9 @@ impl Frame {
         None
     }
 
-    fn unwind_loop(&mut self, vm: &mut VirtualMachine) -> Block {
+    fn unwind_loop(&self, vm: &mut VirtualMachine) -> Block {
         loop {
-            let block = self.current_block().cloned().expect("not in a loop");
+            let block = self.current_block().expect("not in a loop");
             match block.typ {
                 BlockType::Loop { .. } => break block,
                 BlockType::TryExcept { .. } => {
@@ -765,11 +764,7 @@ impl Frame {
         }
     }
 
-    fn unwind_exception(
-        &mut self,
-        vm: &mut VirtualMachine,
-        exc: PyObjectRef,
-    ) -> Option<PyObjectRef> {
+    fn unwind_exception(&self, vm: &mut VirtualMachine, exc: PyObjectRef) -> Option<PyObjectRef> {
         // unwind block stack on exception and find any handlers:
         while let Some(block) = self.pop_block() {
             match block.typ {
@@ -837,13 +832,13 @@ impl Frame {
         vm.call_method(context_manager, "__exit__", args)
     }
 
-    fn store_name(&mut self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
+    fn store_name(&self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
         let obj = self.pop_value();
         vm.ctx.set_attr(&self.locals, name, obj);
         Ok(None)
     }
 
-    fn delete_name(&mut self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
+    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!"),
@@ -855,7 +850,7 @@ impl Frame {
         Ok(None)
     }
 
-    fn load_name(&mut self, vm: &mut VirtualMachine, name: &str) -> FrameResult {
+    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();
         loop {
@@ -874,33 +869,33 @@ impl Frame {
         }
     }
 
-    fn subscript(&mut self, vm: &mut VirtualMachine, a: PyObjectRef, b: PyObjectRef) -> PyResult {
+    fn subscript(&self, vm: &mut VirtualMachine, a: PyObjectRef, b: PyObjectRef) -> PyResult {
         vm.call_method(&a, "__getitem__", vec![b])
     }
 
-    fn execute_store_subscript(&mut self, vm: &mut VirtualMachine) -> FrameResult {
+    fn execute_store_subscript(&self, vm: &mut VirtualMachine) -> FrameResult {
         let idx = self.pop_value();
         let obj = self.pop_value();
         let value = self.pop_value();
         vm.call_method(&obj, "__setitem__", vec![idx, value])?;
         Ok(None)
     }
 
-    fn execute_delete_subscript(&mut self, vm: &mut VirtualMachine) -> FrameResult {
+    fn execute_delete_subscript(&self, vm: &mut VirtualMachine) -> FrameResult {
         let idx = self.pop_value();
         let obj = self.pop_value();
         vm.call_method(&obj, "__delitem__", vec![idx])?;
         Ok(None)
     }
 
-    fn jump(&mut self, label: bytecode::Label) {
+    fn jump(&self, label: bytecode::Label) {
         let target_pc = self.code.label_map[&label];
         trace!("program counter from {:?} to {:?}", self.lasti, target_pc);
-        self.lasti = target_pc;
+        *self.lasti.borrow_mut() = target_pc;
     }
 
     fn execute_binop(
-        &mut self,
+        &self,
         vm: &mut VirtualMachine,
         op: &bytecode::BinaryOperator,
         inplace: bool,
@@ -943,11 +938,7 @@ impl Frame {
         Ok(None)
     }
 
-    fn execute_unop(
-        &mut self,
-        vm: &mut VirtualMachine,
-        op: &bytecode::UnaryOperator,
-    ) -> FrameResult {
+    fn execute_unop(&self, vm: &mut VirtualMachine, op: &bytecode::UnaryOperator) -> FrameResult {
         let a = self.pop_value();
         let value = match *op {
             bytecode::UnaryOperator::Minus => vm.call_method(&a, "__neg__", vec![])?,
@@ -968,7 +959,7 @@ impl Frame {
 
     // https://docs.python.org/3/reference/expressions.html#membership-test-operations
     fn _membership(
-        &mut self,
+        &self,
         vm: &mut VirtualMachine,
         needle: PyObjectRef,
         haystack: &PyObjectRef,
@@ -978,12 +969,7 @@ impl Frame {
         // not implemented.
     }
 
-    fn _in(
-        &mut self,
-        vm: &mut VirtualMachine,
-        needle: PyObjectRef,
-        haystack: PyObjectRef,
-    ) -> PyResult {
+    fn _in(&self, vm: &mut VirtualMachine, needle: PyObjectRef, haystack: PyObjectRef) -> PyResult {
         match self._membership(vm, needle, &haystack) {
             Ok(found) => Ok(found),
             Err(_) => Err(vm.new_type_error(format!(
@@ -994,7 +980,7 @@ impl Frame {
     }
 
     fn _not_in(
-        &mut self,
+        &self,
         vm: &mut VirtualMachine,
         needle: PyObjectRef,
         haystack: PyObjectRef,
@@ -1020,7 +1006,7 @@ impl Frame {
     }
 
     fn execute_compare(
-        &mut self,
+        &self,
         vm: &mut VirtualMachine,
         op: &bytecode::ComparisonOperator,
     ) -> FrameResult {
@@ -1043,47 +1029,47 @@ impl Frame {
         Ok(None)
     }
 
-    fn load_attr(&mut self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
+    fn load_attr(&self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
         let parent = self.pop_value();
         let attr_name = vm.new_str(attr_name.to_string());
         let obj = vm.get_attribute(parent, attr_name)?;
         self.push_value(obj);
         Ok(None)
     }
 
-    fn store_attr(&mut self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
+    fn store_attr(&self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
         let parent = self.pop_value();
         let value = self.pop_value();
         vm.ctx.set_attr(&parent, attr_name, value);
         Ok(None)
     }
 
-    fn delete_attr(&mut self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
+    fn delete_attr(&self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
         let parent = self.pop_value();
         let name = vm.ctx.new_str(attr_name.to_string());
         vm.del_attr(&parent, name)?;
         Ok(None)
     }
 
     pub fn get_lineno(&self) -> ast::Location {
-        self.code.locations[self.lasti].clone()
+        self.code.locations[*self.lasti.borrow()].clone()
     }
 
-    fn push_block(&mut self, typ: BlockType) {
-        self.blocks.push(Block {
+    fn push_block(&self, typ: BlockType) {
+        self.blocks.borrow_mut().push(Block {
             typ,
             level: self.stack.borrow().len(),
         });
     }
 
-    fn pop_block(&mut self) -> Option<Block> {
-        let block = self.blocks.pop()?;
+    fn pop_block(&self) -> Option<Block> {
+        let block = self.blocks.borrow_mut().pop()?;
         self.stack.borrow_mut().truncate(block.level);
         Some(block)
     }
 
-    fn current_block(&self) -> Option<&Block> {
-        self.blocks.last()
+    fn current_block(&self) -> Option<Block> {
+        self.blocks.borrow().last().cloned()
     }
 
     pub fn push_value(&self, obj: PyObjectRef) {
@@ -1094,7 +1080,7 @@ impl Frame {
         self.stack.borrow_mut().pop().unwrap()
     }
 
-    fn pop_multiple(&mut self, count: usize) -> Vec<PyObjectRef> {
+    fn pop_multiple(&self, count: usize) -> Vec<PyObjectRef> {
         let mut objs: Vec<PyObjectRef> = Vec::new();
         for _x in 0..count {
             objs.push(self.pop_value());
@@ -1123,6 +1109,7 @@ impl fmt::Debug for Frame {
             .join("");
         let block_str = self
             .blocks
+            .borrow()
             .iter()
             .map(|elem| format!("\n  > {:?}", elem))
             .collect::<Vec<_>>()

vm/src/obj/objframe.rs

@@ -41,12 +41,12 @@ fn frame_flocals(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 
 fn frame_fcode(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(vm, args, required = [(frame, Some(vm.ctx.frame_type()))]);
-    Ok(vm.ctx.new_code_object(get_value(frame).code))
+    Ok(vm.ctx.new_code_object(get_value(frame).code.clone()))
 }
 
-pub fn get_value(obj: &PyObjectRef) -> Frame {
+pub fn get_value(obj: &PyObjectRef) -> &Frame {
     if let PyObjectPayload::Frame { frame } = &obj.payload {
-        frame.clone()
+        frame
     } else {
         panic!("Inner error getting int {:?}", obj);
     }

vm/src/vm.rs

@@ -82,7 +82,7 @@ impl VirtualMachine {
 
     pub fn run_frame(&mut self, frame: PyObjectRef) -> Result<ExecutionResult, PyObjectRef> {
         self.frames.push(frame.clone());
-        let mut frame = objframe::get_value(&frame);
+        let frame = objframe::get_value(&frame);
         let result = frame.run(self);
         self.frames.pop();
         result