From cd3e47e0f8078f525f31f32c47e0aabcd2bcc101 Mon Sep 17 00:00:00 2001 From: orizi <104711814+orizi@users.noreply.github.com> Date: Sun, 11 Jun 2023 16:45:04 +0300 Subject: [PATCH] All elements of 0 sized types are equivalent within the type now. (#3362) --- .../src/annotations.rs | 5 +- .../src/compiler_test.rs | 61 +++++++++---------- tests/bug_samples/issue3345.cairo | 44 +++++++++++++ tests/bug_samples/lib.cairo | 1 + 4 files changed, 76 insertions(+), 35 deletions(-) create mode 100644 tests/bug_samples/issue3345.cairo diff --git a/crates/cairo-lang-sierra-to-casm/src/annotations.rs b/crates/cairo-lang-sierra-to-casm/src/annotations.rs index b2439c1baa6..1b9ad11a98b 100644 --- a/crates/cairo-lang-sierra-to-casm/src/annotations.rs +++ b/crates/cairo-lang-sierra-to-casm/src/annotations.rs @@ -439,9 +439,8 @@ fn test_var_consistency( return true; } // If the variable is not ap-dependent it can always be merged. - // We consider empty variables as ap-dependent since we don't know their actual - // source. - if !actual.expression.cells.is_empty() && actual.expression.can_apply_unknown() { + // Note: This makes the assumption that empty variables are always mergable. + if actual.expression.can_apply_unknown() { return true; } // Ap tracking must be enabled when merging non-stack ap-dependent variables. diff --git a/crates/cairo-lang-sierra-to-casm/src/compiler_test.rs b/crates/cairo-lang-sierra-to-casm/src/compiler_test.rs index 51d8b4ef5a1..74151ac85e4 100644 --- a/crates/cairo-lang-sierra-to-casm/src/compiler_test.rs +++ b/crates/cairo-lang-sierra-to-casm/src/compiler_test.rs @@ -357,6 +357,35 @@ use crate::test_utils::{build_metadata, read_sierra_example_file, strip_comments ret; "}; "empty_enum")] +#[test_case(indoc! {" + type felt252 = felt252; + type NonZeroFelt252 = NonZero; + type Unit = Struct; + + libfunc branch_align = branch_align; + libfunc jump = jump; + libfunc felt252_is_zero = felt252_is_zero; + libfunc drop_nz_felt252 = drop; + libfunc revoke_ap_tracking = revoke_ap_tracking; + + revoke_ap_tracking() -> (); + felt252_is_zero([1]) { fallthrough() 4([1]) }; + branch_align() -> (); + jump() { 6() }; + branch_align() -> (); + drop_nz_felt252([1]) -> (); + return ([2]); + + test_program@0([1]: felt252, [2]: Unit) -> (Unit); + "}, + false, + indoc! {" + jmp rel 4 if [fp + -3] != 0; + jmp rel 2; + ret; + "}; + "merge unit param")] + fn sierra_to_casm(sierra_code: &str, check_gas_usage: bool, expected_casm: &str) { let program = ProgramParser::new().parse(sierra_code).unwrap(); pretty_assertions::assert_eq!( @@ -530,38 +559,6 @@ fn sierra_to_casm(sierra_code: &str, check_gas_usage: bool, expected_casm: &str) test_program@0([1]: felt252) -> (felt252, felt252); "}, "#11: Inconsistent references annotations."; "Inconsistent references - different locations on stack")] -#[test_case(indoc! {" - type felt252 = felt252; - type unit = Struct; - type unit_pair = Struct; - type NonZeroFelt252 = NonZero; - - libfunc branch_align = branch_align; - libfunc felt252_dup = dup; - libfunc jump = jump; - libfunc felt252_is_zero = felt252_is_zero; - libfunc store_temp_felt252 = store_temp; - libfunc store_temp_unit_pair = store_temp; - libfunc drop_nz_felt252 = drop; - libfunc drop_unit = drop; - libfunc rename_unit = rename; - libfunc unit_pair_deconstruct = struct_deconstruct; - - store_temp_unit_pair([2]) -> ([2]); - unit_pair_deconstruct([2]) -> ([3], [4]); - felt252_is_zero([1]) { fallthrough() 7([1]) }; - branch_align() -> (); - drop_unit([4]) -> (); - rename_unit([3]) -> ([4]); - jump() { 10() }; - branch_align() -> (); // statement #7. - drop_nz_felt252([1]) -> (); - drop_unit([3]) -> (); - return ([4]); // The failed merge statement #10. - - test_program@0([1]: felt252, [2]: unit_pair) -> (unit); - "}, "#10: Inconsistent references annotations."; - "Inconsistent references - merge on old variable not created at the same point")] #[test_case(indoc! {" type felt252 = felt252; type NonZeroFelt252 = NonZero; diff --git a/tests/bug_samples/issue3345.cairo b/tests/bug_samples/issue3345.cairo new file mode 100644 index 00000000000..d21676b9534 --- /dev/null +++ b/tests/bug_samples/issue3345.cairo @@ -0,0 +1,44 @@ +#[derive(Copy, Drop, storage_access::StorageAccess)] +struct Node { + left: u128, + right: u128 +} + +#[starknet::interface] +trait ITree { + fn sorted_list(ref self: TContractState, root: u128); +} + +#[starknet::contract] +mod ExampleFailure { + use super::Node; + use super::ITree; + use array::{ArrayTrait, Array}; + + #[storage] + struct Storage { + nodes: LegacyMap::, + } + + #[generate_trait] + impl Impl of MyTrait { + fn append_in_order_nodes(self: @ContractState, ref list: Array<(u128, Node)>, at: u128) { + let node = self.nodes.read(at); + if (node.left != 0) { + self.append_in_order_nodes(ref list, node.left); + } + list.append((at, node)); + if (node.right != 0) { + self.append_in_order_nodes(ref list, node.right); + } + } + } + + #[external(v0)] + impl Tree of ITree { + fn sorted_list(ref self: ContractState, root: u128) { + let mut in_order: Array<(u128, Node)> = ArrayTrait::new(); + self.append_in_order_nodes(ref in_order, root); + } + } +} diff --git a/tests/bug_samples/lib.cairo b/tests/bug_samples/lib.cairo index cabd52047b0..5e7ea79eb43 100644 --- a/tests/bug_samples/lib.cairo +++ b/tests/bug_samples/lib.cairo @@ -19,6 +19,7 @@ mod issue2995; mod issue3153; mod issue3192; mod issue3211; +mod issue3345; mod loop_only_change; mod inconsistent_gas; mod partial_param_local;