lambdaclass/cairo_native

Implement CoreTypeConcrete::RangeCheck return type

Closed this issue · 5 comments

Implement the parse result for CoreTypeConcrete::RangeCheck

CoreTypeConcrete::RangeCheck(_) => todo!(),

We should be able to run the primitive_types2.cairo program

// #[test_case("tests/cases/cairo_vm/primitive_types2.cairo")]

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