Implement IndentationError #1512
Conversation
| LalrpopError::UnrecognizedToken { token, expected } => { | ||
| // Hacky, but it's how CPython does it. See PyParser_AddToken, | ||
| // in particular "Only one possible expected token" comment. | ||
| let expected = if expected.len() == 1 { |
There was a problem hiding this comment.
This might be an opportunity to use the first function.
There was a problem hiding this comment.
No, because the intent is to get None if expected has more than two elements.
There was a problem hiding this comment.
Ah, okay, I understand. Thank you!
| @@ -416,7 +416,9 @@ impl VirtualMachine { | |||
|
|
|||
| #[cfg(feature = "rustpython-compiler")] | |||
| pub fn new_syntax_error(&self, error: &CompileError) -> PyObjectRef { | |||
| let syntax_error_type = if error.is_tab_error() { | |||
| let syntax_error_type = if error.is_indentation_error() { | |||
There was a problem hiding this comment.
Instead of using a sequence of if statements, we could use here match statements on the error variable to check all it's variants.
I would prefer to use here match indeed, to create the proper matching python error based upon the CompilerErrorType variant, and even the inner LexicalErrorType.
match error {
CompilerErrorType::ParseError(parse) => {
match parse {
.... => self.ctx.exceptions.indentation_error.clone(),
.... => self.ctx.exceptions.tab_error.clone(),
}
},
... => syntaxError.clone()
}There was a problem hiding this comment.
The main motivation of current design is to simplify compiler/VM public interface. In a sense {CompilerErrorType, ParseErrorType, LexicalErrorType} really are internal implementation details, the only thing VM needs is exception type, message, and location.
There was a problem hiding this comment.
I understand the motivation, but I still feel, the vm is so much connected to the parser, it is actually fine to leak the internals of the parser into the vm code.
In any case, let's go with the current design.
|
I took suggestions for |
cc #1385