"improve: error msgs and handling" #209
Conversation
|
Quick question: in mast/mod.rs/42:13: should the constructor of |
|
actually I see this is a draft, so let me know when it's good to be merged! |
|
Panics are now limited to circuit_writer.rs and some constructors |
crypdoughdoteth
changed the title
| } | ||
|
|
||
| terms | ||
| Ok(terms) | ||
| } |
| Err(Error::new( | ||
| "constraint-generation", | ||
| ErrorKind::AssertTypeMismatch("rhs", rhs), | ||
| span, |
There was a problem hiding this comment.
I think UnexpectedError is still good here, as I don't think we should run into these things (the type checker will catch that before we hit this place)
There was a problem hiding this comment.
The main issue that I ran into here is that I wanted to also include res_info.typ in the error message just like in the panic version. format!() returns String and UnexpectedError requires &'static str as its tagged field. Borrowing the String does not last for 'static unless I leak the allocation. In order to get around this, I created this new error variant. The alternative was a slightly more opaque error message. I'm not sure if this is the right thing per se, but worth discussing briefly
src/type_checker/checker.rs
Outdated
| .clone(); | ||
|
|
||
| // and is mutable | ||
| if !lhs_info.mutable { | ||
| return Err(self.error(ErrorKind::AssignmentToImmutableVariable, expr.span)); | ||
| if let Some(lhs_info) = lhs_info { |
There was a problem hiding this comment.
wait why is lhs_info still an option here
There was a problem hiding this comment.
I figured out what went wrong here: I didn't handle the Result with ? produced from get_type_info()
| .get_type_info(&lhs_name) | ||
| .or_else(|_| { | ||
| Err(self.error(ErrorKind::UnexpectedError("variable not found"), expr.span)) | ||
| })? |
|
Lgtm! Thanks so much for the PR! I'll let @katat merge after looking at #209 (comment) |
|
Looks good, thanks! |
|
CI should be green now 🫡 |
Closes #26 - removes panics in favor of clear error messages and improves error handling in some places