add thiserror, split error::Error into different modules#324
Conversation
ddf1800 to
d1bb37b
Compare
| ArgumentMissing(WitnessName), | ||
| ArgumentTypeMismatch(WitnessName, ResolvedType, ResolvedType), | ||
| LexerError(String), | ||
| ParsingError(crate::parse::Error), |
There was a problem hiding this comment.
I was expecting to see ParsingError(#[from] crate::parse::Error), is there a reason it's not done this way?
There was a problem hiding this comment.
I'm leaving the old Display implementation and not migrating to thiserror to keep the changes minimal. Also, I hope that we remove this enum completely, so I think it's fine to leave it as is.
| use crate::types::{AliasedType, BuiltinAlias, TypeConstructible, UIntType}; | ||
|
|
||
| #[derive(Debug, Clone, Eq, PartialEq, Hash)] | ||
| pub struct SyntaxError { |
There was a problem hiding this comment.
| pub struct SyntaxError { | |
| pub struct SyntaxErrorInfo { |
Perhaps better to use a name that doesn't end in Error so that it is not confused with an error that can be returned
| SyntaxError(#[from] SyntaxError), | ||
|
|
||
| #[error("Incompatible match arms: {0}, {1}")] | ||
| IncompatibleMatchArms(MatchPattern, MatchPattern), |
There was a problem hiding this comment.
| IncompatibleMatchArms(MatchPattern, MatchPattern), | |
| IncompatibleMatchArms{ left: MatchPattern, right: MatchPattern}, |
More of a personal preference, but I prefer using the named struct pattern when the inner members are not obvious
There was a problem hiding this comment.
I would also like to implement this in future PRs to keep the changes minimal. The plan is to also refactor these errors to contain more information (additional spans, notes, etc.)
We moved SyntaxError as a separate struct so we can make custom Display implementation
also add inside test reexport for parse::Error so there wouldn't be conflicts after deleteting Error variants
This also made change inside some modules to use ast::Error instead of error::Error.
d1bb37b to
c30d9ca
Compare
Implementing the first step of the error refactor issue: #204 (comment).
I tried to keep the changes minimal, so I did not refactor RichError completely. Also, keep in mind that we will most likely remove
error::Errorand replace RichError with something more like a Diagnostic.