Parse command return types #46
Conversation
This reverts commit c2d77b5.
| let return_ty = return_ty | ||
| .map(|ty| { | ||
| let AstNode::InOutTypes(types) = self.compiler.get_node(ty) else { | ||
| panic!("internal error: return type is not a return type"); |
There was a problem hiding this comment.
This pattern is repeated a lot and is kind of unwieldy. It could use some helper or API refactor. Just a note for the future.
There was a problem hiding this comment.
Yeah. I was thinking about a macro
There was a problem hiding this comment.
I was thinking along the lines that if we made each AST node its own type, we could use generics, similar to this solution. That would also allow us to deduplicate all the .is_<token>() and .<token>() methods of the Parser.
I'm not a fan of macros since they have different syntax from the rest of Rust, and they can make code hard to read, but sometimes they're helpful.
| name, | ||
| params, | ||
| return_ty: None, | ||
| return_ty, |
There was a problem hiding this comment.
I guess the return_ty can be renamed to inout_types or similar at this point.
There was a problem hiding this comment.
Good point, I'll update Def if I make a PR in the future related to it.
|
Looks good, thanks! I left some notes/comments but nothing major, merging. |
2 participants
This PR parses return types for commands and records these in the typechecker, although it doesn't check that the actual result type matches the expected type.
Regarding parsing, I chose to add a
AstNode::InOutTypes(Vec<NodeId>)variant to hold the list of input-output type pairs. Making that list its own node means we can highlight the entire span[ foo -> bar ]in error messages, including the brackets. But if that's not something we need, I can get rid of theInOutTypesvariant and directly store aVec<NodeId>inside eachAstNode::Def. That'd be simpler.