feat(ir): Normalize catch clauses in the IR - #2031
Conversation
Removes `CatchClauseError` in favor of `CatchClauseKind` which is an enum with the valid variants for `Error`, `Panic` and low-level. This ensures the IR only contains valid catch clauses (modulo parameters, to be checked later when type info is available) and the clause kind is encoded in the enum variant. Also removes the now obsolete `ErrorOrPanic` built-in.
|
| mutator.add_choice_variant("CatchClauseKind", "ClauseErrorKind"); | ||
| mutator.add_choice_variant("CatchClauseKind", "ClausePanicKind"); | ||
| mutator.add_choice_variant("CatchClauseKind", "ClauseLowLevelKind"); |
There was a problem hiding this comment.
nit: for the three variants, the Kind suffix reads to me like they are enums on their own. WDYT of renaming them to ErrorCatchClause, PanicCatchClause, and LowLevelCatchClause? similar to other types in the codebase.
hedgar2017
left a comment
hedgar2017
left a comment
There was a problem hiding this comment.
One API request from the solx side; the normalized shape otherwise works well — I've already adapted solx to this PR and it compiles and passes tests.
| use crate::ir; | ||
|
|
||
| impl ir::CatchClauseStruct { | ||
| pub fn parameters(&self) -> Option<&ir::Parameters> { |
There was a problem hiding this comment.
Could we mirror this extension on the AST layer as ast::CatchClauseStruct::parameters() -> Option<Parameters>? In solx we consume catch clauses through ast::CatchClauseStruct, and binding the clause parameter currently requires matching all three CatchClauseKind variants just to reach parameters():
let parameters = match node.kind() {
CatchClauseKind::ClauseErrorKind(kind) => Some(kind.parameters()),
CatchClauseKind::ClausePanicKind(kind) => Some(kind.parameters()),
CatchClauseKind::ClauseLowLevelKind(kind) => kind.parameters(),
};An AST mirror would collapse this to a single call, while the kind-specific cardinality stays available on the variants for callers that need it.
3 participants
This spawned from the discussion in #2023. But the code changed so much that I decided to submit a new PR.
Removes
CatchClauseErrorin favor ofCatchClauseKindwhich is an enum with the valid variants forError,Panicand low-level.This ensures the IR only contains valid catch clauses (modulo parameters, to be checked later when type info is available) and the clause kind is encoded in the enum variant.
Also removes the now obsolete
ErrorOrPanicbuilt-in.