Skip to content

Commit

Permalink
Fixing issue #4034. Removing unused vars goto mappings. (#4069)
Browse files Browse the repository at this point in the history
  • Loading branch information
orizi authored Sep 14, 2023
1 parent 7f25914 commit bb11012
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 2 deletions.
3 changes: 3 additions & 0 deletions crates/cairo-lang-lowering/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,9 @@ fn concrete_function_with_body_lowered(
optimize_remappings(&mut lowered);
reorder_statements(db, &mut lowered);
reorganize_blocks(&mut lowered);
// Removed blocks may have caused some remappings to be redundent, so they need to be removed,
// as SierraGen drop additions assumes all remappings are of used variables.
optimize_remappings(&mut lowered);
Ok(Arc::new(lowered))
}

Expand Down
3 changes: 2 additions & 1 deletion crates/cairo-lang-lowering/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ fn test_function_lowering_phases(
apply_stage("after_lower_implicits", &|lowered| lower_implicits(&db, function_id, lowered));
apply_stage("after_optimize_remappings2", &optimize_remappings);
apply_stage("after_reorder_statements3", &|lowered| reorder_statements(&db, lowered));
apply_stage("after_reorganize_blocks (final)", &reorganize_blocks);
apply_stage("after_reorganize_blocks", &reorganize_blocks);
apply_stage("after_optimize_remappings3 (final)", &optimize_remappings);

let after_all = db.concrete_function_with_body_lowered(function_id).unwrap();

Expand Down
47 changes: 46 additions & 1 deletion crates/cairo-lang-lowering/src/test_data/lowering_phases
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,52 @@ Statements:
End:
Return(v34, v35, v25)

//! > after_reorganize_blocks (final)
//! > after_reorganize_blocks
Parameters: v26: core::RangeCheck, v27: core::gas::GasBuiltin
blk0 (root):
Statements:
(v16: core::gas::BuiltinCosts) <- core::gas::get_builtin_costs()
End:
Match(match core::gas::withdraw_gas_all(v26, v27, v16) {
Option::Some(v28, v29) => blk1,
Option::None(v30, v31) => blk4,
})

blk1:
Statements:
(v34: core::RangeCheck, v35: core::gas::GasBuiltin, v17: core::panics::PanicResult::<((),)>) <- test::foo(v28, v29)
End:
Match(match_enum(v17) {
PanicResult::Ok(v18) => blk2,
PanicResult::Err(v20) => blk3,
})

blk2:
Statements:
(v19: ()) <- struct_destructure(v18)
(v21: ((),)) <- struct_construct(v19)
(v22: core::panics::PanicResult::<((),)>) <- PanicResult::Ok(v21)
End:
Return(v34, v35, v22)

blk3:
Statements:
(v25: core::panics::PanicResult::<((),)>) <- PanicResult::Err(v20)
End:
Return(v34, v35, v25)

blk4:
Statements:
(v11: core::array::Array::<core::felt252>) <- core::array::array_new::<core::felt252>()
(v12: core::felt252) <- 375233589013918064796019u
(v14: core::array::Array::<core::felt252>) <- core::array::array_append::<core::felt252>(v11, v12)
(v13: core::panics::Panic) <- struct_construct()
(v15: (core::panics::Panic, core::array::Array::<core::felt252>)) <- struct_construct(v13, v14)
(v24: core::panics::PanicResult::<((),)>) <- PanicResult::Err(v15)
End:
Return(v30, v31, v24)

//! > after_optimize_remappings3 (final)
Parameters: v26: core::RangeCheck, v27: core::gas::GasBuiltin
blk0 (root):
Statements:
Expand Down
7 changes: 7 additions & 0 deletions tests/bug_samples/issue4034.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[test]
fn var_dropped_after_merge_remapping() {
let mut x: u32 = 0;
if (x == 0 && x != 1 && false) {
x = x + 1;
}
}
1 change: 1 addition & 0 deletions tests/bug_samples/lib.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ mod issue3345;
mod issue3658;
mod issue3863;
mod issue4007;
mod issue4034;
mod issue4036;
mod loop_only_change;
mod inconsistent_gas;
Expand Down

0 comments on commit bb11012

Please sign in to comment.