diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 0a4676288..18ae446d9 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt; /// Sourcode location. -#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct Location { row: usize, column: usize, diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index b85525188..034e0f9a1 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -19,6 +19,8 @@ use rustpython_parser::{ast, parser}; type BasicOutputStream = PeepholeOptimizer; +type CompileResult = Result; + /// Main structure holding the state of compilation. struct Compiler { output_stack: Vec, @@ -59,33 +61,29 @@ pub fn compile( mode: Mode, source_path: String, optimize: u8, -) -> Result { +) -> CompileResult { match mode { Mode::Exec => { let ast = parser::parse_program(source)?; - compile_program(ast, source_path.clone(), optimize) + compile_program(ast, source_path, optimize) } Mode::Eval => { let statement = parser::parse_statement(source)?; - compile_statement_eval(statement, source_path.clone(), optimize) + compile_statement_eval(statement, source_path, optimize) } Mode::Single => { let ast = parser::parse_program(source)?; - compile_program_single(ast, source_path.clone(), optimize) + compile_program_single(ast, source_path, optimize) } } - .map_err(|mut err| { - err.update_source_path(&source_path); - err - }) } /// A helper function for the shared code of the different compile functions fn with_compiler( source_path: String, optimize: u8, - f: impl FnOnce(&mut Compiler) -> Result<(), CompileError>, -) -> Result { + f: impl FnOnce(&mut Compiler) -> CompileResult<()>, +) -> CompileResult { let mut compiler = Compiler::new(optimize); compiler.source_path = Some(source_path); compiler.push_new_code_object("".to_owned()); @@ -100,7 +98,7 @@ pub fn compile_program( ast: ast::Program, source_path: String, optimize: u8, -) -> Result { +) -> CompileResult { with_compiler(source_path, optimize, |compiler| { let symbol_table = make_symbol_table(&ast)?; compiler.compile_program(&ast, symbol_table) @@ -112,7 +110,7 @@ pub fn compile_statement_eval( statement: Vec, source_path: String, optimize: u8, -) -> Result { +) -> CompileResult { with_compiler(source_path, optimize, |compiler| { let symbol_table = statements_to_symbol_table(&statement)?; compiler.compile_statement_eval(&statement, symbol_table) @@ -124,7 +122,7 @@ pub fn compile_program_single( ast: ast::Program, source_path: String, optimize: u8, -) -> Result { +) -> CompileResult { with_compiler(source_path, optimize, |compiler| { let symbol_table = make_symbol_table(&ast)?; compiler.compile_program_single(&ast, symbol_table) @@ -157,6 +155,18 @@ impl Compiler { } } + fn error(&self, error: CompileErrorType) -> CompileError { + self.error_loc(error, self.current_source_location) + } + fn error_loc(&self, error: CompileErrorType, location: ast::Location) -> CompileError { + CompileError { + error, + location, + source_path: self.source_path.clone(), + statement: None, + } + } + fn push_output(&mut self, code: CodeObject) { self.output_stack.push(code.into()); } @@ -184,7 +194,7 @@ impl Compiler { &mut self, program: &ast::Program, symbol_table: SymbolTable, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { let size_before = self.output_stack.len(); self.symbol_table_stack.push(symbol_table); @@ -214,7 +224,7 @@ impl Compiler { &mut self, program: &ast::Program, symbol_table: SymbolTable, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { self.symbol_table_stack.push(symbol_table); let mut emitted_return = false; @@ -253,25 +263,20 @@ impl Compiler { &mut self, statements: &[ast::Statement], symbol_table: SymbolTable, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { self.symbol_table_stack.push(symbol_table); for statement in statements { if let ast::StatementType::Expression { ref expression } = statement.node { self.compile_expression(expression)?; } else { - return Err(CompileError { - statement: None, - error: CompileErrorType::ExpectExpr, - location: statement.location.clone(), - source_path: None, - }); + return Err(self.error_loc(CompileErrorType::ExpectExpr, statement.location)); } } self.emit(Instruction::ReturnValue); Ok(()) } - fn compile_statements(&mut self, statements: &[ast::Statement]) -> Result<(), CompileError> { + fn compile_statements(&mut self, statements: &[ast::Statement]) -> CompileResult<()> { for statement in statements { self.compile_statement(statement)? } @@ -304,9 +309,9 @@ impl Compiler { }); } - fn compile_statement(&mut self, statement: &ast::Statement) -> Result<(), CompileError> { + fn compile_statement(&mut self, statement: &ast::Statement) -> CompileResult<()> { trace!("Compiling {:?}", statement); - self.set_source_location(&statement.location); + self.set_source_location(statement.location); use ast::StatementType::*; match &statement.node { @@ -443,7 +448,7 @@ impl Compiler { } Ok(end_label) }) - .collect::, CompileError>>()?; + .collect::>>()?; self.compile_statements(body)?; @@ -539,34 +544,21 @@ impl Compiler { } Break => { if !self.ctx.in_loop { - return Err(CompileError { - statement: None, - error: CompileErrorType::InvalidBreak, - location: statement.location.clone(), - source_path: None, - }); + return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location)); } self.emit(Instruction::Break); } Continue => { if !self.ctx.in_loop { - return Err(CompileError { - statement: None, - error: CompileErrorType::InvalidContinue, - location: statement.location.clone(), - source_path: None, - }); + return Err( + self.error_loc(CompileErrorType::InvalidContinue, statement.location) + ); } self.emit(Instruction::Continue); } Return { value } => { if !self.ctx.in_func() { - return Err(CompileError { - statement: None, - error: CompileErrorType::InvalidReturn, - location: statement.location.clone(), - source_path: None, - }); + return Err(self.error_loc(CompileErrorType::InvalidReturn, statement.location)); } match value { Some(v) => { @@ -616,7 +608,7 @@ impl Compiler { Ok(()) } - fn compile_delete(&mut self, expression: &ast::Expression) -> Result<(), CompileError> { + fn compile_delete(&mut self, expression: &ast::Expression) -> CompileResult<()> { match &expression.node { ast::ExpressionType::Identifier { name } => { self.emit(Instruction::DeleteName { @@ -639,19 +631,12 @@ impl Compiler { self.compile_delete(element)?; } } - _ => { - return Err(CompileError { - statement: None, - error: CompileErrorType::Delete(expression.name()), - location: self.current_source_location.clone(), - source_path: None, - }); - } + _ => return Err(self.error(CompileErrorType::Delete(expression.name()))), } Ok(()) } - fn enter_function(&mut self, name: &str, args: &ast::Parameters) -> Result<(), CompileError> { + fn enter_function(&mut self, name: &str, args: &ast::Parameters) -> CompileResult<()> { let have_defaults = !args.defaults.is_empty(); if have_defaults { // Construct a tuple: @@ -710,10 +695,7 @@ impl Compiler { Ok(()) } - fn prepare_decorators( - &mut self, - decorator_list: &[ast::Expression], - ) -> Result<(), CompileError> { + fn prepare_decorators(&mut self, decorator_list: &[ast::Expression]) -> CompileResult<()> { for decorator in decorator_list { self.compile_expression(decorator)?; } @@ -735,7 +717,7 @@ impl Compiler { handlers: &[ast::ExceptHandler], orelse: &Option, finalbody: &Option, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { let mut handler_label = self.new_label(); let finally_handler_label = self.new_label(); let else_label = self.new_label(); @@ -849,7 +831,7 @@ impl Compiler { decorator_list: &[ast::Expression], returns: &Option, // TODO: use type hint somehow.. is_async: bool, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { // Create bytecode for this function: // remember to restore self.ctx.in_loop to the original after the function is compiled let prev_ctx = self.ctx; @@ -962,7 +944,7 @@ impl Compiler { bases: &[ast::Expression], keywords: &[ast::Keyword], decorator_list: &[ast::Expression], - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { let prev_ctx = self.ctx; self.ctx = CompileContext { func: FunctionContext::NoFunction, @@ -1102,7 +1084,7 @@ impl Compiler { test: &ast::Expression, body: &[ast::Statement], orelse: &Option>, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { let start_label = self.new_label(); let else_label = self.new_label(); let end_label = self.new_label(); @@ -1138,7 +1120,7 @@ impl Compiler { body: &[ast::Statement], orelse: &Option>, is_async: bool, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { // Start loop let start_label = self.new_label(); let else_label = self.new_label(); @@ -1223,7 +1205,7 @@ impl Compiler { &mut self, vals: &[ast::Expression], ops: &[ast::Comparison], - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { assert!(!ops.is_empty()); assert_eq!(vals.len(), ops.len() + 1); @@ -1294,7 +1276,7 @@ impl Compiler { target: &ast::Expression, annotation: &ast::Expression, value: &Option, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { if let Some(value) = value { self.compile_expression(value)?; self.compile_store(target)?; @@ -1322,7 +1304,7 @@ impl Compiler { Ok(()) } - fn compile_store(&mut self, target: &ast::Expression) -> Result<(), CompileError> { + fn compile_store(&mut self, target: &ast::Expression) -> CompileResult<()> { match &target.node { ast::ExpressionType::Identifier { name } => { self.store_name(name); @@ -1345,12 +1327,7 @@ impl Compiler { for (i, element) in elements.iter().enumerate() { if let ast::ExpressionType::Starred { .. } = &element.node { if seen_star { - return Err(CompileError { - statement: None, - error: CompileErrorType::StarArgs, - location: self.current_source_location.clone(), - source_path: None, - }); + return Err(self.error(CompileErrorType::StarArgs)); } else { seen_star = true; self.emit(Instruction::UnpackEx { @@ -1375,14 +1352,7 @@ impl Compiler { } } } - _ => { - return Err(CompileError { - statement: None, - error: CompileErrorType::Assign(target.name()), - location: self.current_source_location.clone(), - source_path: None, - }); - } + _ => return Err(self.error(CompileErrorType::Assign(target.name()))), } Ok(()) @@ -1420,7 +1390,7 @@ impl Compiler { expression: &ast::Expression, condition: bool, target_label: Label, - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { // Compile expression for test, and jump to label if false match &expression.node { ast::ExpressionType::BoolOp { op, values } => { @@ -1498,7 +1468,7 @@ impl Compiler { &mut self, op: &ast::BooleanOperator, values: &[ast::Expression], - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { let end_label = self.new_label(); let (last_value, values) = values.split_last().unwrap(); @@ -1524,7 +1494,7 @@ impl Compiler { fn compile_dict( &mut self, pairs: &[(Option, ast::Expression)], - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { let mut size = 0; let mut has_unpacking = false; for (is_unpacking, subpairs) in &pairs.iter().group_by(|e| e.0.is_none()) { @@ -1568,9 +1538,9 @@ impl Compiler { Ok(()) } - fn compile_expression(&mut self, expression: &ast::Expression) -> Result<(), CompileError> { + fn compile_expression(&mut self, expression: &ast::Expression) -> CompileResult<()> { trace!("Compiling {:?}", expression); - self.set_source_location(&expression.location); + self.set_source_location(expression.location); use ast::ExpressionType::*; match &expression.node { @@ -1662,12 +1632,7 @@ impl Compiler { } Yield { value } => { if !self.ctx.in_func() { - return Err(CompileError { - statement: Option::None, - error: CompileErrorType::InvalidYield, - location: self.current_source_location.clone(), - source_path: Option::None, - }); + return Err(self.error(CompileErrorType::InvalidYield)); } self.mark_generator(); match value { @@ -1758,14 +1723,11 @@ impl Compiler { self.compile_comprehension(kind, generators)?; } Starred { .. } => { - return Err(CompileError { - statement: Option::None, - error: CompileErrorType::SyntaxError(std::string::String::from( + return Err( + self.error(CompileErrorType::SyntaxError(std::string::String::from( "Invalid starred expression", - )), - location: self.current_source_location.clone(), - source_path: Option::None, - }); + ))), + ); } IfExpression { test, body, orelse } => { let no_label = self.new_label(); @@ -1784,7 +1746,7 @@ impl Compiler { Ok(()) } - fn compile_keywords(&mut self, keywords: &[ast::Keyword]) -> Result<(), CompileError> { + fn compile_keywords(&mut self, keywords: &[ast::Keyword]) -> CompileResult<()> { let mut size = 0; for (is_unpacking, subkeywords) in &keywords.iter().group_by(|e| e.name.is_none()) { if is_unpacking { @@ -1828,7 +1790,7 @@ impl Compiler { function: &ast::Expression, args: &[ast::Expression], keywords: &[ast::Keyword], - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { self.compile_expression(function)?; let count = args.len() + keywords.len(); @@ -1889,7 +1851,7 @@ impl Compiler { // Given a vector of expr / star expr generate code which gives either // a list of expressions on the stack, or a list of tuples. - fn gather_elements(&mut self, elements: &[ast::Expression]) -> Result { + fn gather_elements(&mut self, elements: &[ast::Expression]) -> CompileResult { // First determine if we have starred elements: let has_stars = elements.iter().any(|e| { if let ast::ExpressionType::Starred { .. } = &e.node { @@ -1920,7 +1882,7 @@ impl Compiler { &mut self, kind: &ast::ComprehensionKind, generators: &[ast::Comprehension], - ) -> Result<(), CompileError> { + ) -> CompileResult<()> { // We must have at least one generator: assert!(!generators.is_empty()); @@ -2087,7 +2049,7 @@ impl Compiler { Ok(()) } - fn compile_string(&mut self, string: &ast::StringGroup) -> Result<(), CompileError> { + fn compile_string(&mut self, string: &ast::StringGroup) -> CompileResult<()> { if let Some(value) = try_get_constant_string(string) { self.emit(Instruction::LoadConst { value: bytecode::Constant::String { value }, @@ -2182,8 +2144,8 @@ impl Compiler { self.current_output().set_label(label) } - fn set_source_location(&mut self, location: &ast::Location) { - self.current_source_location = location.clone(); + fn set_source_location(&mut self, location: ast::Location) { + self.current_source_location = location; } fn get_source_line_number(&mut self) -> usize { diff --git a/compiler/src/peephole.rs b/compiler/src/peephole.rs index 3a64380bd..b85266867 100644 --- a/compiler/src/peephole.rs +++ b/compiler/src/peephole.rs @@ -15,7 +15,7 @@ impl From> for InstructionMetadata { fn from(metas: Vec) -> Self { debug_assert!(!metas.is_empty(), "`metas` must not be empty"); InstructionMetadata { - loc: metas[0].loc.clone(), + loc: metas[0].loc, labels: metas .into_iter() .flat_map(|meta| meta.labels.into_iter()) diff --git a/parser/src/error.rs b/parser/src/error.rs index 6a8c0340c..ebe683c8c 100644 --- a/parser/src/error.rs +++ b/parser/src/error.rs @@ -193,7 +193,7 @@ impl fmt::Display for ParseErrorType { ParseErrorType::UnrecognizedToken(ref tok, ref expected) => { if *tok == Tok::Indent { write!(f, "unexpected indent") - } else if expected.clone() == Some("Indent".to_owned()) { + } else if expected.as_deref() == Some("Indent") { write!(f, "expected an indented block") } else { write!(f, "Got unexpected token {}", tok) diff --git a/parser/src/function.rs b/parser/src/function.rs index 98f2dbca5..96366d3c5 100644 --- a/parser/src/function.rs +++ b/parser/src/function.rs @@ -22,7 +22,7 @@ pub fn parse_params( // have defaults return Err(LexicalError { error: LexicalErrorType::DefaultArgumentError, - location: name.location.clone(), + location: name.location, }); } } diff --git a/parser/src/lexer.rs b/parser/src/lexer.rs index 581b388a4..ab6bd5675 100644 --- a/parser/src/lexer.rs +++ b/parser/src/lexer.rs @@ -285,7 +285,7 @@ where let end_pos = self.get_pos(); let value = BigInt::from_str_radix(&value_text, radix).map_err(|e| LexicalError { error: LexicalErrorType::OtherError(format!("{:?}", e)), - location: start_pos.clone(), + location: start_pos, })?; Ok((start_pos, Tok::Int { value }, end_pos)) } @@ -711,9 +711,8 @@ where Ordering::Greater => { // New indentation level: self.indentation_stack.push(indentation_level); - let tok_start = self.get_pos(); - let tok_end = tok_start.clone(); - self.emit((tok_start, Tok::Indent, tok_end)); + let tok_pos = self.get_pos(); + self.emit((tok_pos, Tok::Indent, tok_pos)); } Ordering::Less => { // One or more dedentations @@ -726,9 +725,8 @@ where match ordering { Ordering::Less => { self.indentation_stack.pop(); - let tok_start = self.get_pos(); - let tok_end = tok_start.clone(); - self.emit((tok_start, Tok::Dedent, tok_end)); + let tok_pos = self.get_pos(); + self.emit((tok_pos, Tok::Dedent, tok_pos)); } Ordering::Equal => { // We arrived at proper level of indentation. @@ -786,16 +784,16 @@ where // Next, insert a trailing newline, if required. if !self.at_begin_of_line { self.at_begin_of_line = true; - self.emit((tok_pos.clone(), Tok::Newline, tok_pos.clone())); + self.emit((tok_pos, Tok::Newline, tok_pos)); } // Next, flush the indentation stack to zero. while self.indentation_stack.len() > 1 { self.indentation_stack.pop(); - self.emit((tok_pos.clone(), Tok::Dedent, tok_pos.clone())); + self.emit((tok_pos, Tok::Dedent, tok_pos)); } - self.emit((tok_pos.clone(), Tok::EndOfFile, tok_pos)); + self.emit((tok_pos, Tok::EndOfFile, tok_pos)); } Ok(()) @@ -1194,7 +1192,7 @@ where /// Helper function to retrieve the current position. fn get_pos(&self) -> Location { - self.location.clone() + self.location } /// Helper function to emit a lexed token to the queue of tokens. diff --git a/parser/src/location.rs b/parser/src/location.rs index c1f8c158a..1006d8060 100644 --- a/parser/src/location.rs +++ b/parser/src/location.rs @@ -3,7 +3,7 @@ use std::fmt; /// A location somewhere in the sourcecode. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Copy, Debug, Default, PartialEq)] pub struct Location { row: usize, column: usize, diff --git a/parser/src/parser.rs b/parser/src/parser.rs index 65649f6e5..4be140e77 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -138,7 +138,7 @@ mod tests { fn as_statement(expr: ast::Expression) -> ast::Statement { ast::Statement { - location: expr.location.clone(), + location: expr.location, node: ast::StatementType::Expression { expression: expr }, } } diff --git a/parser/src/python.lalrpop b/parser/src/python.lalrpop index 6f08248cb..548f256e8 100644 --- a/parser/src/python.lalrpop +++ b/parser/src/python.lalrpop @@ -874,9 +874,9 @@ SubscriptList: ast::Expression = { Subscript: ast::Expression = { Test, ":" => { - let s1 = e1.unwrap_or(ast::Expression { location: location.clone(), node: ast::ExpressionType::None }); - let s2 = e2.unwrap_or(ast::Expression { location: location.clone(), node: ast::ExpressionType::None }); - let s3 = e3.unwrap_or(ast::Expression { location: location.clone(), node: ast::ExpressionType::None }); + let s1 = e1.unwrap_or(ast::Expression { location, node: ast::ExpressionType::None }); + let s2 = e2.unwrap_or(ast::Expression { location, node: ast::ExpressionType::None }); + let s3 = e3.unwrap_or(ast::Expression { location, node: ast::ExpressionType::None }); ast::Expression { location, node: ast::ExpressionType::Slice { elements: vec![s1, s2, s3] } @@ -1061,7 +1061,7 @@ FunctionArgument: (Option>, ast::Expression) = { => { let expr = match c { Some(c) => ast::Expression { - location: e.location.clone(), + location: e.location, node: ast::ExpressionType::Comprehension { kind: Box::new(ast::ComprehensionKind::GeneratorExpression { element: e }), generators: c, @@ -1071,7 +1071,7 @@ FunctionArgument: (Option>, ast::Expression) = { }; (None, expr) }, - "=" => (Some(Some(i.clone())), e), + "=" => (Some(Some(i)), e), "*" => (None, ast::Expression { location, node: ast::ExpressionType::Starred { value: Box::new(e) } }), "**" => (Some(None), e), }; @@ -1105,7 +1105,7 @@ StringGroup: ast::StringGroup = { let mut values = vec![]; for (value, is_fstring) in s { values.push(if is_fstring { - parse_located_fstring(&value, loc.clone())? + parse_located_fstring(&value, loc)? } else { ast::StringGroup::Constant { value } }) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 681d5b2d4..21c54277a 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -1348,7 +1348,7 @@ impl Frame { } pub fn get_lineno(&self) -> bytecode::Location { - self.code.locations[self.lasti.get()].clone() + self.code.locations[self.lasti.get()] } fn push_block(&self, typ: BlockType) {