diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 371165739..07f191b09 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -32,6 +32,7 @@ impl Location { } pub trait Constant: Sized { + type Name: AsRef; fn borrow_constant(&self) -> BorrowedConstant; fn into_data(self) -> ConstantData { self.borrow_constant().into_data() @@ -41,6 +42,7 @@ pub trait Constant: Sized { } } impl Constant for ConstantData { + type Name = String; fn borrow_constant(&self) -> BorrowedConstant { use BorrowedConstant::*; match self { @@ -69,6 +71,10 @@ pub trait ConstantBag: Sized { fn make_constant_borrowed(&self, constant: BorrowedConstant) -> Self::Constant { self.make_constant(constant.into_data()) } + fn make_name(&self, name: String) -> ::Name; + fn make_name_ref(&self, name: &str) -> ::Name { + self.make_name(name.to_owned()) + } } #[derive(Clone)] @@ -78,8 +84,8 @@ impl ConstantBag for BasicBag { fn make_constant(&self, constant: ConstantData) -> Self::Constant { constant } - fn make_constant_borrowed(&self, constant: BorrowedConstant) -> Self::Constant { - constant.into_data() + fn make_name(&self, name: String) -> ::Name { + name } } @@ -93,14 +99,17 @@ pub struct CodeObject { pub locations: Vec, pub flags: CodeFlags, pub posonlyarg_count: usize, // Number of positional-only arguments - pub arg_names: Vec, // Names of positional arguments - pub varargs_name: Option, // *args or * - pub kwonlyarg_names: Vec, - pub varkeywords_name: Option, // **kwargs or ** + pub arg_count: usize, + pub kwonlyarg_count: usize, pub source_path: String, pub first_line_number: usize, pub obj_name: String, // Name of the object that created this code object pub constants: Vec, + #[serde(bound( + deserialize = "C::Name: serde::Deserialize<'de>", + serialize = "C::Name: serde::Serialize" + ))] + pub names: Vec, } bitflags! { @@ -172,37 +181,39 @@ pub enum ConversionFlag { Repr, } +pub type NameIdx = usize; + /// A Single bytecode instruction. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum Instruction { Import { - name: Option, - symbols: Vec, + name_idx: Option, + symbols_idx: Vec, level: usize, }, ImportStar, ImportFrom { - name: String, + idx: NameIdx, }, LoadName { - name: String, + idx: NameIdx, scope: NameScope, }, StoreName { - name: String, + idx: NameIdx, scope: NameScope, }, DeleteName { - name: String, + idx: NameIdx, }, Subscript, StoreSubscript, DeleteSubscript, StoreAttr { - name: String, + idx: NameIdx, }, DeleteAttr { - name: String, + idx: NameIdx, }, LoadConst { /// index into constants vec @@ -216,7 +227,7 @@ pub enum Instruction { inplace: bool, }, LoadAttr { - name: String, + idx: NameIdx, }, CompareOperation { op: ComparisonOperator, @@ -503,15 +514,37 @@ pub enum BlockType { } */ +pub struct Arguments<'a, N: AsRef> { + pub posonlyargs: &'a [N], + pub args: &'a [N], + pub vararg: Option<&'a N>, + pub kwonlyargs: &'a [N], + pub varkwarg: Option<&'a N>, +} + +impl> fmt::Debug for Arguments<'_, N> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + macro_rules! fmt_slice { + ($x:expr) => { + format_args!("[{}]", $x.iter().map(AsRef::as_ref).format(", ")) + }; + } + f.debug_struct("Arguments") + .field("posonlyargs", &fmt_slice!(self.posonlyargs)) + .field("args", &fmt_slice!(self.posonlyargs)) + .field("vararg", &self.vararg.map(N::as_ref)) + .field("kwonlyargs", &fmt_slice!(self.kwonlyargs)) + .field("varkwarg", &self.varkwarg.map(N::as_ref)) + .finish() + } +} + impl CodeObject { - #[allow(clippy::too_many_arguments)] pub fn new( flags: CodeFlags, posonlyarg_count: usize, - arg_names: Vec, - varargs_name: Option, - kwonlyarg_names: Vec, - varkeywords_name: Option, + arg_count: usize, + kwonlyarg_count: usize, source_path: String, first_line_number: usize, obj_name: String, @@ -522,40 +555,45 @@ impl CodeObject { locations: Vec::new(), flags, posonlyarg_count, - arg_names, - varargs_name, - kwonlyarg_names, - varkeywords_name, + arg_count, + kwonlyarg_count, source_path, first_line_number, obj_name, constants: Vec::new(), + names: Vec::new(), } } - pub fn varnames(&self) -> impl Iterator + '_ { - self.arg_names - .iter() - .map(String::as_str) - .chain(self.kwonlyarg_names.iter().map(String::as_str)) - .chain(self.varargs_name.as_deref()) - .chain(self.varkeywords_name.as_deref()) - .chain( - self.instructions - .iter() - .filter_map(|i| match i { - Instruction::LoadName { - name, - scope: NameScope::Local, - } - | Instruction::StoreName { - name, - scope: NameScope::Local, - } => Some(name.as_str()), - _ => None, - }) - .unique(), - ) + // like inspect.getargs + pub fn arg_names(&self) -> Arguments { + 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 vararg = if self.flags.contains(CodeFlags::HAS_VARARGS) { + let vararg = &self.names[varargspos]; + varargspos += 1; + Some(vararg) + } else { + None + }; + let varkwarg = if self.flags.contains(CodeFlags::HAS_VARKEYWORDS) { + Some(&self.names[varargspos]) + } else { + None + }; + + Arguments { + posonlyargs, + args, + vararg, + kwonlyargs, + varkwarg, + } } fn display_inner( @@ -579,6 +617,7 @@ impl CodeObject { f, &self.label_map, &self.constants, + &self.names, expand_codeobjects, level, )?; @@ -603,16 +642,19 @@ impl CodeObject { .into_iter() .map(|x| x.map_constant(bag)) .collect(), + names: self + .names + .into_iter() + .map(|x| bag.make_name_ref(x.as_ref())) + .collect(), instructions: self.instructions, label_map: self.label_map, locations: self.locations, flags: self.flags, posonlyarg_count: self.posonlyarg_count, - arg_names: self.arg_names, - varargs_name: self.varargs_name, - kwonlyarg_names: self.kwonlyarg_names, - varkeywords_name: self.varkeywords_name, + arg_count: self.arg_count, + kwonlyarg_count: self.kwonlyarg_count, source_path: self.source_path, first_line_number: self.first_line_number, obj_name: self.obj_name, @@ -626,16 +668,19 @@ impl CodeObject { .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(), instructions: self.instructions.clone(), label_map: self.label_map.clone(), locations: self.locations.clone(), flags: self.flags, posonlyarg_count: self.posonlyarg_count, - arg_names: self.arg_names.clone(), - varargs_name: self.varargs_name.clone(), - kwonlyarg_names: self.kwonlyarg_names.clone(), - varkeywords_name: self.varkeywords_name.clone(), + arg_count: self.arg_count, + kwonlyarg_count: self.kwonlyarg_count, source_path: self.source_path.clone(), first_line_number: self.first_line_number, obj_name: self.obj_name.clone(), @@ -677,6 +722,7 @@ impl Instruction { f: &mut fmt::Formatter, label_map: &BTreeMap, constants: &[C], + names: &[C::Name], expand_codeobjects: bool, level: usize, ) -> fmt::Result { @@ -704,25 +750,31 @@ impl Instruction { match self { Import { - name, - symbols, + name_idx, + symbols_idx, level, } => w!( Import, - format!("{:?}", name), - format!("{:?}", symbols), + format!("{:?}", name_idx.map(|idx| names[idx].as_ref())), + format!( + "({:?})", + symbols_idx + .iter() + .map(|&idx| names[idx].as_ref()) + .format(", ") + ), level ), ImportStar => w!(ImportStar), - ImportFrom { name } => w!(ImportFrom, name), - LoadName { name, scope } => w!(LoadName, name, format!("{:?}", scope)), - StoreName { name, scope } => w!(StoreName, name, format!("{:?}", scope)), - DeleteName { name } => w!(DeleteName, name), + 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()), Subscript => w!(Subscript), StoreSubscript => w!(StoreSubscript), DeleteSubscript => w!(DeleteSubscript), - StoreAttr { name } => w!(StoreAttr, name), - DeleteAttr { name } => w!(DeleteAttr, name), + StoreAttr { idx } => w!(StoreAttr, names[*idx].as_ref()), + DeleteAttr { idx } => w!(DeleteAttr, names[*idx].as_ref()), LoadConst { idx } => { let value = &constants[*idx]; match value.borrow_constant() { @@ -740,7 +792,7 @@ impl Instruction { } UnaryOperation { op } => w!(UnaryOperation, format!("{:?}", op)), BinaryOperation { op, inplace } => w!(BinaryOperation, format!("{:?}", op), inplace), - LoadAttr { name } => w!(LoadAttr, name), + LoadAttr { idx } => w!(LoadAttr, names[*idx].as_ref()), CompareOperation { op } => w!(CompareOperation, format!("{:?}", op)), Pop => w!(Pop), Rotate { amount } => w!(Rotate, amount), @@ -818,6 +870,10 @@ impl fmt::Debug for CodeObject { #[derive(Serialize, Deserialize)] pub struct FrozenModule { + #[serde(bound( + deserialize = "C: serde::Deserialize<'de>, C::Name: serde::Deserialize<'de>", + serialize = "C: serde::Serialize, C::Name: serde::Serialize" + ))] pub code: CodeObject, pub package: bool, } diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 6c371af56..7a6bf0f81 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -10,6 +10,7 @@ pub use crate::mode::Mode; use crate::symboltable::{ make_symbol_table, statements_to_symbol_table, Symbol, SymbolScope, SymbolTable, }; +use indexmap::IndexSet; use itertools::Itertools; use num_complex::Complex64; use rustpython_ast as ast; @@ -19,7 +20,7 @@ type CompileResult = Result; /// Main structure holding the state of compilation. struct Compiler { - output_stack: Vec, + output_stack: Vec<(CodeObject, IndexSet)>, symbol_table_stack: Vec, nxt_label: usize, source_path: String, @@ -150,7 +151,7 @@ impl Compiler { } fn push_output(&mut self, code: CodeObject) { - self.output_stack.push(code); + self.output_stack.push((code, IndexSet::new())); } fn push_new_code_object(&mut self, obj_name: String) { @@ -158,10 +159,8 @@ impl Compiler { self.push_output(CodeObject::new( Default::default(), 0, - Vec::new(), - None, - Vec::new(), - None, + 0, + 0, self.source_path.clone(), line_number, obj_name, @@ -169,7 +168,20 @@ impl Compiler { } fn pop_code_object(&mut self) -> CodeObject { - self.output_stack.pop().unwrap() + let (mut code, names) = self.output_stack.pop().unwrap(); + code.names.extend(names); + 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.output_stack.last_mut().expect("nothing on stack").1; + if let Some(x) = cache.get_index_of(name) { + x + } else { + cache.insert_full(name.to_owned()).0 + } } fn compile_program( @@ -183,8 +195,9 @@ impl Compiler { let (statements, doc) = get_doc(&program.statements); if let Some(value) = doc { self.emit_constant(bytecode::ConstantData::Str { value }); + let doc = self.name("__doc__"); self.emit(Instruction::StoreName { - name: "__doc__".to_owned(), + idx: doc, scope: bytecode::NameScope::Global, }); } @@ -279,18 +292,14 @@ impl Compiler { fn load_name(&mut self, name: &str) { let scope = self.scope_for_name(name); - self.emit(Instruction::LoadName { - name: name.to_owned(), - scope, - }); + let idx = self.name(name); + self.emit(Instruction::LoadName { idx, scope }); } fn store_name(&mut self, name: &str) { let scope = self.scope_for_name(name); - self.emit(Instruction::StoreName { - name: name.to_owned(), - scope, - }); + let idx = self.name(name); + self.emit(Instruction::StoreName { idx, scope }); } fn compile_statement(&mut self, statement: &ast::Statement) -> CompileResult<()> { @@ -312,16 +321,16 @@ impl Compiler { Import { names } => { // import a, b, c as d for name in names { + let name_idx = Some(self.name(&name.symbol)); self.emit(Instruction::Import { - name: Some(name.symbol.clone()), - symbols: vec![], + name_idx, + symbols_idx: vec![], level: 0, }); if let Some(alias) = &name.alias { for part in name.symbol.split('.').skip(1) { - self.emit(Instruction::LoadAttr { - name: part.to_owned(), - }); + let idx = self.name(part); + self.emit(Instruction::LoadAttr { idx }); } self.store_name(alias); } else { @@ -336,31 +345,33 @@ impl Compiler { } => { let import_star = names.iter().any(|n| n.symbol == "*"); + let module_idx = module.as_ref().map(|s| self.name(s)); + if import_star { + let star = self.name("*"); // from .... import * self.emit(Instruction::Import { - name: module.clone(), - symbols: vec!["*".to_owned()], + name_idx: module_idx, + symbols_idx: vec![star], level: *level, }); self.emit(Instruction::ImportStar); } else { // from mod import a, b as c // First, determine the fromlist (for import lib): - let from_list = names.iter().map(|n| n.symbol.clone()).collect(); + let from_list = names.iter().map(|n| self.name(&n.symbol)).collect(); // Load module once: self.emit(Instruction::Import { - name: module.clone(), - symbols: from_list, + name_idx: module_idx, + symbols_idx: from_list, level: *level, }); for name in names { + let idx = self.name(&name.symbol); // import symbol from module: - self.emit(Instruction::ImportFrom { - name: name.symbol.to_owned(), - }); + self.emit(Instruction::ImportFrom { idx }); // Store module under proper name: if let Some(alias) = &name.alias { @@ -513,8 +524,9 @@ impl Compiler { if self.opts.optimize == 0 { let end_label = self.new_label(); self.compile_jump_if(test, true, end_label)?; + let assertion_error = self.name("AssertionError"); self.emit(Instruction::LoadName { - name: String::from("AssertionError"), + idx: assertion_error, scope: bytecode::NameScope::Global, }); match msg { @@ -612,15 +624,13 @@ impl Compiler { fn compile_delete(&mut self, expression: &ast::Expression) -> CompileResult<()> { match &expression.node { ast::ExpressionType::Identifier { name } => { - self.emit(Instruction::DeleteName { - name: name.to_owned(), - }); + let idx = self.name(name); + self.emit(Instruction::DeleteName { idx }); } ast::ExpressionType::Attribute { value, name } => { self.compile_expression(value)?; - self.emit(Instruction::DeleteAttr { - name: name.to_owned(), - }); + let idx = self.name(name); + self.emit(Instruction::DeleteAttr { idx }); } ast::ExpressionType::Subscript { a, b } => { self.compile_expression(a)?; @@ -677,33 +687,35 @@ impl Compiler { flags |= bytecode::CodeFlags::HAS_KW_ONLY_DEFAULTS; } - let mut compile_varargs = |va: &ast::Varargs, flag| match va { - ast::Varargs::None => None, - ast::Varargs::Unnamed => { - flags |= flag; - None - } - ast::Varargs::Named(name) => { - flags |= flag; - Some(name.arg.clone()) - } - }; - - let varargs_name = compile_varargs(&args.vararg, bytecode::CodeFlags::HAS_VARARGS); - let varkeywords_name = compile_varargs(&args.kwarg, bytecode::CodeFlags::HAS_VARKEYWORDS); - let line_number = self.get_source_line_number(); self.push_output(CodeObject::new( flags, args.posonlyargs_count, - args.args.iter().map(|a| a.arg.clone()).collect(), - varargs_name, - args.kwonlyargs.iter().map(|a| a.arg.clone()).collect(), - varkeywords_name, + args.args.len(), + args.kwonlyargs.len(), self.source_path.clone(), line_number, name.to_owned(), )); + + for name in &args.args { + self.name(&name.arg); + } + for name in &args.kwonlyargs { + self.name(&name.arg); + } + + let mut compile_varargs = |va: &ast::Varargs, flag| match va { + ast::Varargs::None | ast::Varargs::Unnamed => {} + ast::Varargs::Named(name) => { + self.current_code().flags |= flag; + self.name(&name.arg); + } + }; + + compile_varargs(&args.vararg, bytecode::CodeFlags::HAS_VARARGS); + compile_varargs(&args.kwarg, bytecode::CodeFlags::HAS_VARKEYWORDS); + self.enter_scope(); Ok(()) @@ -948,9 +960,8 @@ impl Compiler { self.emit(Instruction::Duplicate); self.load_docstring(doc_str); self.emit(Instruction::Rotate { amount: 2 }); - self.emit(Instruction::StoreAttr { - name: "__doc__".to_owned(), - }); + let doc = self.name("__doc__"); + self.emit(Instruction::StoreAttr { idx: doc }); self.apply_decorators(decorator_list); self.store_name(name); @@ -1041,10 +1052,8 @@ impl Compiler { self.push_output(CodeObject::new( Default::default(), 0, - vec![], - None, - vec![], - None, + 0, + 0, self.source_path.clone(), line_number, name.to_owned(), @@ -1053,24 +1062,28 @@ impl Compiler { let (new_body, doc_str) = get_doc(body); + let dunder_name = self.name("__name__"); self.emit(Instruction::LoadName { - name: "__name__".to_owned(), + idx: dunder_name, scope: bytecode::NameScope::Global, }); + let dunder_module = self.name("__module__"); self.emit(Instruction::StoreName { - name: "__module__".to_owned(), + idx: dunder_module, scope: bytecode::NameScope::Free, }); self.emit_constant(bytecode::ConstantData::Str { value: qualified_name.clone(), }); + let qualname = self.name("__qualname__"); self.emit(Instruction::StoreName { - name: "__qualname__".to_owned(), + idx: qualname, scope: bytecode::NameScope::Free, }); self.load_docstring(doc_str); + let doc = self.name("__doc__"); self.emit(Instruction::StoreName { - name: "__doc__".to_owned(), + idx: doc, scope: bytecode::NameScope::Free, }); // setup annotations @@ -1222,8 +1235,9 @@ impl Compiler { self.set_label(check_asynciter_label); self.emit(Instruction::Duplicate); + let stopasynciter = self.name("StopAsyncIteration"); self.emit(Instruction::LoadName { - name: "StopAsyncIteration".to_owned(), + idx: stopasynciter, scope: bytecode::NameScope::Global, }); self.emit(Instruction::CompareOperation { @@ -1363,8 +1377,9 @@ 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 { - name: String::from("__annotations__"), + idx: annotations, scope: bytecode::NameScope::Local, }); self.emit_constant(bytecode::ConstantData::Str { @@ -1390,9 +1405,8 @@ impl Compiler { } ast::ExpressionType::Attribute { value, name } => { self.compile_expression(value)?; - self.emit(Instruction::StoreAttr { - name: name.to_owned(), - }); + let idx = self.name(name); + self.emit(Instruction::StoreAttr { idx }); } ast::ExpressionType::List { elements } | ast::ExpressionType::Tuple { elements } => { let mut seen_star = false; @@ -1658,9 +1672,8 @@ impl Compiler { } Attribute { value, name } => { self.compile_expression(value)?; - self.emit(Instruction::LoadAttr { - name: name.to_owned(), - }); + let idx = self.name(name); + self.emit(Instruction::LoadAttr { idx }); } Compare { vals, ops } => { self.compile_chained_comparison(vals, ops)?; @@ -1966,14 +1979,13 @@ impl Compiler { self.push_output(CodeObject::new( Default::default(), 1, - vec![".0".to_owned()], - None, - vec![], - None, + 1, + 0, self.source_path.clone(), line_number, name.clone(), )); + let arg0 = self.name(".0"); self.enter_scope(); // Create empty object of proper type: @@ -2009,7 +2021,7 @@ impl Compiler { if loop_labels.is_empty() { // Load iterator onto stack (passed as first argument): self.emit(Instruction::LoadName { - name: String::from(".0"), + idx: arg0, scope: bytecode::NameScope::Local, }); } else { @@ -2228,9 +2240,11 @@ impl Compiler { } fn current_code(&mut self) -> &mut CodeObject { - self.output_stack + &mut self + .output_stack .last_mut() .expect("No OutputStream on stack") + .0 } // Generate a new label diff --git a/derive/Cargo.toml b/derive/Cargo.toml index 0aea9d660..afd8c7d87 100644 --- a/derive/Cargo.toml +++ b/derive/Cargo.toml @@ -11,7 +11,7 @@ edition = "2018" proc-macro = true [dependencies] -syn = { version = "1.0", features = ["full"] } +syn = { version = "1.0", features = ["full", "extra-traits"] } syn-ext = { version = "0.2.1", features = ["full"] } quote = "1.0" proc-macro2 = "1.0" diff --git a/jit/src/instructions.rs b/jit/src/instructions.rs index a698fe2f6..584970298 100644 --- a/jit/src/instructions.rs +++ b/jit/src/instructions.rs @@ -34,12 +34,17 @@ pub struct FunctionCompiler<'a, 'b> { } impl<'a, 'b> FunctionCompiler<'a, 'b> { - pub fn new( + pub fn new( builder: &'a mut FunctionBuilder<'b>, - arg_names: &[String], + arg_names: I, arg_types: &[JitType], entry_block: Block, - ) -> FunctionCompiler<'a, 'b> { + ) -> FunctionCompiler<'a, 'b> + where + I: IntoIterator, + I::IntoIter: ExactSizeIterator, + S: AsRef, + { let mut compiler = FunctionCompiler { builder, stack: Vec::new(), @@ -51,11 +56,12 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> { }, }; let params = compiler.builder.func.dfg.block_params(entry_block).to_vec(); + let arg_names = arg_names.into_iter(); debug_assert_eq!(arg_names.len(), arg_types.len()); debug_assert_eq!(arg_names.len(), params.len()); - for ((name, ty), val) in arg_names.iter().zip(arg_types).zip(params) { + for ((name, ty), val) in arg_names.zip(arg_types).zip(params) { compiler - .store_variable(name.clone(), JitValue::new(val, ty.clone())) + .store_variable(name.as_ref().to_owned(), JitValue::new(val, ty.clone())) .unwrap(); } compiler @@ -131,7 +137,7 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> { continue; } - self.add_instruction(&instruction, &bytecode.constants)?; + self.add_instruction(&instruction, &bytecode.constants, &bytecode.names)?; } Ok(()) @@ -177,6 +183,7 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> { &mut self, instruction: &Instruction, constants: &[C], + names: &[C::Name], ) -> Result<(), JitCompileError> { match instruction { Instruction::JumpIfFalse { target } => { @@ -212,12 +219,12 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> { Ok(()) } Instruction::LoadName { - name, + idx, scope: NameScope::Local, } => { let local = self .variables - .get(name) + .get(names[*idx].as_ref()) .ok_or(JitCompileError::BadBytecode)?; self.stack.push(JitValue { val: self.builder.use_var(local.var), @@ -226,11 +233,11 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> { Ok(()) } Instruction::StoreName { - name, + idx, scope: NameScope::Local, } => { let val = self.stack.pop().ok_or(JitCompileError::BadBytecode)?; - self.store_variable(name.clone(), val) + self.store_variable(names[*idx].as_ref().to_owned(), val) } Instruction::LoadConst { idx } => self.load_const(constants[*idx].borrow_constant()), Instruction::ReturnValue => { diff --git a/jit/src/lib.rs b/jit/src/lib.rs index 756b715c9..31803223e 100644 --- a/jit/src/lib.rs +++ b/jit/src/lib.rs @@ -65,9 +65,8 @@ impl Jit { builder.switch_to_block(entry_block); let sig = { - let mut arg_names = bytecode.arg_names.clone(); - arg_names.extend(bytecode.kwonlyarg_names.iter().cloned()); - let mut compiler = FunctionCompiler::new(&mut builder, &arg_names, args, entry_block); + let arg_names = &bytecode.names[..bytecode.arg_count + bytecode.kwonlyarg_count]; + let mut compiler = FunctionCompiler::new(&mut builder, arg_names, args, entry_block); compiler.compile(bytecode)?; diff --git a/jit/tests/common.rs b/jit/tests/common.rs index a8e0955df..13483c262 100644 --- a/jit/tests/common.rs +++ b/jit/tests/common.rs @@ -13,7 +13,7 @@ pub struct Function { impl Function { pub fn compile(self) -> CompiledCode { let mut arg_types = Vec::new(); - for arg in self.code.arg_names.iter() { + for arg in self.code.arg_names().args { let arg_type = match self.annotations.get(arg) { Some(StackValue::String(annotation)) => match annotation.as_str() { "int" => JitType::Int, @@ -65,7 +65,7 @@ impl StackMachine { pub fn run(&mut self, code: CodeObject) { for instruction in code.instructions { - if self.process_instruction(instruction, &code.constants) { + if self.process_instruction(instruction, &code.constants, &code.names) { break; } } @@ -75,15 +75,17 @@ impl StackMachine { &mut self, instruction: Instruction, constants: &[ConstantData], + names: &[String], ) -> bool { match instruction { Instruction::LoadConst { idx } => self.stack.push(constants[idx].clone().into()), Instruction::LoadName { - name, + idx, scope: NameScope::Free, - } => self.stack.push(StackValue::String(name)), - Instruction::StoreName { name, .. } => { - self.locals.insert(name, self.stack.pop().unwrap()); + } => self.stack.push(StackValue::String(names[idx].clone())), + Instruction::StoreName { idx, .. } => { + self.locals + .insert(names[idx].clone(), self.stack.pop().unwrap()); } Instruction::StoreAttr { .. } => { // Do nothing except throw away the stack values diff --git a/src/shell/helper.rs b/src/shell/helper.rs index 7439c861c..c639a004d 100644 --- a/src/shell/helper.rs +++ b/src/shell/helper.rs @@ -1,6 +1,6 @@ use rustpython_vm::builtins::PyStrRef; use rustpython_vm::pyobject::{BorrowValue, PyIterable, PyResult, TryFromObject}; -use rustpython_vm::scope::{NameProtocol, Scope}; +use rustpython_vm::scope::Scope; use rustpython_vm::VirtualMachine; pub struct ShellHelper<'vm> { @@ -74,7 +74,7 @@ impl<'vm> ShellHelper<'vm> { // last: the last word, could be empty if it ends with a dot // parents: the words before the dot - let mut current = self.scope.load_global(self.vm, first)?; + let mut current = self.scope.load_global(self.vm, first.as_str())?; for attr in parents { current = self.vm.get_attribute(current.clone(), attr.as_str()).ok()?; diff --git a/vm/src/builtins/code.rs b/vm/src/builtins/code.rs index 0e4c3af4f..3b3d25ae3 100644 --- a/vm/src/builtins/code.rs +++ b/vm/src/builtins/code.rs @@ -5,7 +5,7 @@ use std::fmt; use std::ops::Deref; -use super::pytype::PyTypeRef; +use super::{PyStrRef, PyTypeRef}; use crate::bytecode::{self, BorrowedConstant, Constant, ConstantBag}; use crate::pyobject::{ BorrowValue, IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, @@ -66,6 +66,7 @@ fn borrow_obj_constant(obj: &PyObjectRef) -> BorrowedConstant { } impl Constant for PyConstant { + type Name = PyStrRef; fn borrow_constant(&self) -> BorrowedConstant { borrow_obj_constant(&self.0) } @@ -85,7 +86,10 @@ impl ConstantBag for PyObjBag<'_> { bytecode::ConstantData::Integer { value } => ctx.new_int(value), bytecode::ConstantData::Float { value } => ctx.new_float(value), bytecode::ConstantData::Complex { value } => ctx.new_complex(value), - bytecode::ConstantData::Str { value } => vm.intern_string(value).into_object(), + bytecode::ConstantData::Str { value } if value.len() <= 20 => { + vm.intern_string(value).into_object() + } + bytecode::ConstantData::Str { value } => vm.ctx.new_str(value), bytecode::ConstantData::Bytes { value } => ctx.new_bytes(value.to_vec()), bytecode::ConstantData::Boolean { value } => ctx.new_bool(value), bytecode::ConstantData::Code { code } => { @@ -110,7 +114,10 @@ impl ConstantBag for PyObjBag<'_> { bytecode::BorrowedConstant::Integer { value } => ctx.new_bigint(value), bytecode::BorrowedConstant::Float { value } => ctx.new_float(value), bytecode::BorrowedConstant::Complex { value } => ctx.new_complex(value), - bytecode::BorrowedConstant::Str { value } => vm.intern_string(value).into_object(), + bytecode::BorrowedConstant::Str { value } if value.len() <= 20 => { + vm.intern_string(value).into_object() + } + bytecode::BorrowedConstant::Str { value } => vm.ctx.new_str(value), bytecode::BorrowedConstant::Bytes { value } => ctx.new_bytes(value.to_vec()), bytecode::BorrowedConstant::Boolean { value } => ctx.new_bool(value), bytecode::BorrowedConstant::Code { code } => { @@ -128,6 +135,12 @@ impl ConstantBag for PyObjBag<'_> { }; PyConstant(obj) } + fn make_name(&self, name: String) -> PyStrRef { + self.0.intern_string(name) + } + fn make_name_ref(&self, name: &str) -> PyStrRef { + self.0.intern_string(name) + } } pub type PyCodeRef = PyRef; @@ -208,7 +221,7 @@ impl PyCodeRef { #[pyproperty] fn co_argcount(self) -> usize { - self.code.arg_names.len() + self.code.arg_count } #[pyproperty] @@ -223,7 +236,7 @@ impl PyCodeRef { #[pyproperty] fn co_kwonlyargcount(self) -> usize { - self.code.kwonlyarg_names.len() + self.code.kwonlyarg_count } #[pyproperty] @@ -244,7 +257,12 @@ impl PyCodeRef { #[pyproperty] fn co_varnames(self, vm: &VirtualMachine) -> PyObjectRef { - let varnames = self.code.varnames().map(|s| vm.ctx.new_str(s)).collect(); + let varnames = self + .code + .names + .iter() + .map(|s| s.clone().into_object()) + .collect(); vm.ctx.new_tuple(varnames) } } diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index 1edc2a51a..595fe63e0 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -1,7 +1,7 @@ #[cfg(feature = "jit")] mod jitfunc; -use super::code::{CodeObject, PyCodeRef}; +use super::code::PyCodeRef; use super::dict::PyDictRef; use super::pystr::PyStrRef; use super::pytype::PyTypeRef; @@ -63,14 +63,13 @@ impl PyFunction { fn fill_locals_from_args( &self, - code_object: &CodeObject, locals: &PyDictRef, func_args: FuncArgs, vm: &VirtualMachine, ) -> PyResult<()> { let nargs = func_args.args.len(); - let nexpected_args = code_object.arg_names.len(); - let posonly_args = &code_object.arg_names[..code_object.posonlyarg_count]; + let nexpected_args = self.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 @@ -84,23 +83,17 @@ impl PyFunction { nargs }; + let mut args_iter = func_args.args.into_iter(); + // Copy positional arguments into local variables - for i in 0..n { - let arg_name = &code_object.arg_names[i]; - let arg = &func_args.args[i]; - locals.set_item(arg_name.as_str(), arg.clone(), vm)?; + for (arg, arg_name) in args_iter.by_ref().take(n).zip(arg_names.args) { + locals.set_item(arg_name.clone(), arg, vm)?; } // Pack other positional arguments in to *args: - if let Some(ref vararg_name) = code_object.varargs_name { - let mut last_args = vec![]; - for i in n..nargs { - let arg = &func_args.args[i]; - last_args.push(arg.clone()); - } - let vararg_value = vm.ctx.new_tuple(last_args); - - locals.set_item(vararg_name.as_str(), vararg_value, vm)?; + if let Some(vararg_name) = arg_names.vararg { + let vararg_value = vm.ctx.new_tuple(args_iter.collect()); + locals.set_item(vararg_name.clone(), vararg_value, vm)?; } else { // Check the number of positional arguments if nargs > nexpected_args { @@ -112,27 +105,25 @@ impl PyFunction { } // Do we support `**kwargs` ? - let kwargs = if code_object - .flags - .contains(bytecode::CodeFlags::HAS_VARKEYWORDS) - { + let kwargs = if let Some(varkwarg) = arg_names.varkwarg { let d = vm.ctx.new_dict(); - if let Some(ref kwargs_name) = code_object.varkeywords_name { - locals.set_item(kwargs_name.as_str(), d.as_object().clone(), vm)?; - } + locals.set_item(varkwarg.clone(), d.as_object().clone(), vm)?; Some(d) } else { None }; + let contains_arg = + |names: &[PyStrRef], name: &str| names.iter().any(|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 code_object.arg_names.contains(&name) - || code_object.kwonlyarg_names.contains(&name) + let dict = if contains_arg(arg_names.args, &name) + || contains_arg(arg_names.kwonlyargs, &name) { - if posonly_args.contains(&name) { + if contains_arg(arg_names.posonlyargs, &name) { posonly_passed_as_kwarg.push(name); continue; } else if locals.contains_key(&name, vm) { @@ -146,12 +137,12 @@ impl PyFunction { vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name)) })? }; - dict.set_item(name.as_str(), 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!( "{}() got some positional-only arguments passed as keyword arguments: '{}'", - &code_object.obj_name, + &self.code.obj_name, posonly_passed_as_kwarg.into_iter().format(", "), ))); } @@ -167,8 +158,8 @@ impl PyFunction { let required_args = nexpected_args - num_defaults_available; let mut missing = vec![]; for i in 0..required_args { - let variable_name = &code_object.arg_names[i]; - if !locals.contains_key(variable_name, vm) { + let variable_name = &arg_names.args[i]; + if !locals.contains_key(variable_name.clone(), vm) { missing.push(variable_name) } } @@ -184,22 +175,20 @@ impl PyFunction { // We have sufficient defaults, so iterate over the corresponding names and use // the default if we don't already have a value for (default_index, i) in (required_args..nexpected_args).enumerate() { - let arg_name = &code_object.arg_names[i]; - if !locals.contains_key(arg_name, vm) { - locals.set_item(arg_name.as_str(), defaults[default_index].clone(), vm)?; + let arg_name = &arg_names.args[i]; + if !locals.contains_key(arg_name.clone(), vm) { + locals.set_item(arg_name.clone(), defaults[default_index].clone(), vm)?; } } } }; // Check if kw only arguments are all present: - for arg_name in &code_object.kwonlyarg_names { - if !locals.contains_key(arg_name, vm) { + for arg_name in arg_names.kwonlyargs { + if !locals.contains_key(arg_name.clone(), vm) { if let Some(kw_only_defaults) = &self.kw_only_defaults { - if let Some(default) = - kw_only_defaults.get_item_option(arg_name.as_str(), vm)? - { - locals.set_item(arg_name.as_str(), default, vm)?; + if let Some(default) = kw_only_defaults.get_item_option(arg_name.clone(), vm)? { + locals.set_item(arg_name.clone(), default, vm)?; continue; } } @@ -242,7 +231,7 @@ impl PyFunction { scope.clone() }; - self.fill_locals_from_args(&code, &scope.get_locals(), func_args, vm)?; + self.fill_locals_from_args(&scope.get_locals(), func_args, vm)?; // Construct frame: let frame = Frame::new(code.clone(), scope, vm).into_ref(vm); diff --git a/vm/src/builtins/function/jitfunc.rs b/vm/src/builtins/function/jitfunc.rs index 5c9a5854b..b68cc3baf 100644 --- a/vm/src/builtins/function/jitfunc.rs +++ b/vm/src/builtins/function/jitfunc.rs @@ -1,6 +1,7 @@ use crate::builtins::dict::PyDictRef; use crate::builtins::function::{PyFunction, PyFunctionRef}; -use crate::builtins::{float, int, pybool}; +use crate::builtins::{float, int, pybool, PyStrRef}; +use crate::bytecode::CodeFlags; use crate::exceptions::PyBaseExceptionRef; use crate::function::FuncArgs; use crate::pyobject::{ @@ -9,7 +10,6 @@ use crate::pyobject::{ }; use crate::VirtualMachine; use num_traits::ToPrimitive; -use rustpython_bytecode::bytecode::CodeFlags; use rustpython_jit::{AbiValue, Args, CompiledCode, JitArgumentError, JitType}; #[derive(Debug, thiserror::Error)] @@ -68,6 +68,8 @@ fn get_jit_arg_type(dict: &PyDictRef, name: &str, vm: &VirtualMachine) -> PyResu } pub fn get_jit_arg_types(func: &PyFunctionRef, vm: &VirtualMachine) -> PyResult> { + let arg_names = func.code.arg_names(); + if func .code .flags @@ -79,7 +81,7 @@ pub fn get_jit_arg_types(func: &PyFunctionRef, vm: &VirtualMachine) -> PyResult< )); } - if func.code.arg_names.is_empty() && func.code.kwonlyarg_names.is_empty() { + if arg_names.args.is_empty() && arg_names.kwonlyargs.is_empty() { return Ok(Vec::new()); } @@ -92,12 +94,12 @@ pub fn get_jit_arg_types(func: &PyFunctionRef, vm: &VirtualMachine) -> PyResult< } else if let Ok(dict) = PyDictRef::try_from_object(vm, annotations) { let mut arg_types = Vec::new(); - for arg in &func.code.arg_names { - arg_types.push(get_jit_arg_type(&dict, arg, vm)?); + for arg in arg_names.args { + arg_types.push(get_jit_arg_type(&dict, arg.borrow_value(), vm)?); } - for arg in &func.code.kwonlyarg_names { - arg_types.push(get_jit_arg_type(&dict, arg, vm)?); + for arg in arg_names.kwonlyargs { + arg_types.push(get_jit_arg_type(&dict, arg.borrow_value(), vm)?); } Ok(arg_types) @@ -136,8 +138,9 @@ pub(crate) fn get_jit_args<'a>( ) -> Result, ArgsError> { let mut jit_args = jitted_code.args_builder(); let nargs = func_args.args.len(); + let arg_names = func.code.arg_names(); - if nargs > func.code.arg_names.len() || nargs < func.code.posonlyarg_count { + if nargs > func.code.arg_count || nargs < func.code.posonlyarg_count { return Err(ArgsError::WrongNumberOfArgs); } @@ -148,14 +151,15 @@ pub(crate) fn get_jit_args<'a>( // Handle keyword arguments for (name, value) in &func_args.kwargs { - if let Some(arg_idx) = func.code.arg_names.iter().position(|arg| arg == name) { + let arg_pos = + |args: &[PyStrRef], name: &str| args.iter().position(|arg| arg.borrow_value() == name); + if let Some(arg_idx) = arg_pos(arg_names.args, name) { if jit_args.is_set(arg_idx) { return Err(ArgsError::ArgPassedMultipleTimes); } jit_args.set(arg_idx, get_jit_value(vm, &value)?)?; - } else if let Some(kwarg_idx) = func.code.kwonlyarg_names.iter().position(|arg| arg == name) - { - let arg_idx = kwarg_idx + func.code.arg_names.len(); + } else if let Some(kwarg_idx) = arg_pos(arg_names.kwonlyargs, name) { + let arg_idx = kwarg_idx + func.code.arg_count; if jit_args.is_set(arg_idx) { return Err(ArgsError::ArgPassedMultipleTimes); } @@ -169,7 +173,7 @@ pub(crate) fn get_jit_args<'a>( if let Some(defaults) = &func.defaults { let defaults = defaults.borrow_value(); for (i, default) in defaults.iter().enumerate() { - let arg_idx = i + func.code.arg_names.len() - defaults.len(); + let arg_idx = i + func.code.arg_count - defaults.len(); if !jit_args.is_set(arg_idx) { jit_args.set(arg_idx, get_jit_value(vm, default)?)?; } @@ -178,11 +182,11 @@ pub(crate) fn get_jit_args<'a>( // fill in keyword only defaults if let Some(kw_only_defaults) = &func.kw_only_defaults { - for (i, name) in func.code.kwonlyarg_names.iter().enumerate() { - let arg_idx = i + func.code.arg_names.len(); + for (i, name) in arg_names.kwonlyargs.iter().enumerate() { + let arg_idx = i + func.code.arg_count; if !jit_args.is_set(arg_idx) { let default = kw_only_defaults - .get_item(name.as_str(), vm) + .get_item(name.clone(), vm) .map_err(|_| ArgsError::NotAllArgsPassed) .and_then(|obj| get_jit_value(vm, &obj))?; jit_args.set(arg_idx, default)?; diff --git a/vm/src/builtins/pystr.rs b/vm/src/builtins/pystr.rs index dbb0f6620..7fa90ee6c 100644 --- a/vm/src/builtins/pystr.rs +++ b/vm/src/builtins/pystr.rs @@ -70,6 +70,11 @@ where s.as_ref().to_owned().into() } } +impl AsRef for PyStrRef { + fn as_ref(&self) -> &str { + &self.value + } +} impl From for PyStr { fn from(s: String) -> PyStr { @@ -1452,6 +1457,7 @@ impl<'s> AnyStr<'s, char> for str { } } +#[derive(Clone)] #[repr(transparent)] pub struct PyStrExact(PyStrRef); impl PyStrExact { @@ -1482,3 +1488,8 @@ impl crate::dictdatatype::DictKey for PyStrExact { self.0.key_eq(vm, other_key) } } +impl TryIntoRef for PyStrExact { + fn try_into_ref(self, _vm: &VirtualMachine) -> PyResult> { + Ok(self.0) + } +} diff --git a/vm/src/builtins/pysuper.rs b/vm/src/builtins/pysuper.rs index 908a9a9f6..b48c25c9d 100644 --- a/vm/src/builtins/pysuper.rs +++ b/vm/src/builtins/pysuper.rs @@ -13,7 +13,6 @@ use crate::pyobject::{ BorrowValue, IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; -use crate::scope::NameProtocol; use crate::slots::{SlotDescriptor, SlotGetattro}; use crate::vm::VirtualMachine; @@ -87,10 +86,10 @@ impl PySuper { obj } else { let frame = vm.current_frame().expect("no current frame for super()"); - if let Some(first_arg) = frame.code.arg_names.get(0) { + if let Some(first_arg) = frame.code.arg_names().args.get(0) { let locals = frame.scope.get_locals(); locals - .get_item_option(first_arg.as_str(), vm)? + .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/frame.rs b/vm/src/frame.rs index 442c92a9f..479f86076 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -26,7 +26,7 @@ use crate::pyobject::{ BorrowValue, IdProtocol, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; -use crate::scope::{NameProtocol, Scope}; +use crate::scope::Scope; use crate::slots::PyComparisonOp; use crate::vm::VirtualMachine; @@ -258,7 +258,6 @@ impl ExecutingFrame<'_> { let idx = self.lasti.fetch_add(1, Ordering::Relaxed); let loc = self.code.locations[idx]; let instr = &self.code.instructions[idx]; - vm.check_signals()?; let result = self.execute_instruction(instr, vm); match result { Ok(None) => {} @@ -338,6 +337,8 @@ impl ExecutingFrame<'_> { instruction: &bytecode::Instruction, vm: &VirtualMachine, ) -> FrameResult { + vm.check_signals()?; + flame_guard!(format!("Frame::execute_instruction({:?})", instruction)); #[cfg(feature = "vm-tracing-logging")] @@ -359,17 +360,15 @@ impl ExecutingFrame<'_> { Ok(None) } bytecode::Instruction::Import { - ref name, - ref symbols, - ref level, - } => self.import(vm, name, symbols, *level), + name_idx, + symbols_idx, + level, + } => self.import(vm, *name_idx, symbols_idx, *level), bytecode::Instruction::ImportStar => self.import_star(vm), - bytecode::Instruction::ImportFrom { ref name } => self.import_from(vm, name), - bytecode::Instruction::LoadName { ref name, scope } => self.load_name(vm, name, *scope), - bytecode::Instruction::StoreName { ref name, scope } => { - self.store_name(vm, name, *scope) - } - bytecode::Instruction::DeleteName { ref name } => self.delete_name(vm, name), + 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::Subscript => self.execute_subscript(vm), bytecode::Instruction::StoreSubscript => self.execute_store_subscript(vm), bytecode::Instruction::DeleteSubscript => self.execute_delete_subscript(vm), @@ -453,9 +452,9 @@ impl ExecutingFrame<'_> { bytecode::Instruction::BinaryOperation { ref op, inplace } => { self.execute_binop(vm, op, *inplace) } - bytecode::Instruction::LoadAttr { ref name } => self.load_attr(vm, name), - bytecode::Instruction::StoreAttr { ref name } => self.store_attr(vm, name), - bytecode::Instruction::DeleteAttr { ref name } => self.delete_attr(vm, name), + bytecode::Instruction::LoadAttr { idx } => self.load_attr(vm, *idx), + bytecode::Instruction::StoreAttr { idx } => self.store_attr(vm, *idx), + bytecode::Instruction::DeleteAttr { idx } => self.delete_attr(vm, *idx), bytecode::Instruction::UnaryOperation { ref op } => self.execute_unop(vm, op), bytecode::Instruction::CompareOperation { ref op } => self.execute_compare(vm, op), bytecode::Instruction::ReturnValue => { @@ -778,24 +777,32 @@ impl ExecutingFrame<'_> { fn import( &mut self, vm: &VirtualMachine, - module: &Option, - symbols: &[String], + module: Option, + symbols: &[bytecode::NameIdx], level: usize, ) -> FrameResult { - let module = module.clone().unwrap_or_default(); - let module = vm.import(&module, symbols, level)?; + let module = match module { + Some(idx) => self.code.names[idx].borrow_value(), + None => "", + }; + let from_list = symbols + .iter() + .map(|&idx| self.code.names[idx].clone()) + .collect::>(); + let module = vm.import(&module, &from_list, level)?; self.push_value(module); Ok(None) } #[cfg_attr(feature = "flame-it", flame("Frame"))] - fn import_from(&mut self, vm: &VirtualMachine, name: &str) -> FrameResult { + fn import_from(&mut self, vm: &VirtualMachine, idx: bytecode::NameIdx) -> FrameResult { let module = self.last_value(); + let name = &self.code.names[idx]; // Load attribute, and transform any error into import error. - let obj = vm - .get_attribute(module, name) - .map_err(|_| vm.new_import_error(format!("cannot import name '{}'", name), name))?; + let obj = vm.get_attribute(module, name.clone()).map_err(|_| { + vm.new_import_error(format!("cannot import name '{}'", name), name.clone()) + })?; self.push_value(obj); Ok(None) } @@ -900,9 +907,10 @@ impl ExecutingFrame<'_> { fn store_name( &mut self, vm: &VirtualMachine, - name: &str, + 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 => { @@ -921,8 +929,9 @@ impl ExecutingFrame<'_> { Ok(None) } - fn delete_name(&self, vm: &VirtualMachine, name: &str) -> FrameResult { - match self.scope.delete_name(vm, name) { + 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))), } @@ -933,14 +942,15 @@ impl ExecutingFrame<'_> { fn load_name( &mut self, vm: &VirtualMachine, - name: &str, + 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), - bytecode::NameScope::NonLocal => self.scope.load_cell(vm, name), - bytecode::NameScope::Local => self.scope.load_local(&vm, name), - bytecode::NameScope::Free => self.scope.load_name(&vm, name), + 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 @@ -1441,24 +1451,26 @@ impl ExecutingFrame<'_> { Ok(None) } - fn load_attr(&mut self, vm: &VirtualMachine, attr_name: &str) -> FrameResult { + fn load_attr(&mut self, vm: &VirtualMachine, attr: bytecode::NameIdx) -> FrameResult { + let attr_name = self.code.names[attr].clone(); let parent = self.pop_value(); let obj = vm.get_attribute(parent, attr_name)?; self.push_value(obj); Ok(None) } - fn store_attr(&mut self, vm: &VirtualMachine, attr_name: &str) -> FrameResult { + fn store_attr(&mut self, vm: &VirtualMachine, attr: bytecode::NameIdx) -> FrameResult { + let attr_name = self.code.names[attr].clone(); let parent = self.pop_value(); let value = self.pop_value(); - vm.set_attr(&parent, vm.ctx.new_str(attr_name), value)?; + vm.set_attr(&parent, attr_name, value)?; Ok(None) } - fn delete_attr(&mut self, vm: &VirtualMachine, attr_name: &str) -> FrameResult { + fn delete_attr(&mut self, vm: &VirtualMachine, attr: bytecode::NameIdx) -> FrameResult { + let attr_name = self.code.names[attr].clone().into_object(); let parent = self.pop_value(); - let name = vm.ctx.new_str(attr_name); - vm.del_attr(&parent, name)?; + vm.del_attr(&parent, attr_name)?; Ok(None) } diff --git a/vm/src/scope.rs b/vm/src/scope.rs index f24f78d61..e77027926 100644 --- a/vm/src/scope.rs +++ b/vm/src/scope.rs @@ -1,8 +1,8 @@ use std::fmt; -use crate::builtins::dict::PyDictRef; -use crate::pyobject::{ItemProtocol, PyContext, PyObjectRef, PyResult}; -use crate::vm::VirtualMachine; +use crate::builtins::{PyDictRef, PyStr, PyStrRef}; +use crate::pyobject::{IntoPyObject, ItemProtocol, PyContext, PyObjectRef, PyResult, TryIntoRef}; +use crate::VirtualMachine; /* * So a scope is a linked list of scopes. @@ -69,24 +69,11 @@ impl Scope { pub fn new_child_scope(&self, ctx: &PyContext) -> Scope { self.new_child_scope_with_locals(ctx.new_dict()) } -} -pub trait NameProtocol { - fn load_name(&self, vm: &VirtualMachine, name: &str) -> Option; - fn store_name(&self, vm: &VirtualMachine, name: &str, value: PyObjectRef); - fn delete_name(&self, vm: &VirtualMachine, name: &str) -> PyResult; - fn load_local(&self, vm: &VirtualMachine, name: &str) -> Option; - fn load_cell(&self, vm: &VirtualMachine, name: &str) -> Option; - fn store_cell(&self, vm: &VirtualMachine, name: &str, value: PyObjectRef); - fn load_global(&self, vm: &VirtualMachine, name: &str) -> Option; - fn store_global(&self, vm: &VirtualMachine, name: &str, value: PyObjectRef); -} - -impl NameProtocol for Scope { #[cfg_attr(feature = "flame-it", flame("Scope"))] - fn load_name(&self, vm: &VirtualMachine, name: &str) -> Option { + 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, vm).unwrap() { + if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { return Some(value); } } @@ -97,23 +84,28 @@ impl NameProtocol for Scope { #[cfg_attr(feature = "flame-it", flame("Scope"))] /// Load a local name. Only check the local dictionary for the given name. - fn load_local(&self, vm: &VirtualMachine, name: &str) -> Option { + 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"))] - fn load_cell(&self, vm: &VirtualMachine, name: &str) -> Option { + 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, vm).unwrap() { + if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { return Some(value); } } None } - fn store_cell(&self, vm: &VirtualMachine, name: &str, value: PyObjectRef) { + 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, vm)) { + 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 { @@ -131,25 +123,37 @@ impl NameProtocol for Scope { } } - fn store_name(&self, vm: &VirtualMachine, key: &str, value: PyObjectRef) { + pub fn store_name(&self, vm: &VirtualMachine, key: impl PyName, value: PyObjectRef) { self.get_locals().set_item(key, value, vm).unwrap(); } - fn delete_name(&self, vm: &VirtualMachine, key: &str) -> PyResult { + 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. - fn load_global(&self, vm: &VirtualMachine, name: &str) -> Option { - if let Some(value) = self.globals.get_item_option(name, vm).unwrap() { + 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() } } - fn store_global(&self, vm: &VirtualMachine, name: &str, value: PyObjectRef) { + pub fn store_global(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { self.globals.set_item(name, value, vm).unwrap(); } } + +mod sealed { + pub trait Sealed {} + impl Sealed for &str {} + impl Sealed for super::PyStrRef {} +} +pub trait PyName: + sealed::Sealed + crate::dictdatatype::DictKey + Clone + IntoPyObject + TryIntoRef +{ +} +impl PyName for &str {} +impl PyName for PyStrRef {} diff --git a/vm/src/stdlib/imp.rs b/vm/src/stdlib/imp.rs index f07a584a7..af9378136 100644 --- a/vm/src/stdlib/imp.rs +++ b/vm/src/stdlib/imp.rs @@ -77,10 +77,9 @@ fn _imp_exec_builtin(_mod: PyModuleRef) -> i32 { } fn _imp_get_frozen_object(name: PyStrRef, vm: &VirtualMachine) -> PyResult { - let name = name.borrow_value(); vm.state .frozen - .get(name) + .get(name.borrow_value()) .map(|frozen| { let mut frozen = frozen.code.clone(); frozen.source_path = format!("frozen {}", name); @@ -94,10 +93,9 @@ fn _imp_init_frozen(name: PyStrRef, vm: &VirtualMachine) -> PyResult { } fn _imp_is_frozen_package(name: PyStrRef, vm: &VirtualMachine) -> PyResult { - let name = name.borrow_value(); vm.state .frozen - .get(name) + .get(name.borrow_value()) .map(|frozen| frozen.package) .ok_or_else(|| vm.new_import_error(format!("No such frozen object named {}", name), name)) } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 676a31e18..ffe55e41a 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -700,10 +700,14 @@ impl VirtualMachine { syntax_error } - pub fn new_import_error(&self, msg: String, name: &str) -> PyBaseExceptionRef { + pub fn new_import_error( + &self, + msg: String, + name: impl TryIntoRef, + ) -> PyBaseExceptionRef { let import_error = self.ctx.exceptions.import_error.clone(); let exc = self.new_exception_msg(import_error, msg); - self.set_attr(exc.as_object(), "name", self.new_pyobj(name)) + self.set_attr(exc.as_object(), "name", name.try_into_ref(self).unwrap()) .unwrap(); exc } @@ -819,7 +823,7 @@ impl VirtualMachine { }) } - pub fn import(&self, module: &str, from_list: &[String], level: usize) -> PyResult { + pub fn import(&self, module: &str, from_list: &[PyStrRef], level: usize) -> PyResult { // if the import inputs seem weird, e.g a package import or something, rather than just // a straight `import ident` let weird = module.contains('.') || level != 0 || !from_list.is_empty(); @@ -859,7 +863,7 @@ impl VirtualMachine { }; let from_list = self .ctx - .new_tuple(from_list.iter().map(|name| self.new_pyobj(name)).collect()); + .new_tuple(from_list.iter().map(|x| x.as_object().clone()).collect()); self.invoke(&import_func, (module, globals, locals, from_list, level)) .map_err(|exc| import::remove_importlib_frames(self, &exc)) } @@ -1642,7 +1646,7 @@ impl VirtualMachine { } pub fn intern_string(&self, s: S) -> PyStrRef { - let (s, _) = self + let (s, ()) = self .ctx .string_cache .setdefault_entry(self, s, || ()) diff --git a/wasm/lib/src/browser_module.rs b/wasm/lib/src/browser_module.rs index 58a8166df..7cc418030 100644 --- a/wasm/lib/src/browser_module.rs +++ b/wasm/lib/src/browser_module.rs @@ -4,7 +4,6 @@ use wasm_bindgen::JsCast; use wasm_bindgen_futures::JsFuture; use rustpython_vm::builtins::{PyDictRef, PyStrRef, PyTypeRef}; -use rustpython_vm::common::rc::PyRc; use rustpython_vm::function::OptionalArg; use rustpython_vm::import::import_file; use rustpython_vm::pyobject::{ @@ -276,6 +275,6 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { } pub fn setup_browser_module(vm: &mut VirtualMachine) { - state.add_native_module("_browser".to_owned(), Box::new(make_module)); + vm.add_native_module("_browser".to_owned(), Box::new(make_module)); vm.add_frozen(py_freeze!(file = "src/browser.py", module_name = "browser")); } diff --git a/wasm/lib/src/js_module.rs b/wasm/lib/src/js_module.rs index 6f40a9248..ecc16524d 100644 --- a/wasm/lib/src/js_module.rs +++ b/wasm/lib/src/js_module.rs @@ -7,7 +7,6 @@ use wasm_bindgen::{closure::Closure, prelude::*, JsCast}; use wasm_bindgen_futures::{future_to_promise, JsFuture}; use rustpython_vm::builtins::{PyFloatRef, PyStrRef, PyTypeRef}; -use rustpython_vm::common::rc::PyRc; use rustpython_vm::exceptions::PyBaseExceptionRef; use rustpython_vm::function::{Args, OptionalArg}; use rustpython_vm::pyobject::{ diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs index 3f27bb3d3..35fc59f13 100644 --- a/wasm/lib/src/vm_class.rs +++ b/wasm/lib/src/vm_class.rs @@ -5,10 +5,9 @@ use std::rc::{Rc, Weak}; use js_sys::{Object, TypeError}; use wasm_bindgen::prelude::*; -use rustpython_vm::common::rc::PyRc; use rustpython_vm::compile::{self, Mode}; use rustpython_vm::pyobject::{ItemProtocol, PyObjectRef, PyObjectWeak, PyValue}; -use rustpython_vm::scope::{NameProtocol, Scope}; +use rustpython_vm::scope::Scope; use rustpython_vm::{InitParameter, Interpreter, PySettings, VirtualMachine}; use crate::browser_module::setup_browser_module; @@ -204,7 +203,7 @@ impl WASMVirtualMachine { pub fn add_to_scope(&self, name: String, value: JsValue) -> Result<(), JsValue> { self.with_vm(move |vm, StoredVirtualMachine { ref scope, .. }| { let value = convert::js_to_py(vm, value); - scope.borrow_mut().store_name(&vm, &name, value); + scope.borrow_mut().store_name(&vm, name.as_str(), value); }) }