Fix Array case in check_next_type for arrays containing simple ty…

…pes & Add logic to handle Panic results in `Struct` case separately (#399)

* document todos and implement bytes31

* layout

* Download alexandria lib in CI

* Add alexandria tests

* Add scarb project

* suggestion and fixes

* Build scarb in test target

* Remove dbg print

* Add karatsuba test

* Clean unused imports

* Add more cases

* Add tests using data structures

* Add more tests

* Clippy

* Comment tests that produce invalid mlir

* Avoid searching twice for func

* Pin alexandria version

* Run scarb formatter

* Dont import unused features

* use a different installation method for scarb

* Fix

* Remove unnecessary flag

* Udpate .PHONY

* Remove char

* Use standard installer with flag

* Remove whitespace

* Fix

* Fix target name

* Try solution

* Output scarb version

* pin scarb version

* Build test files in coverage target

* Fix scarb version

* fix

* Fix command duplication

* Use action to install scarb in macos CI

* Allow compare_output to check arrays of simple values

* Fix

* Unignore collatz sequence test

* Update text

* Add logic to handle Panic results separately

* Add test case

* Typo

* Clippy

---------

Co-authored-by: Edgar Luque <[email protected]>

tests/alexandria.rs

@@ -41,8 +41,8 @@ fn compare_inputless_function(function_name: &str) {
 #[test_case("fib")]
 #[test_case("karatsuba")]
 #[test_case("armstrong_number")]
+#[test_case("collatz_sequence")]
 #[test_case("aliquot_sum")]
-#[test_case("collatz_sequence" => ignore["Result mismatch"])]
 #[test_case("extended_euclidean_algorithm")]
 // alexandria_data_structures
 #[test_case("vec" => ignore["Gas mismatch"])]

tests/cases.rs

@@ -18,6 +18,7 @@ mod common;
 #[test_case("tests/cases/pedersen_hash.cairo")]
 #[test_case("tests/cases/unwrap_non_zero.cairo")]
 #[test_case("tests/cases/poseidon.cairo")]
+#[test_case("tests/cases/panic_array.cairo")]
 #[test_case("tests/cases/generic_fn_loop.cairo")]
 // enums
 // TODO: compare error: Fail(Reason("assertion failed: `(left == right)` \n  left: `0`,\n right: `10` at tests/common.rs:453"))

tests/cases/panic_array.cairo

@@ -0,0 +1,9 @@
+use array::Array;
+use array::ArrayTrait;
+
+fn main() {
+    let mut data: Array<felt252> = ArrayTrait::new();
+    data.append('This is an error');
+    data.append('Spanning over two felts');
+    panic(data);
+}

tests/common.rs

@@ -3,6 +3,7 @@
 
 #![allow(dead_code)]
 
+use cairo_felt::Felt252;
 use cairo_lang_compiler::{
     compile_prepared_db, db::RootDatabase, project::setup_project, CompilerConfig,
 };
@@ -361,17 +362,33 @@ pub fn compare_outputs(
         native_rets: &mut impl Iterator<Item = &'a JITValue>,
         vm_rets: &mut Peekable<Iter<'_, String>>,
         reg: &ProgramRegistry<CoreType, CoreLibfunc>,
+        vm_memory: &Vec<Option<Felt252>>,
     ) -> Result<(), TestCaseError> {
         let mut native_rets = native_rets.into_iter().peekable();
         match ty {
             CoreTypeConcrete::Array(info) => {
                 if let JITValue::Array(data) = native_rets.next().unwrap() {
-                    for value in data {
+                    // When it comes to arrays, the vm result contains the starting and ending addresses of the array in memory,
+                    // so we need to fetch each value from the relocated vm memory
+                    // NOTE: This will only work for basic types (those represented by a single felt) and will fail for arrays containing other arrays
+                    let start_address: usize = vm_rets.next().unwrap().parse().unwrap();
+                    let end_address: usize = vm_rets.next().unwrap().parse().unwrap();
+                    assert!(
+                        end_address - start_address == data.len(),
+                        "Comparison between arrays containing non-simple types is not implemented"
+                    );
+                    for (i, value) in data.iter().enumerate() {
+                        // We can't mutate the peekable, so we will create a new one for this value
+                        let memory_value =
+                            format!("{}", vm_memory[start_address + i].clone().unwrap());
+                        let vm_rets = vec![memory_value];
+                        let mut vm_rets = vm_rets.iter().peekable();
                         check_next_type(
                             reg.get_type(&info.ty).expect("type should exist"),
                             &mut [value].into_iter(),
-                            vm_rets,
+                            &mut vm_rets,
                             reg,
+                            vm_memory,
                         )?;
                     }
                 } else {
@@ -497,6 +514,7 @@ pub fn compare_outputs(
                     &mut [native_rets.next().unwrap()].into_iter(),
                     vm_rets,
                     reg,
+                    vm_memory,
                 )?;
             }
             CoreTypeConcrete::Nullable(info) => {
@@ -516,6 +534,7 @@ pub fn compare_outputs(
                         &mut [native_rets.next().unwrap()].into_iter(),
                         vm_rets,
                         reg,
+                        vm_memory,
                     )?;
                 }
             }
@@ -569,6 +588,7 @@ pub fn compare_outputs(
                             &mut [&**value].into_iter(),
                             vm_rets,
                             reg,
+                            vm_memory,
                         )?;
                     } else {
                         let vm_tag = {
@@ -617,6 +637,7 @@ pub fn compare_outputs(
                             &mut [&**value].into_iter(),
                             vm_rets,
                             reg,
+                            vm_memory,
                         )?;
                     }
                 } else {
@@ -625,12 +646,52 @@ pub fn compare_outputs(
             }
             CoreTypeConcrete::Struct(info) => {
                 if let JITValue::Struct { fields, .. } = native_rets.next().unwrap() {
+                    // Check if it is a Panic result
+                    if let Some(JITValue::Struct {
+                        fields: _,
+                        debug_name,
+                    }) = fields.get(0)
+                    {
+                        if debug_name == &Some("core::panics::Panic".to_owned()) {
+                            // The next field of the original struct will be an Array
+                            // But contrary to the standard Array return values, this one will contain the
+                            // actual felt values in the array instead of it's start and end addresses
+                            // So we need to handle it separately
+                            assert!(
+                                fields.len() == 2,
+                                "Panic result has incorrect number of fields"
+                            );
+                            if let JITValue::Array(panic_data) = &fields[1] {
+                                // This is easier than crafting a ConcreteType::Felt252 variant
+                                let felt_concrete_type =
+                                    match reg.get_type(&info.members[1]).unwrap() {
+                                        CoreTypeConcrete::Array(info) => {
+                                            reg.get_type(&info.ty).expect("type should exist")
+                                        }
+                                        _ => unreachable!(),
+                                    };
+                                for value in panic_data {
+                                    check_next_type(
+                                        felt_concrete_type,
+                                        &mut [value].into_iter(),
+                                        vm_rets,
+                                        reg,
+                                        vm_memory,
+                                    )?;
+                                }
+                                return Ok(());
+                            } else {
+                                panic!("Panic result should have an Array as it's second field")
+                            }
+                        }
+                    }
                     for (field, container) in info.members.iter().zip(fields.iter()) {
                         check_next_type(
                             reg.get_type(field).unwrap(),
                             &mut [container].into_iter(),
                             vm_rets,
                             reg,
+                            vm_memory,
                         )?;
                     }
                 } else {
@@ -750,7 +811,7 @@ pub fn compare_outputs(
 
     for ty in ret_types {
         let ty = reg.get_type(ty).unwrap();
-        check_next_type(ty, &mut native_rets, &mut vm_rets, &reg)?;
+        check_next_type(ty, &mut native_rets, &mut vm_rets, &reg, &vm_result.memory)?;
     }
 
     Ok(())