JIT support for match expressions. #1838
Conversation
| @@ -283,6 +284,26 @@ impl<'a, T: FieldElement> CodeGenerator<'a, T> { | |||
| .unwrap_or_default() | |||
| ) | |||
| } | |||
| Expression::MatchExpression(_, MatchExpression { scrutinee, arms }) => { | |||
| // TODO try to find a solution where we do not introduce a variable | |||
There was a problem hiding this comment.
Not to be done in this PR.
jit-compiler/src/codegen.rs
Outdated
| if let Ok(n) = BigUint::try_from(n) { | ||
| format_unsigned_integer(&n) | ||
| } else { | ||
| format_signed_integer(&-(n.clone())) |
There was a problem hiding this comment.
why is the sign "ignored" here when formatting? also, is there some test using negative integers?
There was a problem hiding this comment.
I don't think this function should be recursive.
| @@ -283,6 +284,26 @@ impl<'a, T: FieldElement> CodeGenerator<'a, T> { | |||
| .unwrap_or_default() | |||
| ) | |||
| } | |||
| Expression::MatchExpression(_, MatchExpression { scrutinee, arms }) => { | |||
There was a problem hiding this comment.
maybe a small comment describing what it compiles to? just sth like "compiles the match arms into a sequence of if let Some(arm_vars) = arm_test { arm_expr } else ..."
jit-compiler/src/codegen.rs
Outdated
| .map(|MatchArm { pattern, value }| { | ||
| let (vars, code) = check_pattern(var_name, pattern)?; | ||
| Ok(format!( | ||
| "if let Some({vars}) = ({code}) {{\n{}\n}}", |
There was a problem hiding this comment.
code could have a more specific name like arm_test
| /// | ||
| /// So if `let (vars, code) = check_pattern("x", pattern)?;`, then the return value | ||
| /// can be used like this: `if let Some({vars}) = ({code}) {{ .. }}` | ||
| fn check_pattern(value_name: &str, pattern: &Pattern) -> Result<(String, String), String> { |
There was a problem hiding this comment.
idk about the name, maybe match_arm_pattern?
There was a problem hiding this comment.
It will also be used in let statements.
| (format!("{value_name}[{i}]"), item) | ||
| }) | ||
| }) | ||
| .map(|(access_name, item)| { |
There was a problem hiding this comment.
i wonder if this map being merged into the preceding filter_map makes it easier to read?
There was a problem hiding this comment.
The issue is that the first is a filter, i.e. returns Option and the second one returns Result. I think it's too complicate to combine both.
| Pattern::Array(_, items) => { | ||
| let mut vars = vec![]; | ||
| let mut ellipsis_seen = false; | ||
| let inner_code = items |
There was a problem hiding this comment.
small comment saying what inner_code will be?
jit-compiler/src/codegen.rs
Outdated
| @@ -436,7 +441,7 @@ fn format_signed_integer(n: &BigInt) -> String { | |||
| if let Ok(n) = BigUint::try_from(n) { | |||
| format_unsigned_integer(&n) | |||
| } else { | |||
| format_signed_integer(&-(n.clone())) | |||
| format!("-{}", format_signed_integer(&-(n.clone()))) | |||
There was a problem hiding this comment.
Is clone really necessary?
Anyway, maybe it is better to avoid recursion:
| format!("-{}", format_signed_integer(&-(n.clone()))) | |
| format!("-{}", format_unsigned_integer(&BigUint::try_from(-n).unwrap()) |
This PR supports match expression, while most of the work regards patterns.
Unfortunately, we cannot use rust patterns directly: The types are too incompatible. For example a
match ("abc", 7) { ("abc", 7) => ...}cannot be directly translated, since the rust types here areStringandibig::IBigand they cannot be used with these literals like that, at least not directly and in all circumstances.The way it is implemented here is that each pattern is compiled to code that is supposed to evaluate to an Option. If the pattern matches, the Option is a Some-value that contains the values that are assigned to the variables in the pattern. The function also returns a string containing the variable names. The function calls itself recursively on recursive data structures.