From 544193872563836a971e4bc353f7ca5487efdc6c Mon Sep 17 00:00:00 2001 From: Jeff Date: Tue, 24 Sep 2024 00:24:09 -0400 Subject: [PATCH] Add better parser error --- dust-lang/src/parser/mod.rs | 61 ++++++++++++++++++++++++----------- dust-lang/src/parser/tests.rs | 26 ++++++++++++++- dust-lang/tests/operations.rs | 6 ++++ 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/dust-lang/src/parser/mod.rs b/dust-lang/src/parser/mod.rs index 0257423..fc0537d 100644 --- a/dust-lang/src/parser/mod.rs +++ b/dust-lang/src/parser/mod.rs @@ -745,31 +745,40 @@ impl<'src> Parser<'src> { } fn parse_if(&mut self, allow_assignment: bool, allow_return: bool) -> Result<(), ParseError> { + let position = self.current_position; + self.advance()?; self.parse_expression()?; - let (second_load_boolean, second_position) = + let (mut condition, condition_position) = self.chunk.pop_instruction(self.current_position)?; - let (first_load_boolean, first_position) = - self.chunk.pop_instruction(self.current_position)?; - let length_after_expression = self.chunk.len(); + if let Operation::LoadBoolean = condition.operation() { + condition.set_second_argument_to_boolean(true); + } + + self.emit_instruction(condition, condition_position); + self.emit_instruction(Instruction::jump(1, true), condition_position); self.parse_block(allow_assignment, allow_return)?; + let jump_position = self.current_position; let jump_start = self.current_register; let jump_index = self.chunk.len(); if self.allow(TokenKind::Else)? { if self.allow(TokenKind::If)? { self.parse_if(allow_assignment, allow_return)?; - } else { + } + + if self.allow(TokenKind::LeftCurlyBrace)? { self.parse_block(allow_assignment, allow_return)?; } - } - if self.chunk.len() == length_after_expression { - self.emit_instruction(first_load_boolean, first_position); - self.emit_instruction(second_load_boolean, second_position); + return Err(ParseError::ExpectedTokenMultiple { + expected: &[TokenKind::If, TokenKind::LeftCurlyBrace], + found: self.current_token.to_owned(), + position: self.current_position, + }); } if let [Some(Operation::LoadBoolean), Some(Operation::LoadBoolean)] = @@ -785,15 +794,20 @@ impl<'src> Parser<'src> { second_load_boolean.set_destination(first_load_boolean.destination()); self.emit_instruction(second_load_boolean, second_position); } else if let Some(Operation::LoadBoolean) = self.chunk.get_last_operation() { - // Skip the jump if the last instruction was a LoadBoolean operation. A LoadBoolean can - // skip the following instruction, so a jump is unnecessary. + let (mut load_boolean, position) = self.chunk.pop_instruction(self.current_position)?; + + load_boolean.set_second_argument_to_boolean(true); + + self.emit_instruction(load_boolean, position); } else { let jump_end = self.current_register; let jump_distance = (jump_end - jump_start).max(1); let jump = Instruction::jump(jump_distance, true); - self.chunk - .insert_instruction(jump_index, jump, self.current_position); + if jump_distance > 1 { + self.chunk + .insert_instruction(jump_index, jump, jump_position); + } } Ok(()) @@ -1287,12 +1301,23 @@ impl AnnotatedError for ParseError { Self::ExpectedTokenMultiple { expected, found, .. } => { - let expected = expected - .iter() - .map(|kind| kind.to_string() + ", ") - .collect::(); + let mut details = String::from("Expected"); - Some(format!("Expected one of {expected}, found \"{found}\"")) + for (index, token) in expected.iter().enumerate() { + details.push_str(&format!(" \"{token}\"")); + + if index < expected.len() - 2 { + details.push_str(", "); + } + + if index == expected.len() - 2 { + details.push_str(" or"); + } + } + + details.push_str(&format!(" found \"{found}\"")); + + Some(details) } Self::InvalidAssignmentTarget { found, .. } => { Some(format!("Invalid assignment target, found \"{found}\"")) diff --git a/dust-lang/src/parser/tests.rs b/dust-lang/src/parser/tests.rs index f0c82cb..3dd2c2c 100644 --- a/dust-lang/src/parser/tests.rs +++ b/dust-lang/src/parser/tests.rs @@ -230,6 +230,30 @@ fn equality_assignment_short() { ); } +#[test] +fn if_expression() { + let source = "if 1 == 1 { 2 }"; + + assert_eq!( + parse(source), + Ok(Chunk::with_data( + vec![ + ( + *Instruction::equal(true, 0, 1) + .set_first_argument_to_constant() + .set_second_argument_to_constant(), + Span(5, 7) + ), + (Instruction::jump(1, true), Span(5, 7)), + (Instruction::load_constant(0, 2), Span(12, 13)), + (Instruction::jump(1, true), Span(5, 7)), + ], + vec![Value::integer(1), Value::integer(1), Value::integer(2)], + vec![] + )), + ); +} + #[test] fn if_else_expression() { let source = "if 1 == 1 { 2 } else { 3 }"; @@ -246,7 +270,7 @@ fn if_else_expression() { ), (Instruction::jump(1, true), Span(5, 7)), (Instruction::load_constant(0, 2), Span(12, 13)), - (Instruction::jump(1, true), Span(26, 26)), + (Instruction::jump(1, true), Span(5, 7)), (Instruction::load_constant(0, 3), Span(23, 24)), ], vec![ diff --git a/dust-lang/tests/operations.rs b/dust-lang/tests/operations.rs index a76292e..3fa7649 100644 --- a/dust-lang/tests/operations.rs +++ b/dust-lang/tests/operations.rs @@ -1,5 +1,11 @@ use dust_lang::*; +#[test] +fn if_expression() { + assert_eq!(run("if true { 1 }"), Ok(Some(Value::integer(1)))); + assert_eq!(run("if false { 1 }"), Ok(None)); +} + #[test] fn less_than() { assert_eq!(run("1 < 2"), Ok(Some(Value::boolean(true))));