From 97029af713d487c31e86b9791bfe69d2a4622645 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sat, 28 Nov 2020 14:51:28 -0600 Subject: [PATCH] Fast locals part 1 --- bytecode/src/bytecode.rs | 127 +++++++----- compiler/src/compile.rs | 347 +++++++++++++++++++-------------- compiler/src/symboltable.rs | 85 ++++---- vm/src/builtins/dict.rs | 7 +- vm/src/builtins/frame.rs | 6 +- vm/src/builtins/function.rs | 86 ++++---- vm/src/builtins/make_module.rs | 24 +-- vm/src/builtins/pysuper.rs | 38 ++-- vm/src/builtins/tuple.rs | 10 +- vm/src/frame.rs | 242 +++++++++++++++-------- vm/src/pyobject.rs | 26 +-- vm/src/scope.rs | 198 +++++++++---------- vm/src/stdlib/symtable.rs | 4 +- vm/src/vm.rs | 26 +-- 14 files changed, 703 insertions(+), 523 deletions(-) diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 74a74af50..42caf7912 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -108,6 +108,9 @@ pub struct CodeObject { serialize = "C::Name: serde::Serialize" ))] pub names: Vec, + pub varnames: Vec, + pub cellvars: Vec, + pub freevars: Vec, } bitflags! { @@ -121,6 +124,7 @@ bitflags! { const IS_COROUTINE = 0x20; const HAS_VARARGS = 0x40; const HAS_VARKEYWORDS = 0x80; + const IS_OPTIMIZED = 0x0100; } } @@ -154,22 +158,6 @@ impl fmt::Display for Label { } } -#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] -/// An indication where the name must be accessed. -pub enum NameScope { - /// The name will be in the local scope. - Local, - - /// The name will be located in scope surrounding the current scope. - NonLocal, - - /// The name will be in global scope. - Global, - - /// The name will be located in any scope between the current scope and the top scope. - Free, -} - /// Transforms a value prior to formatting it. #[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum ConversionFlag { @@ -195,17 +183,20 @@ pub enum Instruction { ImportFrom { idx: NameIdx, }, - LoadName { - idx: NameIdx, - scope: NameScope, - }, - StoreName { - idx: NameIdx, - scope: NameScope, - }, - DeleteName { - idx: NameIdx, - }, + LoadFast(NameIdx), + LoadLocal(NameIdx), + LoadGlobal(NameIdx), + LoadDeref(NameIdx), + LoadClassDeref(NameIdx), + StoreFast(NameIdx), + StoreLocal(NameIdx), + StoreGlobal(NameIdx), + StoreDeref(NameIdx), + DeleteFast(NameIdx), + DeleteLocal(NameIdx), + DeleteGlobal(NameIdx), + DeleteDeref(NameIdx), + LoadClosure(NameIdx), Subscript, StoreSubscript, DeleteSubscript, @@ -561,6 +552,9 @@ impl CodeObject { obj_name, constants: Vec::new(), names: Vec::new(), + varnames: Vec::new(), + cellvars: Vec::new(), + freevars: Vec::new(), } } @@ -569,19 +563,19 @@ impl CodeObject { let nargs = self.arg_count; let nkwargs = self.kwonlyarg_count; let mut varargspos = nargs + nkwargs; - let posonlyargs = &self.names[..self.posonlyarg_count]; - let args = &self.names[..nargs]; - let kwonlyargs = &self.names[nargs..varargspos]; + let posonlyargs = &self.varnames[..self.posonlyarg_count]; + let args = &self.varnames[..nargs]; + let kwonlyargs = &self.varnames[nargs..varargspos]; let vararg = if self.flags.contains(CodeFlags::HAS_VARARGS) { - let vararg = &self.names[varargspos]; + let vararg = &self.varnames[varargspos]; varargspos += 1; Some(vararg) } else { None }; let varkwarg = if self.flags.contains(CodeFlags::HAS_VARKEYWORDS) { - Some(&self.names[varargspos]) + Some(&self.varnames[varargspos]) } else { None }; @@ -617,8 +611,10 @@ impl CodeObject { } #[rustfmt::skip] - Import { .. } | ImportStar | ImportFrom { .. } | LoadName { .. } | StoreName { .. } - | DeleteName { .. } | Subscript | StoreSubscript | DeleteSubscript + Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadLocal(_) + | LoadGlobal(_) | LoadDeref(_) | LoadClassDeref(_) | StoreFast(_) | StoreLocal(_) + | StoreGlobal(_) | StoreDeref(_) | DeleteFast(_) | DeleteLocal(_) | DeleteGlobal(_) + | DeleteDeref(_) | LoadClosure(_) | Subscript | StoreSubscript | DeleteSubscript | StoreAttr { .. } | DeleteAttr { .. } | LoadConst { .. } | UnaryOperation { .. } | BinaryOperation { .. } | LoadAttr { .. } | CompareOperation { .. } | Pop | Rotate { .. } | Duplicate | GetIter | Continue | Break | MakeFunction @@ -652,7 +648,16 @@ impl CodeObject { write!(f, " ")?; } write!(f, "{} {:5} ", arrow, offset)?; - instruction.fmt_dis(f, &self.constants, &self.names, expand_codeobjects, level)?; + instruction.fmt_dis( + f, + &self.constants, + &self.names, + &self.varnames, + &self.cellvars, + &self.freevars, + expand_codeobjects, + level, + )?; } Ok(()) } @@ -668,17 +673,22 @@ impl CodeObject { } pub fn map_bag(self, bag: &Bag) -> CodeObject { + let map_names = |names: Vec| { + names + .into_iter() + .map(|x| bag.make_name_ref(x.as_ref())) + .collect::>() + }; CodeObject { constants: self .constants .into_iter() .map(|x| x.map_constant(bag)) .collect(), - names: self - .names - .into_iter() - .map(|x| bag.make_name_ref(x.as_ref())) - .collect(), + names: map_names(self.names), + varnames: map_names(self.varnames), + cellvars: map_names(self.cellvars), + freevars: map_names(self.freevars), instructions: self.instructions, locations: self.locations, @@ -693,17 +703,22 @@ impl CodeObject { } pub fn map_clone_bag(&self, bag: &Bag) -> CodeObject { + let map_names = |names: &[C::Name]| { + names + .iter() + .map(|x| bag.make_name_ref(x.as_ref())) + .collect() + }; CodeObject { constants: self .constants .iter() .map(|x| bag.make_constant_borrowed(x.borrow_constant())) .collect(), - names: self - .names - .iter() - .map(|x| bag.make_name_ref(x.as_ref())) - .collect(), + names: map_names(&self.names), + varnames: map_names(&self.varnames), + cellvars: map_names(&self.cellvars), + freevars: map_names(&self.freevars), instructions: self.instructions.clone(), locations: self.locations.clone(), @@ -755,6 +770,9 @@ impl Instruction { f: &mut fmt::Formatter, constants: &[C], names: &[C::Name], + varnames: &[C::Name], + cellvars: &[C::Name], + freevars: &[C::Name], expand_codeobjects: bool, level: usize, ) -> fmt::Result { @@ -780,6 +798,8 @@ impl Instruction { }; } + let cellname = |i: usize| cellvars.get(i).unwrap_or_else(|| &freevars[i]).as_ref(); + match self { Import { name_idx, @@ -799,9 +819,20 @@ impl Instruction { ), ImportStar => w!(ImportStar), ImportFrom { idx } => w!(ImportFrom, names[*idx].as_ref()), - LoadName { idx, scope } => w!(LoadName, names[*idx].as_ref(), format!("{:?}", scope)), - StoreName { idx, scope } => w!(StoreName, names[*idx].as_ref(), format!("{:?}", scope)), - DeleteName { idx } => w!(DeleteName, names[*idx].as_ref()), + LoadFast(idx) => w!(LoadFast, varnames[*idx].as_ref()), + LoadLocal(idx) => w!(LoadLocal, names[*idx].as_ref()), + LoadGlobal(idx) => w!(LoadGlobal, names[*idx].as_ref()), + LoadDeref(idx) => w!(LoadDeref, cellname(*idx)), + LoadClassDeref(idx) => w!(LoadClassDeref, cellname(*idx)), + StoreFast(idx) => w!(StoreFast, varnames[*idx].as_ref()), + StoreLocal(idx) => w!(StoreLocal, names[*idx].as_ref()), + StoreGlobal(idx) => w!(StoreGlobal, names[*idx].as_ref()), + StoreDeref(idx) => w!(StoreDeref, cellname(*idx)), + DeleteFast(idx) => w!(DeleteFast, varnames[*idx].as_ref()), + DeleteLocal(idx) => w!(DeleteLocal, names[*idx].as_ref()), + DeleteGlobal(idx) => w!(DeleteGlobal, names[*idx].as_ref()), + DeleteDeref(idx) => w!(DeleteDeref, cellname(*idx)), + LoadClosure(i) => w!(LoadClosure, cellname(*i)), Subscript => w!(Subscript), StoreSubscript => w!(StoreSubscript), DeleteSubscript => w!(DeleteSubscript), diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index d06188307..3137b47b4 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -7,9 +7,7 @@ use crate::error::{CompileError, CompileErrorType}; pub use crate::mode::Mode; -use crate::symboltable::{ - make_symbol_table, statements_to_symbol_table, Symbol, SymbolScope, SymbolTable, -}; +use crate::symboltable::{make_symbol_table, statements_to_symbol_table, SymbolScope, SymbolTable}; use indexmap::IndexSet; use itertools::Itertools; use num_complex::Complex64; @@ -21,6 +19,9 @@ type CompileResult = Result; struct CodeInfo { code: CodeObject, name_cache: IndexSet, + varname_cache: IndexSet, + cellvar_cache: IndexSet, + freevar_cache: IndexSet, label_map: Vec>, } impl CodeInfo { @@ -28,9 +29,17 @@ impl CodeInfo { let CodeInfo { mut code, name_cache, + varname_cache, + cellvar_cache, + freevar_cache, label_map, } = self; + code.names.extend(name_cache); + code.varnames.extend(varname_cache); + code.cellvars.extend(cellvar_cache); + code.freevars.extend(freevar_cache); + for instruction in &mut code.instructions { use Instruction::*; // this is a little bit hacky, as until now the data stored inside Labels in @@ -55,8 +64,10 @@ impl CodeInfo { } #[rustfmt::skip] - Import { .. } | ImportStar | ImportFrom { .. } | LoadName { .. } | StoreName { .. } - | DeleteName { .. } | Subscript | StoreSubscript | DeleteSubscript + Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadLocal(_) + | LoadGlobal(_) | LoadDeref(_) | LoadClassDeref(_) | StoreFast(_) | StoreLocal(_) + | StoreGlobal(_) | StoreDeref(_) | DeleteFast(_) | DeleteLocal(_) | DeleteGlobal(_) + | DeleteDeref(_) | LoadClosure(_) | Subscript | StoreSubscript | DeleteSubscript | StoreAttr { .. } | DeleteAttr { .. } | LoadConst { .. } | UnaryOperation { .. } | BinaryOperation { .. } | LoadAttr { .. } | CompareOperation { .. } | Pop | Rotate { .. } | Duplicate | GetIter | Continue | Break | MakeFunction @@ -73,6 +84,12 @@ impl CodeInfo { } } +enum NameUsage { + Load, + Store, + Delete, +} + /// Main structure holding the state of compilation. struct Compiler { code_stack: Vec, @@ -100,6 +117,7 @@ impl Default for CompileOpts { #[derive(Clone, Copy)] struct CompileContext { in_loop: bool, + in_class: bool, func: FunctionContext, } @@ -122,8 +140,7 @@ fn with_compiler( opts: CompileOpts, f: impl FnOnce(&mut Compiler) -> CompileResult<()>, ) -> CompileResult { - let mut compiler = Compiler::new(opts, source_path); - compiler.push_new_code_object("".to_owned()); + let mut compiler = Compiler::new(opts, source_path, "".to_owned()); f(&mut compiler)?; let code = compiler.pop_code_object(); trace!("Compilation completed: {:?}", code); @@ -176,9 +193,25 @@ pub fn compile_program_single( } impl Compiler { - fn new(opts: CompileOpts, source_path: String) -> Self { + fn new(opts: CompileOpts, source_path: String, code_name: String) -> Self { + let module_code = CodeInfo { + code: CodeObject::new( + Default::default(), + 0, + 0, + 0, + source_path.clone(), + 0, + code_name, + ), + name_cache: IndexSet::new(), + varname_cache: IndexSet::new(), + cellvar_cache: IndexSet::new(), + freevar_cache: IndexSet::new(), + label_map: Vec::new(), + }; Compiler { - code_stack: Vec::new(), + code_stack: vec![module_code], symbol_table_stack: Vec::new(), source_path, current_source_location: ast::Location::default(), @@ -186,6 +219,7 @@ impl Compiler { done_with_future_stmts: false, ctx: CompileContext { in_loop: false, + in_class: false, func: FunctionContext::NoFunction, }, opts, @@ -204,43 +238,63 @@ impl Compiler { } fn push_output(&mut self, code: CodeObject) { - self.code_stack.push(CodeInfo { + let table = self + .symbol_table_stack + .last_mut() + .unwrap() + .sub_tables + .remove(0); + + let cellvar_cache = table + .symbols + .iter() + .filter(|(_, s)| matches!(s.scope, SymbolScope::Cell)) + .map(|(var, _)| var.clone()) + .collect(); + let freevar_cache = table + .symbols + .iter() + // TODO: check if Free or FREE_CLASS symbol + .filter(|(_, s)| matches!(s.scope, SymbolScope::Free)) + .map(|(var, _)| var.clone()) + .collect(); + + self.symbol_table_stack.push(table); + + let info = CodeInfo { code, name_cache: IndexSet::new(), + varname_cache: IndexSet::new(), + cellvar_cache, + freevar_cache, label_map: Vec::new(), - }); - } - - fn push_new_code_object(&mut self, obj_name: String) { - let line_number = self.get_source_line_number(); - self.push_output(CodeObject::new( - Default::default(), - 0, - 0, - 0, - self.source_path.clone(), - line_number, - obj_name, - )); + }; + self.code_stack.push(info); } fn pop_code_object(&mut self) -> CodeObject { + let table = self.symbol_table_stack.pop().unwrap(); + assert!(table.sub_tables.is_empty()); self.code_stack.pop().unwrap().finalize_code() } // could take impl Into>, but everything is borrowed from ast structs; we never // actually have a `String` to pass fn name(&mut self, name: &str) -> bytecode::NameIdx { - let cache = &mut self - .code_stack - .last_mut() - .expect("nothing on stack") - .name_cache; - if let Some(x) = cache.get_index_of(name) { - x - } else { - cache.insert_full(name.to_owned()).0 - } + self._name_inner(name, |i| &mut i.name_cache) + } + fn varname(&mut self, name: &str) -> bytecode::NameIdx { + self._name_inner(name, |i| &mut i.varname_cache) + } + fn _name_inner( + &mut self, + name: &str, + cache: impl FnOnce(&mut CodeInfo) -> &mut IndexSet, + ) -> bytecode::NameIdx { + let cache = cache(self.code_stack.last_mut().expect("nothing on stack")); + cache + .get_index_of(name) + .unwrap_or_else(|| cache.insert_full(name.to_owned()).0) } fn compile_program( @@ -255,10 +309,7 @@ impl Compiler { if let Some(value) = doc { self.emit_constant(bytecode::ConstantData::Str { value }); let doc = self.name("__doc__"); - self.emit(Instruction::StoreName { - idx: doc, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::StoreGlobal(doc)) } self.compile_statements(statements)?; @@ -331,34 +382,73 @@ impl Compiler { Ok(()) } - fn scope_for_name(&self, name: &str) -> bytecode::NameScope { - let symbol = self.lookup_name(name); - match symbol.scope { - SymbolScope::Global => bytecode::NameScope::Global, - SymbolScope::Nonlocal => bytecode::NameScope::NonLocal, - SymbolScope::Unknown => bytecode::NameScope::Free, - SymbolScope::Local => { - // Only in function block, we use load local - // https://github.com/python/cpython/blob/master/Python/compile.c#L3582 - if self.ctx.in_func() { - bytecode::NameScope::Local - } else { - bytecode::NameScope::Free - } - } - } - } - fn load_name(&mut self, name: &str) { - let scope = self.scope_for_name(name); - let idx = self.name(name); - self.emit(Instruction::LoadName { idx, scope }); + self.compile_name(name, NameUsage::Load) } fn store_name(&mut self, name: &str) { - let scope = self.scope_for_name(name); - let idx = self.name(name); - self.emit(Instruction::StoreName { idx, scope }); + self.compile_name(name, NameUsage::Store) + } + + fn compile_name(&mut self, name: &str, usage: NameUsage) { + let symbol_table = self.symbol_table_stack.last().unwrap(); + let symbol = symbol_table.lookup(name).expect( + "The symbol must be present in the symbol table, even when it is undefined in python.", + ); + let info = self.code_stack.last_mut().unwrap(); + let mut cache = &mut info.name_cache; + enum NameOpType { + Fast, + Global, + Deref, + Local, + } + let op_typ = match symbol.scope { + SymbolScope::Local if self.ctx.in_func() => { + cache = &mut info.varname_cache; + NameOpType::Fast + } + SymbolScope::Local => NameOpType::Local, + SymbolScope::GlobalImplicit if self.ctx.in_func() => NameOpType::Global, + SymbolScope::GlobalImplicit => NameOpType::Local, + SymbolScope::GlobalExplicit => NameOpType::Global, + SymbolScope::Free => { + cache = &mut info.freevar_cache; + NameOpType::Deref + } + SymbolScope::Cell => { + cache = &mut info.cellvar_cache; + NameOpType::Deref + } + // TODO: is this right? + SymbolScope::Unknown => NameOpType::Global, + }; + let idx = cache + .get_index_of(name) + .unwrap_or_else(|| cache.insert_full(name.to_owned()).0); + let op = match op_typ { + NameOpType::Fast => match usage { + NameUsage::Load => Instruction::LoadFast, + NameUsage::Store => Instruction::StoreFast, + NameUsage::Delete => Instruction::DeleteFast, + }, + NameOpType::Global => match usage { + NameUsage::Load => Instruction::LoadGlobal, + NameUsage::Store => Instruction::StoreGlobal, + NameUsage::Delete => Instruction::DeleteGlobal, + }, + NameOpType::Deref => match usage { + NameUsage::Load => Instruction::LoadDeref, + NameUsage::Store => Instruction::StoreDeref, + NameUsage::Delete => Instruction::DeleteDeref, + }, + NameOpType::Local => match usage { + NameUsage::Load => Instruction::LoadLocal, + NameUsage::Store => Instruction::StoreLocal, + NameUsage::Delete => Instruction::DeleteLocal, + }, + }; + self.emit(op(idx)); } fn compile_statement(&mut self, statement: &ast::Statement) -> CompileResult<()> { @@ -584,10 +674,7 @@ impl Compiler { let end_label = self.new_label(); self.compile_jump_if(test, true, end_label)?; let assertion_error = self.name("AssertionError"); - self.emit(Instruction::LoadName { - idx: assertion_error, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::LoadGlobal(assertion_error)); match msg { Some(e) => { self.compile_expression(e)?; @@ -683,8 +770,7 @@ impl Compiler { fn compile_delete(&mut self, expression: &ast::Expression) -> CompileResult<()> { match &expression.node { ast::ExpressionType::Identifier { name } => { - let idx = self.name(name); - self.emit(Instruction::DeleteName { idx }); + self.compile_name(name, NameUsage::Delete); } ast::ExpressionType::Attribute { value, name } => { self.compile_expression(value)?; @@ -738,7 +824,7 @@ impl Compiler { }); } - let mut flags = bytecode::CodeFlags::default(); + let mut flags = bytecode::CodeFlags::NEW_LOCALS | bytecode::CodeFlags::IS_OPTIMIZED; if have_defaults { flags |= bytecode::CodeFlags::HAS_DEFAULTS; } @@ -775,8 +861,6 @@ impl Compiler { compile_varargs(&args.vararg, bytecode::CodeFlags::HAS_VARARGS); compile_varargs(&args.kwarg, bytecode::CodeFlags::HAS_VARKEYWORDS); - self.enter_scope(); - Ok(()) } @@ -923,6 +1007,7 @@ impl Compiler { self.ctx = CompileContext { in_loop: false, + in_class: prev_ctx.in_class, func: if is_async { FunctionContext::AsyncFunction } else { @@ -954,7 +1039,6 @@ impl Compiler { } let mut code = self.pop_code_object(); - self.leave_scope(); // Prepare type annotations: let mut num_annotations = 0; @@ -1006,6 +1090,27 @@ impl Compiler { code.flags |= bytecode::CodeFlags::IS_COROUTINE; } + if !code.freevars.is_empty() { + for var in &code.freevars { + let symbol = self.symbol_table_stack.last().unwrap().lookup(var).unwrap(); + let parent_code = self.code_stack.last().unwrap(); + let vars = match symbol.scope { + SymbolScope::Free => &parent_code.freevar_cache, + SymbolScope::Cell => &parent_code.cellvar_cache, + _ => unreachable!(), + }; + let mut idx = vars.get_index_of(var).unwrap(); + if let SymbolScope::Free = symbol.scope { + idx += parent_code.cellvar_cache.len(); + } + self.emit(Instruction::LoadClosure(idx)) + } + self.emit(Instruction::BuildTuple { + size: code.freevars.len(), + unpack: false, + }) + } + self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), }); @@ -1098,6 +1203,7 @@ impl Compiler { let prev_ctx = self.ctx; self.ctx = CompileContext { func: FunctionContext::NoFunction, + in_class: true, in_loop: false, }; @@ -1109,7 +1215,7 @@ impl Compiler { self.emit(Instruction::LoadBuildClass); let line_number = self.get_source_line_number(); self.push_output(CodeObject::new( - Default::default(), + bytecode::CodeFlags::empty(), 0, 0, 0, @@ -1117,34 +1223,21 @@ impl Compiler { line_number, name.to_owned(), )); - self.enter_scope(); let (new_body, doc_str) = get_doc(body); let dunder_name = self.name("__name__"); - self.emit(Instruction::LoadName { - idx: dunder_name, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::LoadGlobal(dunder_name)); let dunder_module = self.name("__module__"); - self.emit(Instruction::StoreName { - idx: dunder_module, - scope: bytecode::NameScope::Free, - }); + self.emit(Instruction::StoreLocal(dunder_module)); self.emit_constant(bytecode::ConstantData::Str { value: qualified_name.clone(), }); let qualname = self.name("__qualname__"); - self.emit(Instruction::StoreName { - idx: qualname, - scope: bytecode::NameScope::Free, - }); + self.emit(Instruction::StoreLocal(qualname)); self.load_docstring(doc_str); let doc = self.name("__doc__"); - self.emit(Instruction::StoreName { - idx: doc, - scope: bytecode::NameScope::Free, - }); + self.emit(Instruction::StoreLocal(doc)); // setup annotations if self.find_ann(body) { self.emit(Instruction::SetupAnnotation); @@ -1153,9 +1246,7 @@ impl Compiler { self.emit_constant(bytecode::ConstantData::None); self.emit(Instruction::ReturnValue); - let mut code = self.pop_code_object(); - code.flags.remove(bytecode::CodeFlags::NEW_LOCALS); - self.leave_scope(); + let code = self.pop_code_object(); self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), @@ -1295,10 +1386,7 @@ impl Compiler { self.set_label(check_asynciter_label); self.emit(Instruction::Duplicate); let stopasynciter = self.name("StopAsyncIteration"); - self.emit(Instruction::LoadName { - idx: stopasynciter, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::LoadGlobal(stopasynciter)); self.emit(Instruction::CompareOperation { op: bytecode::ComparisonOperator::ExceptionMatch, }); @@ -1436,15 +1524,14 @@ impl Compiler { if let ast::ExpressionType::Identifier { name } = &target.node { // Store as dict entry in __annotations__ dict: - let annotations = self.name("__annotations__"); - self.emit(Instruction::LoadName { - idx: annotations, - scope: bytecode::NameScope::Local, - }); - self.emit_constant(bytecode::ConstantData::Str { - value: name.to_owned(), - }); - self.emit(Instruction::StoreSubscript); + if !self.ctx.in_func() { + let annotations = self.name("__annotations__"); + self.emit(Instruction::LoadLocal(annotations)); + self.emit_constant(bytecode::ConstantData::Str { + value: name.to_owned(), + }); + self.emit(Instruction::StoreSubscript); + } } else { // Drop annotation if not assigned to simple identifier. self.emit(Instruction::Pop); @@ -1846,6 +1933,7 @@ impl Compiler { let prev_ctx = self.ctx; self.ctx = CompileContext { in_loop: false, + in_class: prev_ctx.in_class, func: FunctionContext::Function, }; @@ -1854,7 +1942,6 @@ impl Compiler { self.compile_expression(body)?; self.emit(Instruction::ReturnValue); let code = self.pop_code_object(); - self.leave_scope(); self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), }); @@ -2044,8 +2131,7 @@ impl Compiler { line_number, name.clone(), )); - let arg0 = self.name(".0"); - self.enter_scope(); + let arg0 = self.varname(".0"); // Create empty object of proper type: match kind { @@ -2079,10 +2165,7 @@ impl Compiler { if loop_labels.is_empty() { // Load iterator onto stack (passed as first argument): - self.emit(Instruction::LoadName { - idx: arg0, - scope: bytecode::NameScope::Local, - }); + self.emit(Instruction::LoadFast(arg0)); } else { // Evaluate iterated item: self.compile_expression(&generator.iter)?; @@ -2169,9 +2252,6 @@ impl Compiler { // Fetch code for listcomp function: let code = self.pop_code_object(); - // Pop scope - self.leave_scope(); - // List comprehension code: self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), @@ -2255,33 +2335,6 @@ impl Compiler { Ok(()) } - // Scope helpers: - fn enter_scope(&mut self) { - // println!("Enter scope {:?}", self.symbol_table_stack); - // Enter first subscope! - let table = self - .symbol_table_stack - .last_mut() - .unwrap() - .sub_tables - .remove(0); - self.symbol_table_stack.push(table); - } - - fn leave_scope(&mut self) { - // println!("Leave scope {:?}", self.symbol_table_stack); - let table = self.symbol_table_stack.pop().unwrap(); - assert!(table.sub_tables.is_empty()); - } - - fn lookup_name(&self, name: &str) -> &Symbol { - // println!("Looking up {:?}", name); - let symbol_table = self.symbol_table_stack.last().unwrap(); - symbol_table.lookup(name).expect( - "The symbol must be present in the symbol table, even when it is undefined in python.", - ) - } - // Low level helper functions: fn emit(&mut self, instruction: Instruction) { let location = compile_location(&self.current_source_location); @@ -2402,9 +2455,11 @@ mod tests { use rustpython_parser::parser; fn compile_exec(source: &str) -> CodeObject { - let mut compiler: Compiler = - Compiler::new(CompileOpts::default(), "source_path".to_owned()); - compiler.push_new_code_object("".to_owned()); + let mut compiler: Compiler = Compiler::new( + CompileOpts::default(), + "source_path".to_owned(), + "".to_owned(), + ); let ast = parser::parse_program(source).unwrap(); let symbol_scope = make_symbol_table(&ast).unwrap(); compiler.compile_program(&ast, symbol_scope).unwrap(); diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 276e55c49..02f15ae79 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -64,7 +64,7 @@ impl SymbolTable { } } -#[derive(Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum SymbolTableType { Module, Class, @@ -85,12 +85,14 @@ impl fmt::Display for SymbolTableType { /// Indicator for a single symbol what the scope of this symbol is. /// The scope can be unknown, which is unfortunate, but not impossible. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub enum SymbolScope { - Global, - Nonlocal, - Local, Unknown, + Local, + GlobalExplicit, + GlobalImplicit, + Free, + Cell, } /// A single symbol in a table. Has various properties such as the scope @@ -104,7 +106,6 @@ pub struct Symbol { pub is_referenced: bool, pub is_assigned: bool, pub is_parameter: bool, - pub is_free: bool, pub is_annotated: bool, pub is_imported: bool, @@ -126,7 +127,6 @@ impl Symbol { is_referenced: false, is_assigned: false, is_parameter: false, - is_free: false, is_annotated: false, is_imported: false, is_assign_namedexpr_in_comprehension: false, @@ -135,7 +135,10 @@ impl Symbol { } pub fn is_global(&self) -> bool { - matches!(self.scope, SymbolScope::Global) + matches!( + self.scope, + SymbolScope::GlobalExplicit | SymbolScope::GlobalImplicit + ) } pub fn is_local(&self) -> bool { @@ -228,7 +231,7 @@ impl<'a> SymbolTableAnalyzer<'a> { self.analyze_symbol_comprehension(symbol, 0)? } else { match symbol.scope { - SymbolScope::Nonlocal => { + SymbolScope::Free => { let scope_depth = self.tables.len(); if scope_depth > 0 { // check if the name is already defined in any outer scope @@ -251,10 +254,10 @@ impl<'a> SymbolTableAnalyzer<'a> { }); } } - SymbolScope::Global => { + SymbolScope::GlobalExplicit | SymbolScope::GlobalImplicit => { // TODO: add more checks for globals? } - SymbolScope::Local => { + SymbolScope::Local | SymbolScope::Cell => { // all is well } SymbolScope::Unknown => { @@ -270,24 +273,28 @@ impl<'a> SymbolTableAnalyzer<'a> { // Interesting stuff about the __class__ variable: // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object symbol.name == "__class__" - || self.tables.iter().skip(1).any(|(symbols, typ)| { - *typ != SymbolTableType::Class && symbols.contains_key(&symbol.name) + || self.tables.iter().skip(1).rev().any(|(symbols, typ)| { + *typ != SymbolTableType::Class + && symbols + .get(&symbol.name) + .map_or(false, |sym| sym.is_local() && sym.is_assigned) }) } fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { - if symbol.is_assigned || symbol.is_parameter { - symbol.scope = SymbolScope::Local; + let scope = if symbol.is_assigned || symbol.is_parameter { + SymbolScope::Local } else if self.found_in_outer_scope(symbol) { // Symbol is in some outer scope. - symbol.is_free = true; + SymbolScope::Free } else if self.tables.is_empty() { // Don't make assumptions when we don't know. - symbol.scope = SymbolScope::Unknown; + SymbolScope::Unknown } else { - // If there are scopes above we can assume global. - symbol.scope = SymbolScope::Global; - } + // If there are scopes above we assume global. + SymbolScope::GlobalImplicit + }; + symbol.scope = scope; } // Implements the symbol analysis and scope extension for names @@ -302,7 +309,7 @@ impl<'a> SymbolTableAnalyzer<'a> { // when this is called, we expect to be in the direct parent scope of the scope that contains 'symbol' let offs = self.tables.len() - 1 - parent_offset; let last = self.tables.get_mut(offs).unwrap(); - let symbols = &mut last.0; + let symbols = &mut *last.0; let table_type = last.1; // it is not allowed to use an iterator variable as assignee in a named expression @@ -319,7 +326,7 @@ impl<'a> SymbolTableAnalyzer<'a> { match table_type { SymbolTableType::Module => { - symbol.scope = SymbolScope::Global; + symbol.scope = SymbolScope::GlobalImplicit; } SymbolTableType::Class => { // named expressions are forbidden in comprehensions on class scope @@ -327,22 +334,18 @@ impl<'a> SymbolTableAnalyzer<'a> { error: "assignment expression within a comprehension cannot be used in a class body".to_string(), // TODO: accurate location info, somehow location: Location::default(), - } ); + }); } SymbolTableType::Function => { if let Some(parent_symbol) = symbols.get_mut(&symbol.name) { if let SymbolScope::Unknown = parent_symbol.scope { - parent_symbol.is_assigned = true; // this information is new, as the asignment is done in inner scope - //self.analyze_unknown_symbol(symbol); // not needed, symbol is analyzed anyhow when its scope is analyzed + // this information is new, as the asignment is done in inner scope + parent_symbol.is_assigned = true; + //self.analyze_unknown_symbol(symbol); // not needed, symbol is analyzed anyhow when its scope is analyzed } - match symbol.scope { - SymbolScope::Global => { - symbol.scope = SymbolScope::Global; - } - _ => { - symbol.scope = SymbolScope::Nonlocal; - } + if !symbol.is_global() { + symbol.scope = SymbolScope::Free; } } else { let mut cloned_sym = symbol.clone(); @@ -372,7 +375,7 @@ impl<'a> SymbolTableAnalyzer<'a> { // ouside, too, and set it therefore to non-local scope. I.e., we expect to // find a definition on a higher level let mut cloned_sym = symbol.clone(); - cloned_sym.scope = SymbolScope::Nonlocal; + cloned_sym.scope = SymbolScope::Free; last.0.insert(cloned_sym.name.to_owned(), cloned_sym); } } @@ -408,7 +411,7 @@ struct SymbolTableBuilder { /// was used. /// In cpython this is stored in the AST, but I think this /// is not logical, since it is not context free. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq)] enum ExpressionContext { Load, Store, @@ -825,6 +828,12 @@ impl SymbolTableBuilder { self.register_name(name, SymbolUsage::Iter, location)?; } } + if context == ExpressionContext::Load + && self.tables.last().unwrap().typ == SymbolTableType::Function + && name == "super" + { + self.register_name("__class__", SymbolUsage::Used, location)?; + } } Lambda { args, body } => { self.enter_function("lambda", args, expression.location.row())?; @@ -949,7 +958,7 @@ impl SymbolTableBuilder { // Role already set.. match role { SymbolUsage::Global => { - if let SymbolScope::Global = symbol.scope { + if symbol.is_global() { // Ok } else { return Err(SymbolTableError { @@ -1014,7 +1023,7 @@ impl SymbolTableBuilder { let symbol = table.symbols.get_mut(name).unwrap(); match role { SymbolUsage::Nonlocal => { - symbol.scope = SymbolScope::Nonlocal; + symbol.scope = SymbolScope::Free; } SymbolUsage::Imported => { symbol.is_assigned = true; @@ -1040,8 +1049,8 @@ impl SymbolTableBuilder { } SymbolUsage::Global => { if let SymbolScope::Unknown = symbol.scope { - symbol.scope = SymbolScope::Global; - } else if let SymbolScope::Global = symbol.scope { + symbol.scope = SymbolScope::GlobalImplicit; + } else if symbol.is_global() { // Global scope can be set to global } else { return Err(SymbolTableError { diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index 04b6241ff..a599102bc 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -500,19 +500,18 @@ where self.as_object().get_item(key, vm) } - fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { if self.class().is(&vm.ctx.types.dict_type) { self.inner_setitem_fast(key, value, vm) - .map(|_| vm.ctx.none()) } else { // Fall back to slow path if we are in a dict subclass: self.as_object().set_item(key, value, vm) } } - fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult { + fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult<()> { if self.class().is(&vm.ctx.types.dict_type) { - self.entries.delete(vm, key).map(|_| vm.ctx.none()) + self.entries.delete(vm, key) } else { // Fall back to slow path if we are in a dict subclass: self.as_object().del_item(key, vm) diff --git a/vm/src/builtins/frame.rs b/vm/src/builtins/frame.rs index 947dd673a..3203c725f 100644 --- a/vm/src/builtins/frame.rs +++ b/vm/src/builtins/frame.rs @@ -44,12 +44,12 @@ impl FrameRef { #[pyproperty] fn f_globals(self) -> PyDictRef { - self.scope.globals.clone() + self.globals.clone() } #[pyproperty] - fn f_locals(self) -> PyDictRef { - self.scope.get_locals().clone() + fn f_locals(self, vm: &VirtualMachine) -> PyDictRef { + self.locals(vm).clone() } #[pyproperty] diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index a62759b6c..2455fa396 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -5,7 +5,7 @@ use super::code::PyCodeRef; use super::dict::PyDictRef; use super::pystr::PyStrRef; use super::pytype::PyTypeRef; -use super::tuple::PyTupleRef; +use super::tuple::{PyTupleRef, PyTupleTyped}; use crate::builtins::asyncgenerator::PyAsyncGen; use crate::builtins::coroutine::PyCoroutine; use crate::builtins::generator::PyGenerator; @@ -36,7 +36,8 @@ pub struct PyFunction { code: PyCodeRef, #[cfg(feature = "jit")] jitted_code: OnceCell, - scope: Scope, + globals: PyDictRef, + closure: Option>, defaults: Option, kw_only_defaults: Option, } @@ -44,7 +45,8 @@ pub struct PyFunction { impl PyFunction { pub fn new( code: PyCodeRef, - scope: Scope, + globals: PyDictRef, + closure: Option>, defaults: Option, kw_only_defaults: Option, ) -> Self { @@ -52,25 +54,23 @@ impl PyFunction { code, #[cfg(feature = "jit")] jitted_code: OnceCell::new(), - scope, + globals, + closure, defaults, kw_only_defaults, } } - pub fn scope(&self) -> &Scope { - &self.scope - } - fn fill_locals_from_args( &self, - locals: &PyDictRef, + frame: &Frame, func_args: FuncArgs, vm: &VirtualMachine, ) -> PyResult<()> { + let code = &*self.code; let nargs = func_args.args.len(); - let nexpected_args = self.code.arg_count; - let arg_names = self.code.arg_names(); + let nexpected_args = code.arg_count; + // let arg_names = self.code.arg_names(); // This parses the arguments from args and kwargs into // the proper variables keeping into account default values @@ -78,23 +78,24 @@ impl PyFunction { // See also: PyEval_EvalCodeWithName in cpython: // https://github.com/python/cpython/blob/master/Python/ceval.c#L3681 - let n = if nargs > nexpected_args { - nexpected_args - } else { - nargs - }; + let n = std::cmp::min(nargs, nexpected_args); let mut args_iter = func_args.args.into_iter(); + let mut fastlocals = frame.fastlocals.lock(); + // let mut fill_locals = fastlocals.iter_mut().enumerate(); + // Copy positional arguments into local variables - for (arg, arg_name) in args_iter.by_ref().take(n).zip(arg_names.args) { - locals.set_item(arg_name.clone(), arg, vm)?; + for (i, arg) in args_iter.by_ref().take(n).enumerate() { + fastlocals[i] = Some(arg); } + let mut vararg_offset = code.arg_count; // Pack other positional arguments in to *args: - if let Some(vararg_name) = arg_names.vararg { + if code.flags.contains(bytecode::CodeFlags::HAS_VARARGS) { let vararg_value = vm.ctx.new_tuple(args_iter.collect()); - locals.set_item(vararg_name.clone(), vararg_value, vm)?; + fastlocals[vararg_offset] = Some(vararg_value); + vararg_offset += 1; } else { // Check the number of positional arguments if nargs > nexpected_args { @@ -106,25 +107,25 @@ impl PyFunction { } // Do we support `**kwargs` ? - let kwargs = if let Some(varkwarg) = arg_names.varkwarg { + let kwargs = if code.flags.contains(bytecode::CodeFlags::HAS_VARKEYWORDS) { let d = vm.ctx.new_dict(); - locals.set_item(varkwarg.clone(), d.as_object().clone(), vm)?; + fastlocals[vararg_offset] = Some(d.clone()); Some(d) } else { None }; - let contains_arg = - |names: &[PyStrRef], name: &str| names.iter().any(|s| s.borrow_value() == name); + // let argpos = + // |names: &[PyStrRef], name: &str| names.iter().position(|s| s.borrow_value() == name); let mut posonly_passed_as_kwarg = Vec::new(); // Handle keyword arguments for (name, value) in func_args.kwargs { // Check if we have a parameter with this name: - let dict = if contains_arg(arg_names.args, &name) - || contains_arg(arg_names.kwonlyargs, &name) + let dict = if let Some(pos) = + argpos(arg_names.args, &name).or_else(|| argpos(arg_names.kwonlyargs, &name)) { - if contains_arg(arg_names.posonlyargs, &name) { + if argpos(arg_names.posonlyargs, &name) { posonly_passed_as_kwarg.push(name); continue; } else if locals.contains_key(&name, vm) { @@ -138,7 +139,7 @@ impl PyFunction { vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name)) })? }; - dict.set_item(vm.intern_string(name).into_object(), value, vm)?; + // dict.set_item(vm.intern_string(name).into_object(), value, vm)?; } if !posonly_passed_as_kwarg.is_empty() { return Err(vm.new_type_error(format!( @@ -204,10 +205,10 @@ impl PyFunction { Ok(()) } - pub fn invoke_with_scope( + pub fn invoke_with_locals( &self, func_args: FuncArgs, - scope: &Scope, + locals: Option, vm: &VirtualMachine, ) -> PyResult { #[cfg(feature = "jit")] @@ -226,16 +227,22 @@ impl PyFunction { let code = &self.code; - let scope = if self.code.flags.contains(bytecode::CodeFlags::NEW_LOCALS) { - scope.new_child_scope(&vm.ctx) + let locals = if self.code.flags.contains(bytecode::CodeFlags::NEW_LOCALS) { + vm.ctx.new_dict() } else { - scope.clone() + locals.unwrap_or_else(|| self.globals.clone()) }; - self.fill_locals_from_args(&scope.get_locals(), func_args, vm)?; - // Construct frame: - let frame = Frame::new(code.clone(), scope, vm).into_ref(vm); + let frame = Frame::new( + code.clone(), + Scope::new(Some(locals), self.globals.clone()), + self.closure.as_ref().map_or(&[], |c| c.borrow_value()), + vm, + ) + .into_ref(vm); + + self.fill_locals_from_args(&frame, func_args, vm)?; // If we have a generator, create a new generator let is_gen = code.flags.contains(bytecode::CodeFlags::IS_GENERATOR); @@ -249,7 +256,7 @@ impl PyFunction { } pub fn invoke(&self, func_args: FuncArgs, vm: &VirtualMachine) -> PyResult { - self.invoke_with_scope(func_args, &self.scope, vm) + self.invoke_with_locals(func_args, None, vm) } } @@ -278,7 +285,7 @@ impl PyFunction { #[pyproperty(magic)] fn globals(&self) -> PyDictRef { - self.scope.globals.clone() + self.globals.clone() } #[cfg(feature = "jit")] @@ -393,10 +400,11 @@ impl PyValue for PyBoundMethod { } #[pyclass(module = false, name = "cell")] -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct PyCell { contents: PyMutex>, } +pub(crate) type PyCellRef = PyRef; impl PyValue for PyCell { fn class(vm: &VirtualMachine) -> &PyTypeRef { diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index 1b2396bb1..dcaab34bc 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -172,7 +172,7 @@ mod decl { fn dir(obj: OptionalArg, vm: &VirtualMachine) -> PyResult { let seq = match obj { OptionalArg::Present(obj) => vm.call_method(&obj, "__dir__", ())?, - OptionalArg::Missing => vm.call_method(&vm.get_locals().into_object(), "keys", ())?, + OptionalArg::Missing => vm.call_method(vm.current_locals().as_object(), "keys", ())?, }; let sorted = sorted(seq, Default::default(), vm)?; Ok(sorted) @@ -243,14 +243,13 @@ mod decl { #[cfg(feature = "rustpython-compiler")] fn make_scope(vm: &VirtualMachine, scope: ScopeArgs) -> PyResult { let globals = scope.globals; - let current_scope = vm.current_scope(); let locals = match scope.locals { Some(dict) => Some(dict), None => { if globals.is_some() { None } else { - current_scope.get_only_locals() + Some(vm.current_locals().clone()) } } }; @@ -262,7 +261,7 @@ mod decl { } dict } - None => current_scope.globals.clone(), + None => vm.current_globals().clone(), }; let scope = Scope::with_builtins(locals, globals, vm); @@ -318,7 +317,7 @@ mod decl { #[pyfunction] fn globals(vm: &VirtualMachine) -> PyResult { - Ok(vm.current_scope().globals.clone()) + Ok(vm.current_globals().clone()) } #[pyfunction] @@ -449,7 +448,7 @@ mod decl { #[pyfunction] fn locals(vm: &VirtualMachine) -> PyDictRef { - let locals = vm.get_locals(); + let locals = vm.current_locals(); locals.copy().into_ref(vm) } @@ -797,7 +796,7 @@ mod decl { if let OptionalArg::Present(obj) = obj { vm.get_attribute(obj, "__dict__") } else { - Ok(vm.get_locals().into_object()) + Ok(vm.current_locals().clone().into_object()) } } @@ -843,20 +842,13 @@ mod decl { let namespace: PyDictRef = TryFromObject::try_from_object(vm, namespace)?; - let cells = vm.ctx.new_dict(); - - let scope = function - .scope() - .new_child_scope_with_locals(cells.clone()) - .new_child_scope_with_locals(namespace.clone()); - - function.invoke_with_scope(().into(), &scope, vm)?; + function.invoke_with_locals(().into(), Some(namespace), vm)?; let class = vm.invoke( metaclass.as_object(), FuncArgs::new(vec![name_obj, bases, namespace.into_object()], kwargs), )?; - cells.set_item("__class__", class.clone(), vm)?; + // cells.set_item("__class__", class.clone(), vm)?; Ok(class) } } diff --git a/vm/src/builtins/pysuper.rs b/vm/src/builtins/pysuper.rs index b48c25c9d..f1950ee0a 100644 --- a/vm/src/builtins/pysuper.rs +++ b/vm/src/builtins/pysuper.rs @@ -61,16 +61,30 @@ impl PySuper { let typ = if let OptionalArg::Present(ty) = py_type { ty } else { - let obj = vm - .current_scope() - .load_cell(vm, "__class__") - .ok_or_else(|| { - vm.new_type_error( - "super must be called with 1 argument or from inside class method" - .to_owned(), - ) - })?; - PyTypeRef::try_from_object(vm, obj)? + let frame = vm + .current_frame() + .expect("super() called from outside of frame?"); + let mut typ = None; + for (i, var) in frame.code.freevars.iter().enumerate() { + if var.borrow_value() == "__class__" { + let i = frame.code.cellvars.len() + i; + let class = frame.cells_frees[i].get().ok_or_else(|| { + vm.new_runtime_error("super(): empty __class__ cell".to_owned()) + })?; + typ = Some(class.downcast().map_err(|o| { + vm.new_type_error(format!( + "super(): __class__ is not a type ({})", + o.class().name + )) + })?); + break; + } + } + typ.ok_or_else(|| { + vm.new_type_error( + "super must be called with 1 argument or from inside class method".to_owned(), + ) + })? }; // Check type argument: @@ -87,8 +101,8 @@ impl PySuper { } else { let frame = vm.current_frame().expect("no current frame for super()"); if let Some(first_arg) = frame.code.arg_names().args.get(0) { - let locals = frame.scope.get_locals(); - locals + frame + .locals .get_item_option(first_arg.clone(), vm)? .ok_or_else(|| { vm.new_type_error(format!("super argument {} was not supplied", first_arg)) diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index a47d8c69a..74991c69d 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -294,7 +294,7 @@ pub(crate) fn init(context: &PyContext) { PyTupleIterator::extend_class(context, &context.types.tuple_iterator_type); } -pub struct PyTupleTyped { +pub struct PyTupleTyped { // SAFETY INVARIANT: T must be repr(transparent) over PyObjectRef, and the // elements must be logically valid when transmuted to T tuple: PyTupleRef, @@ -315,9 +315,15 @@ impl TryFromObject for PyTupleTyped { } } -impl<'a, T: 'a> BorrowValue<'a> for PyTupleTyped { +impl<'a, T: TransmuteFromObject + 'a> BorrowValue<'a> for PyTupleTyped { type Borrowed = &'a [T]; fn borrow_value(&'a self) -> Self::Borrowed { unsafe { &*(self.tuple.borrow_value() as *const [PyObjectRef] as *const [T]) } } } + +impl fmt::Debug for PyTupleTyped { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.borrow_value().fmt(f) + } +} diff --git a/vm/src/frame.rs b/vm/src/frame.rs index fee9e595c..8c504d3ae 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -9,12 +9,13 @@ use crate::builtins::asyncgenerator::PyAsyncGenWrappedValue; use crate::builtins::code::PyCodeRef; use crate::builtins::coroutine::PyCoroutine; use crate::builtins::dict::{PyDict, PyDictRef}; +use crate::builtins::function::{PyCell, PyCellRef, PyFunction}; use crate::builtins::generator::PyGenerator; use crate::builtins::pystr::{self, PyStr, PyStrRef}; use crate::builtins::pytype::PyTypeRef; use crate::builtins::slice::PySlice; use crate::builtins::traceback::PyTraceback; -use crate::builtins::tuple::PyTuple; +use crate::builtins::tuple::{PyTuple, PyTupleTyped}; use crate::builtins::{list, pybool, set}; use crate::bytecode; use crate::common::lock::PyMutex; @@ -91,7 +92,12 @@ struct FrameState { #[pyclass(module = false, name = "frame")] pub struct Frame { pub code: PyCodeRef, - pub scope: Scope, + + pub fastlocals: PyMutex]>>, + pub cells_frees: Box<[PyCellRef]>, + pub locals: PyDictRef, + pub globals: PyDictRef, + /// index of last instruction ran pub lasti: AtomicUsize, /// tracer function for this frame (usually is None) @@ -151,7 +157,7 @@ impl ExecutionResult { pub type FrameResult = PyResult>; impl Frame { - pub fn new(code: PyCodeRef, scope: Scope, vm: &VirtualMachine) -> Frame { + pub fn new(code: PyCodeRef, scope: Scope, closure: &[PyCellRef], vm: &VirtualMachine) -> Frame { //populate the globals and locals //TODO: This is wrong, check https://github.com/nedbat/byterun/blob/31e6c4a8212c35b5157919abff43a7daa0f377c6/byterun/pyvm2.py#L95 /* @@ -162,10 +168,17 @@ impl Frame { */ // let locals = globals; // locals.extend(callargs); + let cells_frees = std::iter::repeat_with(|| PyCell::default().into_ref(vm)) + .take(code.cellvars.len()) + .chain(closure.iter().cloned()) + .collect(); Frame { + fastlocals: PyMutex::new(vec![None; code.varnames.len()].into_boxed_slice()), + cells_frees, + locals: scope.locals, + globals: scope.globals, code, - scope, lasti: AtomicUsize::new(0), state: PyMutex::new(FrameState { stack: Vec::new(), @@ -182,7 +195,10 @@ impl FrameRef { let mut state = self.state.lock(); let exec = ExecutingFrame { code: &self.code, - scope: &self.scope, + fastlocals: &self.fastlocals, + cells_frees: &self.cells_frees, + locals: &self.locals, + globals: &self.globals, lasti: &self.lasti, object: &self, state: &mut state, @@ -190,6 +206,10 @@ impl FrameRef { f(exec) } + pub fn locals(&self, vm: &VirtualMachine) -> &PyDictRef { + todo!() + } + // #[cfg_attr(feature = "flame-it", flame("Frame"))] pub fn run(&self, vm: &VirtualMachine) -> PyResult { self.with_exec(|mut exec| exec.run(vm)) @@ -233,7 +253,10 @@ impl FrameRef { /// with the mutable data inside struct ExecutingFrame<'a> { code: &'a PyCodeRef, - scope: &'a Scope, + fastlocals: &'a PyMutex]>>, + cells_frees: &'a [PyCellRef], + locals: &'a PyDictRef, + globals: &'a PyDictRef, object: &'a FrameRef, lasti: &'a AtomicUsize, state: &'a mut FrameState, @@ -243,7 +266,7 @@ impl fmt::Debug for ExecutingFrame<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("ExecutingFrame") .field("code", self.code) - .field("scope", self.scope) + // .field("scope", self.scope) .field("lasti", self.lasti) .field("state", self.state) .finish() @@ -330,6 +353,24 @@ impl ExecutingFrame<'_> { } } + fn unbound_cell_exception(&self, i: usize, vm: &VirtualMachine) -> PyBaseExceptionRef { + if let Some(name) = self.code.cellvars.get(i) { + vm.new_exception_msg( + vm.ctx.exceptions.unbound_local_error.clone(), + format!( + "local variable '{}' referenced before assignment", + self.code.cellvars[i] + ), + ) + } else { + let name = &self.code.freevars[i - self.code.cellvars.len()]; + vm.new_name_error(format!( + "free variable '{}' referenced before assignment in enclosing scope", + name + )) + } + } + /// Execute a single instruction. #[inline(always)] fn execute_instruction( @@ -366,9 +407,100 @@ impl ExecutingFrame<'_> { } => self.import(vm, *name_idx, symbols_idx, *level), bytecode::Instruction::ImportStar => self.import_star(vm), bytecode::Instruction::ImportFrom { idx } => self.import_from(vm, *idx), - bytecode::Instruction::LoadName { idx, scope } => self.load_name(vm, *idx, *scope), - bytecode::Instruction::StoreName { idx, scope } => self.store_name(vm, *idx, *scope), - bytecode::Instruction::DeleteName { idx } => self.delete_name(vm, *idx), + bytecode::Instruction::LoadFast(idx) => { + let x = self.fastlocals.lock()[*idx].clone().ok_or_else(|| { + vm.new_exception_msg( + vm.ctx.exceptions.unbound_local_error.clone(), + format!( + "local variable '{}' referenced before assignment", + self.code.varnames[*idx] + ), + ) + })?; + self.push_value(x); + Ok(None) + } + bytecode::Instruction::LoadLocal(idx) => { + let name = &self.code.names[*idx]; + let x = self + .locals + .get_item_option(name.clone(), vm)? + .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; + self.push_value(x); + Ok(None) + } + bytecode::Instruction::LoadGlobal(idx) => { + let name = &self.code.names[*idx]; + let x = self + .globals + .get_item_option(name.clone(), vm)? + .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; + self.push_value(x); + Ok(None) + } + bytecode::Instruction::LoadDeref(i) => { + let i = *i; + self.cells_frees[i] + .get() + .ok_or_else(|| self.unbound_cell_exception(i, vm))?; + Ok(None) + } + bytecode::Instruction::LoadClassDeref(i) => { + let i = *i; + let name = self.code.freevars[i - self.code.cellvars.len()].clone(); + let value = if let Some(value) = self.locals.get_item_option(name, vm)? { + value + } else { + self.cells_frees[i] + .get() + .ok_or_else(|| self.unbound_cell_exception(i, vm))? + }; + self.push_value(value); + Ok(None) + } + bytecode::Instruction::StoreFast(idx) => { + let value = self.pop_value(); + self.fastlocals.lock()[*idx] = Some(value); + Ok(None) + } + bytecode::Instruction::StoreLocal(idx) => { + let value = self.pop_value(); + self.locals + .set_item(self.code.names[*idx].clone(), value, vm)?; + Ok(None) + } + bytecode::Instruction::StoreGlobal(idx) => { + let value = self.pop_value(); + self.globals + .set_item(self.code.names[*idx].clone(), value, vm)?; + Ok(None) + } + bytecode::Instruction::StoreDeref(i) => { + let value = self.pop_value(); + self.cells_frees[*i].set(Some(value)); + Ok(None) + } + bytecode::Instruction::DeleteFast(idx) => { + self.fastlocals.lock()[*idx] = None; + Ok(None) + } + bytecode::Instruction::DeleteLocal(idx) => { + self.locals.del_item(self.code.names[*idx].clone(), vm)?; + Ok(None) + } + bytecode::Instruction::DeleteGlobal(idx) => { + self.globals.del_item(self.code.names[*idx].clone(), vm)?; + Ok(None) + } + bytecode::Instruction::DeleteDeref(i) => { + self.cells_frees[*i].set(None); + Ok(None) + } + bytecode::Instruction::LoadClosure(i) => { + let value = self.cells_frees[*i].clone(); + self.push_value(value.into_object()); + Ok(None) + } bytecode::Instruction::Subscript => self.execute_subscript(vm), bytecode::Instruction::StoreSubscript => self.execute_store_subscript(vm), bytecode::Instruction::DeleteSubscript => self.execute_delete_subscript(vm), @@ -472,9 +604,9 @@ impl ExecutingFrame<'_> { } bytecode::Instruction::YieldFrom => self.execute_yield_from(vm), bytecode::Instruction::SetupAnnotation => { - let locals = self.scope.get_locals(); - if !locals.contains_key("__annotations__", vm) { - locals.set_item("__annotations__", vm.ctx.new_dict().into_object(), vm)?; + if !self.locals.contains_key("__annotations__", vm) { + self.locals + .set_item("__annotations__", vm.ctx.new_dict().into_object(), vm)?; } Ok(None) } @@ -826,9 +958,8 @@ impl ExecutingFrame<'_> { }; for (k, v) in &dict { let k = PyStrRef::try_from_object(vm, k)?; - let k = k.as_ref(); - if filter_pred(k) { - self.scope.store_name(&vm, k, v); + if filter_pred(k.borrow_value()) { + self.locals.set_item(k, v, vm); } } } @@ -904,61 +1035,6 @@ impl ExecutingFrame<'_> { } } - fn store_name( - &mut self, - vm: &VirtualMachine, - idx: bytecode::NameIdx, - name_scope: bytecode::NameScope, - ) -> FrameResult { - let name = self.code.names[idx].clone(); - let obj = self.pop_value(); - match name_scope { - bytecode::NameScope::Global => { - self.scope.store_global(vm, name, obj); - } - bytecode::NameScope::NonLocal => { - self.scope.store_cell(vm, name, obj); - } - bytecode::NameScope::Local => { - self.scope.store_name(vm, name, obj); - } - bytecode::NameScope::Free => { - self.scope.store_name(vm, name, obj); - } - } - Ok(None) - } - - fn delete_name(&self, vm: &VirtualMachine, idx: bytecode::NameIdx) -> FrameResult { - let name = &self.code.names[idx]; - match self.scope.delete_name(vm, name.clone()) { - Ok(_) => Ok(None), - Err(_) => Err(vm.new_name_error(format!("name '{}' is not defined", name))), - } - } - - #[cfg_attr(feature = "flame-it", flame("Frame"))] - #[inline] - fn load_name( - &mut self, - vm: &VirtualMachine, - idx: bytecode::NameIdx, - name_scope: bytecode::NameScope, - ) -> FrameResult { - let name = &self.code.names[idx]; - let optional_value = match name_scope { - bytecode::NameScope::Global => self.scope.load_global(vm, name.clone()), - bytecode::NameScope::NonLocal => self.scope.load_cell(vm, name.clone()), - bytecode::NameScope::Local => self.scope.load_local(vm, name.clone()), - bytecode::NameScope::Free => self.scope.load_name(vm, name.clone()), - }; - - let value = optional_value - .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; - self.push_value(value); - Ok(None) - } - fn execute_rotate(&mut self, amount: usize) -> FrameResult { // Shuffles top of stack amount down if amount < 2 { @@ -1278,6 +1354,12 @@ impl ExecutingFrame<'_> { let flags = code_obj.flags; + let closure = if code_obj.freevars.is_empty() { + None + } else { + Some(PyTupleTyped::try_from_object(vm, self.pop_value()).unwrap()) + }; + let annotations = if flags.contains(bytecode::CodeFlags::HAS_ANNOTATIONS) { self.pop_value() } else { @@ -1306,10 +1388,15 @@ impl ExecutingFrame<'_> { // pop argc arguments // argument: name, args, globals - let scope = self.scope.clone(); - let func_obj = vm - .ctx - .new_pyfunction(code_obj, scope, defaults, kw_only_defaults); + // let scope = self.scope.clone(); + let func_obj = PyFunction::new( + code_obj, + self.globals.clone(), + closure, + defaults, + kw_only_defaults, + ) + .into_object(vm); vm.set_attr(&func_obj, "__doc__", vm.ctx.none())?; @@ -1320,7 +1407,7 @@ impl ExecutingFrame<'_> { .unwrap(); vm.set_attr(&func_obj, "__name__", vm.ctx.new_str(name))?; vm.set_attr(&func_obj, "__qualname__", qualified_name)?; - let module = vm.unwrap_or_none(self.scope.globals.get_item_option("__name__", vm)?); + let module = vm.unwrap_or_none(self.globals.get_item_option("__name__", vm)?); vm.set_attr(&func_obj, "__module__", module)?; vm.set_attr(&func_obj, "__annotations__", annotations)?; @@ -1546,7 +1633,8 @@ impl fmt::Debug for Frame { .iter() .map(|elem| format!("\n > {:?}", elem)) .collect::(); - let dict = self.scope.get_locals(); + // TODO: fix this up + let dict = self.locals.clone(); let local_str = dict .into_iter() .map(|elem| format!("\n {:?} = {:?}", elem.0, elem.1)) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 03dc97d71..90f4ca583 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -348,20 +348,6 @@ impl PyContext { PyRef::new_ref(code::PyCode::new(code), self.types.code_type.clone(), None) } - pub fn new_pyfunction( - &self, - code_obj: PyCodeRef, - scope: Scope, - defaults: Option, - kw_only_defaults: Option, - ) -> PyObjectRef { - PyObject::new( - PyFunction::new(code_obj, scope, defaults, kw_only_defaults), - self.types.function_type.clone(), - Some(self.new_dict()), - ) - } - pub fn new_bound_method(&self, function: PyObjectRef, object: PyObjectRef) -> PyObjectRef { PyObject::new( PyBoundMethod::new(object, function), @@ -637,8 +623,8 @@ where T: IntoPyObject + ?Sized, { fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult; - fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult; - fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult; + fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()>; + fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()>; } impl ItemProtocol for PyObjectRef @@ -649,12 +635,12 @@ where vm.call_method(self, "__getitem__", (key,)) } - fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { - vm.call_method(self, "__setitem__", (key, value)) + fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + vm.call_method(self, "__setitem__", (key, value)).map(drop) } - fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult { - vm.call_method(self, "__delitem__", (key,)) + fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()> { + vm.call_method(self, "__delitem__", (key,)).map(drop) } } diff --git a/vm/src/scope.rs b/vm/src/scope.rs index e77027926..ed6da1350 100644 --- a/vm/src/scope.rs +++ b/vm/src/scope.rs @@ -1,17 +1,13 @@ use std::fmt; use crate::builtins::{PyDictRef, PyStr, PyStrRef}; +use crate::common::lock::PyMutex; use crate::pyobject::{IntoPyObject, ItemProtocol, PyContext, PyObjectRef, PyResult, TryIntoRef}; use crate::VirtualMachine; -/* - * So a scope is a linked list of scopes. - * When a name is looked up, it is check in its scope. - */ -#[derive(Clone)] pub struct Scope { - locals: Vec, - pub globals: PyDictRef, + pub(crate) locals: PyDictRef, + pub(crate) globals: PyDictRef, } impl fmt::Debug for Scope { @@ -22,14 +18,10 @@ impl fmt::Debug for Scope { } impl Scope { - pub fn new(locals: Option, globals: PyDictRef, vm: &VirtualMachine) -> Scope { - let locals = match locals { - Some(dict) => vec![dict], - None => vec![], - }; - let scope = Scope { locals, globals }; - scope.store_name(vm, "__annotations__", vm.ctx.new_dict().into_object()); - scope + #[inline] + pub fn new(locals: Option, globals: PyDictRef) -> Scope { + let locals = locals.unwrap_or_else(|| globals.clone()); + Scope { locals, globals } } pub fn with_builtins( @@ -42,108 +34,108 @@ impl Scope { .set_item("__builtins__", vm.builtins.clone(), vm) .unwrap(); } - Scope::new(locals, globals, vm) + Scope::new(locals, globals) } - pub fn get_locals(&self) -> &PyDictRef { - match self.locals.first() { - Some(dict) => dict, - None => &self.globals, - } - } + // pub fn get_locals(&self) -> &PyDictRef { + // match self.locals.first() { + // Some(dict) => dict, + // None => &self.globals, + // } + // } - pub fn get_only_locals(&self) -> Option { - self.locals.first().cloned() - } + // pub fn get_only_locals(&self) -> Option { + // self.locals.first().cloned() + // } - pub fn new_child_scope_with_locals(&self, locals: PyDictRef) -> Scope { - let mut new_locals = Vec::with_capacity(self.locals.len() + 1); - new_locals.push(locals); - new_locals.extend_from_slice(&self.locals); - Scope { - locals: new_locals, - globals: self.globals.clone(), - } - } + // pub fn new_child_scope_with_locals(&self, locals: PyDictRef) -> Scope { + // let mut new_locals = Vec::with_capacity(self.locals.len() + 1); + // new_locals.push(locals); + // new_locals.extend_from_slice(&self.locals); + // Scope { + // locals: new_locals, + // globals: self.globals.clone(), + // } + // } - pub fn new_child_scope(&self, ctx: &PyContext) -> Scope { - self.new_child_scope_with_locals(ctx.new_dict()) - } + // pub fn new_child_scope(&self, ctx: &PyContext) -> Scope { + // self.new_child_scope_with_locals(ctx.new_dict()) + // } - #[cfg_attr(feature = "flame-it", flame("Scope"))] - pub fn load_name(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - for dict in self.locals.iter() { - if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { - return Some(value); - } - } + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // pub fn load_name(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // for dict in self.locals.iter() { + // if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { + // return Some(value); + // } + // } - // Fall back to loading a global after all scopes have been searched! - self.load_global(vm, name) - } + // // Fall back to loading a global after all scopes have been searched! + // self.load_global(vm, name) + // } - #[cfg_attr(feature = "flame-it", flame("Scope"))] - /// Load a local name. Only check the local dictionary for the given name. - pub fn load_local(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - self.get_locals().get_item_option(name, vm).unwrap() - } + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // /// Load a local name. Only check the local dictionary for the given name. + // pub fn load_local(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // self.get_locals().get_item_option(name, vm).unwrap() + // } - #[cfg_attr(feature = "flame-it", flame("Scope"))] - pub fn load_cell(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - for dict in self.locals.iter().skip(1) { - if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { - return Some(value); - } - } - None - } + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // pub fn load_cell(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // for dict in self.locals.iter().skip(1) { + // if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { + // return Some(value); + // } + // } + // None + // } - pub fn store_cell(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { - // find the innermost outer scope that contains the symbol name - if let Some(locals) = self - .locals - .iter() - .rev() - .find(|l| l.contains_key(name.clone(), vm)) - { - // add to the symbol - locals.set_item(name, value, vm).unwrap(); - } else { - // somewhat limited solution -> fallback to the old rustpython strategy - // and store the next outer scope - // This case is usually considered as a failure case, but kept for the moment - // to support the scope propagation for named expression assignments to so far - // unknown names in comprehensions. We need to consider here more context - // information for correct handling. - self.locals - .get(1) - .expect("no outer scope for non-local") - .set_item(name, value, vm) - .unwrap(); - } - } + // pub fn store_cell(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { + // // find the innermost outer scope that contains the symbol name + // if let Some(locals) = self + // .locals + // .iter() + // .rev() + // .find(|l| l.contains_key(name.clone(), vm)) + // { + // // add to the symbol + // locals.set_item(name, value, vm).unwrap(); + // } else { + // // somewhat limited solution -> fallback to the old rustpython strategy + // // and store the next outer scope + // // This case is usually considered as a failure case, but kept for the moment + // // to support the scope propagation for named expression assignments to so far + // // unknown names in comprehensions. We need to consider here more context + // // information for correct handling. + // self.locals + // .get(1) + // .expect("no outer scope for non-local") + // .set_item(name, value, vm) + // .unwrap(); + // } + // } - pub fn store_name(&self, vm: &VirtualMachine, key: impl PyName, value: PyObjectRef) { - self.get_locals().set_item(key, value, vm).unwrap(); - } + // pub fn store_name(&self, vm: &VirtualMachine, key: impl PyName, value: PyObjectRef) { + // self.get_locals().set_item(key, value, vm).unwrap(); + // } - pub fn delete_name(&self, vm: &VirtualMachine, key: impl PyName) -> PyResult { - self.get_locals().del_item(key, vm) - } + // pub fn delete_name(&self, vm: &VirtualMachine, key: impl PyName) -> PyResult { + // self.get_locals().del_item(key, vm) + // } - #[cfg_attr(feature = "flame-it", flame("Scope"))] - /// Load a global name. - pub fn load_global(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - if let Some(value) = self.globals.get_item_option(name.clone(), vm).unwrap() { - Some(value) - } else { - vm.get_attribute(vm.builtins.clone(), name).ok() - } - } + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // /// Load a global name. + // pub fn load_global(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // if let Some(value) = self.globals.get_item_option(name.clone(), vm).unwrap() { + // Some(value) + // } else { + // vm.get_attribute(vm.builtins.clone(), name).ok() + // } + // } - pub fn store_global(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { - self.globals.set_item(name, value, vm).unwrap(); - } + // pub fn store_global(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { + // self.globals.set_item(name, value, vm).unwrap(); + // } } mod sealed { diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index e645dc1de..b6825a6f3 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -205,7 +205,7 @@ mod decl { #[pymethod(name = "is_nonlocal")] fn is_nonlocal(&self) -> bool { - matches!(self.symbol.scope, SymbolScope::Nonlocal) + matches!(self.symbol.scope, SymbolScope::Free) } #[pymethod(name = "is_referenced")] @@ -225,7 +225,7 @@ mod decl { #[pymethod(name = "is_free")] fn is_free(&self) -> bool { - self.symbol.is_free + matches!(self.symbol.scope, SymbolScope::Free) } #[pymethod(name = "is_namespace")] diff --git a/vm/src/vm.rs b/vm/src/vm.rs index ce1baf5a8..8a114e976 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -448,7 +448,7 @@ impl VirtualMachine { } pub fn run_code_obj(&self, code: PyCodeRef, scope: Scope) -> PyResult { - let frame = Frame::new(code, scope, self).into_ref(self); + let frame = Frame::new(code, scope, &[], self).into_ref(self); self.run_frame_full(frame) } @@ -495,11 +495,18 @@ impl VirtualMachine { } } - pub fn current_scope(&self) -> Ref { + pub fn current_locals(&self) -> Ref { let frame = self .current_frame() - .expect("called current_scope but no frames on the stack"); - Ref::map(frame, |f| &f.scope) + .expect("called current_locals but no frames on the stack"); + Ref::map(frame, |f| f.locals(self)) + } + + pub fn current_globals(&self) -> Ref { + let frame = self + .current_frame() + .expect("called current_globals but no frames on the stack"); + Ref::map(frame, |f| &f.globals) } pub fn try_class(&self, module: &str, class: &str) -> PyResult { @@ -794,10 +801,6 @@ impl VirtualMachine { obj.unwrap_or_else(|| self.ctx.none()) } - pub fn get_locals(&self) -> PyDictRef { - self.current_scope().get_locals().clone() - } - // Container of the virtual machine state: pub fn to_str(&self, obj: &PyObjectRef) -> PyResult { if obj.class().is(&self.ctx.types.str_type) { @@ -872,12 +875,9 @@ impl VirtualMachine { })?; let (locals, globals) = if let Some(frame) = self.current_frame() { - ( - frame.scope.get_locals().clone().into_object(), - frame.scope.globals.clone().into_object(), - ) + (Some(frame.locals.clone()), Some(frame.globals.clone())) } else { - (self.ctx.none(), self.ctx.none()) + (None, None) }; let from_list = self .ctx