cairo_native
cairo_native copied to clipboard
Implement CoreTypeConcrete::RangeCheck return type
Implement the parse result for CoreTypeConcrete::RangeCheck https://github.com/lambdaclass/cairo_native/blob/ddfd28217c10f7202a03f10234f0a19113c46904/src/executor.rs#L795
We should be able to run the primitive_types2.cairo program https://github.com/lambdaclass/cairo_native/blob/e89d38ad2f01bbd85c9f9de0b37841ac98661b50/tests/tests/cases.rs#L133
hey @pefontana are you working on this or can I give it a try?
Hi @greged93 You can tackle it!
Hello, I think there's been a misunderstanding in the issue.
The RangeCheck is a builtin. All builtins are parsed before parse_result is invoked, therefore making parse_result parse RangeCheck makes no sense. I'm thinking that this issue was caused when a program that returns nothing but uses RangeCheck is executed since the compiler would trigger the panic in parse_result for that specific builtin.
The correct fix would be to check whether the last return value is a builtin. If it's not a builtin then invoke parse_result. Here's the code that should be modified instead:
// Parse return values.
let return_value = function_signature
.ret_types
.last()
.map(|ret_type| {
parse_result(
context,
ret_type,
registry,
module,
metadata,
return_ptr,
ret_registers,
// TODO: Consider returning an Option<JitValue> as return_value instead
// As cairo functions can not have a return value
)
})
.unwrap_or_else(|| JitValue::Struct {
fields: vec![],
debug_name: None,
});
Since you already have one, would you mind fixing that in your PR @greged93 ? The changes in src/executor.rs and tests/common.rs should be reverted, of course.
Makes sense, I was actually wondering why we needed to convert the builtin since builtins were already handled as you said. Will quickly update tmr!
Fixed by #626