Skip to content

Commit b562ff5

Browse files
coolreader18coolreader18
authored
Merge pull request #1922 from youknowone/unpack-ex
Fix unpack_ex error messages
2 parents 0ec86f1 + 9338dcd commit b562ff5

File tree

5 files changed

+42
-23
lines changed

5 files changed

+42
-23
lines changed

‎compiler/src/compile.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ impl<O: OutputStream> Compiler<O> {
13741374
for (i, element) in elements.iter().enumerate() {
13751375
if let ast::ExpressionType::Starred { .. } = &element.node {
13761376
if seen_star {
1377-
return Err(self.error(CompileErrorType::StarArgs));
1377+
return Err(self.error(CompileErrorType::MultipleStarArgs));
13781378
} else {
13791379
seen_star = true;
13801380
self.emit(Instruction::UnpackEx {
@@ -1399,7 +1399,14 @@ impl<O: OutputStream> Compiler<O> {
13991399
}
14001400
}
14011401
}
1402-
_ => return Err(self.error(CompileErrorType::Assign(target.name()))),
1402+
_ => {
1403+
return Err(self.error(match target.node {
1404+
ast::ExpressionType::Starred { .. } => CompileErrorType::SyntaxError(
1405+
"starred assignment target must be in a list or tuple".to_owned(),
1406+
),
1407+
_ => CompileErrorType::Assign(target.name()),
1408+
}))
1409+
}
14031410
}
14041411

14051412
Ok(())
@@ -1782,11 +1789,7 @@ impl<O: OutputStream> Compiler<O> {
17821789
self.compile_comprehension(kind, generators)?;
17831790
}
17841791
Starred { .. } => {
1785-
return Err(
1786-
self.error(CompileErrorType::SyntaxError(std::string::String::from(
1787-
"Invalid starred expression",
1788-
))),
1789-
);
1792+
return Err(self.error(CompileErrorType::InvalidStarExpr));
17901793
}
17911794
IfExpression { test, body, orelse } => {
17921795
let no_label = self.new_label();
@@ -2031,21 +2034,33 @@ impl<O: OutputStream> Compiler<O> {
20312034
}
20322035
}
20332036

2037+
let mut compile_element = |element| {
2038+
self.compile_expression(element).map_err(|e| {
2039+
if matches!(e.error, CompileErrorType::InvalidStarExpr) {
2040+
self.error(CompileErrorType::SyntaxError(
2041+
"iterable unpacking cannot be used in comprehension".to_owned(),
2042+
))
2043+
} else {
2044+
e
2045+
}
2046+
})
2047+
};
2048+
20342049
match kind {
20352050
ast::ComprehensionKind::GeneratorExpression { element } => {
2036-
self.compile_expression(element)?;
2051+
compile_element(element)?;
20372052
self.mark_generator();
20382053
self.emit(Instruction::YieldValue);
20392054
self.emit(Instruction::Pop);
20402055
}
20412056
ast::ComprehensionKind::List { element } => {
2042-
self.compile_expression(element)?;
2057+
compile_element(element)?;
20432058
self.emit(Instruction::ListAppend {
20442059
i: 1 + generators.len(),
20452060
});
20462061
}
20472062
ast::ComprehensionKind::Set { element } => {
2048-
self.compile_expression(element)?;
2063+
compile_element(element)?;
20492064
self.emit(Instruction::SetAdd {
20502065
i: 1 + generators.len(),
20512066
});

‎compiler/src/error.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ pub enum CompileErrorType {
4747
Parse(ParseErrorType),
4848
SyntaxError(String),
4949
/// Multiple `*` detected
50-
StarArgs,
50+
MultipleStarArgs,
51+
/// Misplaced `*` expression
52+
InvalidStarExpr,
5153
/// Break statement outside of loop.
5254
InvalidBreak,
5355
/// Continue statement outside of loop.
@@ -97,7 +99,10 @@ impl fmt::Display for CompileError {
9799
CompileErrorType::ExpectExpr => "Expecting expression, got statement".to_owned(),
98100
CompileErrorType::Parse(err) => err.to_string(),
99101
CompileErrorType::SyntaxError(err) => err.to_string(),
100-
CompileErrorType::StarArgs => "Two starred expressions in assignment".to_owned(),
102+
CompileErrorType::MultipleStarArgs => {
103+
"two starred expressions in assignment".to_owned()
104+
}
105+
CompileErrorType::InvalidStarExpr => "can't use starred expression here".to_owned(),
101106
CompileErrorType::InvalidBreak => "'break' outside loop".to_owned(),
102107
CompileErrorType::InvalidContinue => "'continue' outside loop".to_owned(),
103108
CompileErrorType::InvalidReturn => "'return' outside function".to_owned(),
@@ -120,7 +125,7 @@ impl fmt::Display for CompileError {
120125
if self.location.column() > 0 {
121126
if let Some(line) = statement.lines().nth(self.location.row() - 1) {
122127
// visualize the error, when location and statement are provided
123-
return write!(f, "\n{}\n{}", line, self.location.visualize(&error_desc));
128+
return write!(f, "{}", self.location.visualize(line, &error_desc));
124129
}
125130
}
126131
}

‎parser/src/location.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,9 @@ impl fmt::Display for Location {
1616
}
1717

1818
impl Location {
19-
pub fn visualize(&self, desc: &str) -> String {
20-
format!(
21-
"{}↑\n{}{}",
22-
" ".repeat(self.column - 1),
23-
" ".repeat(self.column - 1),
24-
desc
25-
)
19+
pub fn visualize(&self, line: &str, desc: &str) -> String {
20+
// desc.to_owned()
21+
format!("{}\n{}\n{}↑", desc, line, " ".repeat(self.column - 1))
2622
}
2723
}
2824

‎vm/src/frame.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,9 @@ impl ExecutingFrame<'_> {
940940
if unpack {
941941
for obj in self.pop_multiple(size) {
942942
// Take all key-value pairs from the dict:
943-
let dict: PyDictRef = obj.downcast().expect("Need a dictionary to build a map.");
943+
let dict: PyDictRef = obj.downcast().map_err(|obj| {
944+
vm.new_type_error(format!("'{}' object is not a mapping", obj.class().name))
945+
})?;
944946
for (key, value) in dict {
945947
if for_call {
946948
if map_obj.contains_key(&key, vm) {
@@ -1014,6 +1016,7 @@ impl ExecutingFrame<'_> {
10141016
let kwargs = if *has_kwargs {
10151017
let kw_dict: PyDictRef = match self.pop_value().downcast() {
10161018
Err(_) => {
1019+
// TODO: check collections.abc.Mapping
10171020
return Err(vm.new_type_error("Kwargs must be a dict.".to_owned()));
10181021
}
10191022
Ok(x) => x,
@@ -1137,7 +1140,7 @@ impl ExecutingFrame<'_> {
11371140
let min_expected = before + after;
11381141
if elements.len() < min_expected {
11391142
Err(vm.new_value_error(format!(
1140-
"Not enough values to unpack (expected at least {}, got {}",
1143+
"not enough values to unpack (expected at least {}, got {})",
11411144
min_expected,
11421145
elements.len()
11431146
)))

‎vm/src/obj/objiter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub fn get_iter(vm: &VirtualMachine, iter_target: &PyObjectRef) -> PyResult {
2626
vm.invoke(&method, vec![])
2727
} else {
2828
vm.get_method_or_type_error(iter_target.clone(), "__getitem__", || {
29-
format!("Cannot iterate over {}", iter_target.class().name)
29+
format!("'{}' object is not iterable", iter_target.class().name)
3030
})?;
3131
Ok(PySequenceIterator::new_forward(iter_target.clone())
3232
.into_ref(vm)

0 commit comments

Comments
 (0)