diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 42c78b2897..53aa36267d 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -1374,7 +1374,7 @@ impl Compiler { for (i, element) in elements.iter().enumerate() { if let ast::ExpressionType::Starred { .. } = &element.node { if seen_star { - return Err(self.error(CompileErrorType::StarArgs)); + return Err(self.error(CompileErrorType::MultipleStarArgs)); } else { seen_star = true; self.emit(Instruction::UnpackEx { @@ -1399,7 +1399,14 @@ impl Compiler { } } } - _ => return Err(self.error(CompileErrorType::Assign(target.name()))), + _ => { + return Err(self.error(match target.node { + ast::ExpressionType::Starred { .. } => CompileErrorType::SyntaxError( + "starred assignment target must be in a list or tuple".to_owned(), + ), + _ => CompileErrorType::Assign(target.name()), + })) + } } Ok(()) @@ -1782,11 +1789,7 @@ impl Compiler { self.compile_comprehension(kind, generators)?; } Starred { .. } => { - return Err( - self.error(CompileErrorType::SyntaxError(std::string::String::from( - "Invalid starred expression", - ))), - ); + return Err(self.error(CompileErrorType::InvalidStarExpr)); } IfExpression { test, body, orelse } => { let no_label = self.new_label(); @@ -2031,21 +2034,33 @@ impl Compiler { } } + let mut compile_element = |element| { + self.compile_expression(element).map_err(|e| { + if matches!(e.error, CompileErrorType::InvalidStarExpr) { + self.error(CompileErrorType::SyntaxError( + "iterable unpacking cannot be used in comprehension".to_owned(), + )) + } else { + e + } + }) + }; + match kind { ast::ComprehensionKind::GeneratorExpression { element } => { - self.compile_expression(element)?; + compile_element(element)?; self.mark_generator(); self.emit(Instruction::YieldValue); self.emit(Instruction::Pop); } ast::ComprehensionKind::List { element } => { - self.compile_expression(element)?; + compile_element(element)?; self.emit(Instruction::ListAppend { i: 1 + generators.len(), }); } ast::ComprehensionKind::Set { element } => { - self.compile_expression(element)?; + compile_element(element)?; self.emit(Instruction::SetAdd { i: 1 + generators.len(), }); diff --git a/compiler/src/error.rs b/compiler/src/error.rs index 85bc9f4625..86635bc951 100644 --- a/compiler/src/error.rs +++ b/compiler/src/error.rs @@ -47,7 +47,9 @@ pub enum CompileErrorType { Parse(ParseErrorType), SyntaxError(String), /// Multiple `*` detected - StarArgs, + MultipleStarArgs, + /// Misplaced `*` expression + InvalidStarExpr, /// Break statement outside of loop. InvalidBreak, /// Continue statement outside of loop. @@ -97,7 +99,10 @@ impl fmt::Display for CompileError { CompileErrorType::ExpectExpr => "Expecting expression, got statement".to_owned(), CompileErrorType::Parse(err) => err.to_string(), CompileErrorType::SyntaxError(err) => err.to_string(), - CompileErrorType::StarArgs => "Two starred expressions in assignment".to_owned(), + CompileErrorType::MultipleStarArgs => { + "two starred expressions in assignment".to_owned() + } + CompileErrorType::InvalidStarExpr => "can't use starred expression here".to_owned(), CompileErrorType::InvalidBreak => "'break' outside loop".to_owned(), CompileErrorType::InvalidContinue => "'continue' outside loop".to_owned(), CompileErrorType::InvalidReturn => "'return' outside function".to_owned(), @@ -120,7 +125,7 @@ impl fmt::Display for CompileError { if self.location.column() > 0 { if let Some(line) = statement.lines().nth(self.location.row() - 1) { // visualize the error, when location and statement are provided - return write!(f, "\n{}\n{}", line, self.location.visualize(&error_desc)); + return write!(f, "{}", self.location.visualize(line, &error_desc)); } } } diff --git a/parser/src/location.rs b/parser/src/location.rs index 1006d80605..1f543e3e36 100644 --- a/parser/src/location.rs +++ b/parser/src/location.rs @@ -16,13 +16,9 @@ impl fmt::Display for Location { } impl Location { - pub fn visualize(&self, desc: &str) -> String { - format!( - "{}↑\n{}{}", - " ".repeat(self.column - 1), - " ".repeat(self.column - 1), - desc - ) + pub fn visualize(&self, line: &str, desc: &str) -> String { + // desc.to_owned() + format!("{}\n{}\n{}↑", desc, line, " ".repeat(self.column - 1)) } } diff --git a/vm/src/frame.rs b/vm/src/frame.rs index e522a8b4f3..0b4416fdb8 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -940,7 +940,9 @@ impl ExecutingFrame<'_> { if unpack { for obj in self.pop_multiple(size) { // Take all key-value pairs from the dict: - let dict: PyDictRef = obj.downcast().expect("Need a dictionary to build a map."); + let dict: PyDictRef = obj.downcast().map_err(|obj| { + vm.new_type_error(format!("'{}' object is not a mapping", obj.class().name)) + })?; for (key, value) in dict { if for_call { if map_obj.contains_key(&key, vm) { @@ -1014,6 +1016,7 @@ impl ExecutingFrame<'_> { let kwargs = if *has_kwargs { let kw_dict: PyDictRef = match self.pop_value().downcast() { Err(_) => { + // TODO: check collections.abc.Mapping return Err(vm.new_type_error("Kwargs must be a dict.".to_owned())); } Ok(x) => x, @@ -1137,7 +1140,7 @@ impl ExecutingFrame<'_> { let min_expected = before + after; if elements.len() < min_expected { Err(vm.new_value_error(format!( - "Not enough values to unpack (expected at least {}, got {}", + "not enough values to unpack (expected at least {}, got {})", min_expected, elements.len() ))) diff --git a/vm/src/obj/objiter.rs b/vm/src/obj/objiter.rs index e79ae0d4da..675df6f33a 100644 --- a/vm/src/obj/objiter.rs +++ b/vm/src/obj/objiter.rs @@ -26,7 +26,7 @@ pub fn get_iter(vm: &VirtualMachine, iter_target: &PyObjectRef) -> PyResult { vm.invoke(&method, vec![]) } else { vm.get_method_or_type_error(iter_target.clone(), "__getitem__", || { - format!("Cannot iterate over {}", iter_target.class().name) + format!("'{}' object is not iterable", iter_target.class().name) })?; Ok(PySequenceIterator::new_forward(iter_target.clone()) .into_ref(vm)