Sitelet https://github.com/starkware-libs/cairo/commit/782884c31fb4da03d17644beec8c11286411c1aa
Skip to content

Commit 782884c

Browse files
oriziorizi
committed
fix(lowering): early_unsafe_panic must keep side-effecting statements
 The pass replaces the dead tail of a function (from which return is unreachable) with unsafe_panic, and is meant to preserve side-effecting libfunc calls (debug::print / internal::trace) that must still run first. But it scanned statements forward and truncated AT the first side effect (`truncate(i)`), deleting the very statement it meant to keep. It also only considered the first side effect, dropping later ones in the same block. Scan in reverse and truncate after the last side effect (`i + 1`). Move the test from PreOptimizations to PostBaseline (where this pass actually runs, so print is the inlined extern it detects) and add a regression case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0ef1296 commit 782884c

3 files changed

Lines changed: 89 additions & 24 deletions

File tree

‎crates/cairo-lang-lowering/src/optimizations/early_unsafe_panic.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,14 @@ impl<'db, 'a> DataflowAnalyzer<'db, 'a> for UnsafePanicContext<'db> {
106106
{
107107
self.fixes.push(((block_id, block.statements.len()), *match_info.location()));
108108
}
109-
if ReachableSideEffects::Reachable == *info {
109+
let ReachableSideEffects::Unreachable(location) = *info else {
110110
return;
111-
}
112-
for (i, stmt) in block.statements.iter().enumerate() {
113-
if self.has_side_effects(stmt)
114-
&& let ReachableSideEffects::Unreachable(locations) = *info
115-
{
116-
self.fixes.push(((block_id, i), locations));
111+
};
112+
// Everything past the last side-effecting statement is dead (no `return` is reachable
113+
// from here), so replace it with an early panic — but keep the side effects themselves.
114+
for (i, stmt) in block.statements.iter().enumerate().rev() {
115+
if self.has_side_effects(stmt) {
116+
self.fixes.push(((block_id, i + 1), location));
117117
*info = ReachableSideEffects::Reachable;
118118
break;
119119
}

‎crates/cairo-lang-lowering/src/optimizations/early_unsafe_panic_test.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ fn test_early_unsafe_panic(
3333
let function_id =
3434
ConcreteFunctionWithBodyId::from_semantic(db, test_function.concrete_function_id);
3535

36-
let before = db.lowered_body(function_id, LoweringStage::PreOptimizations).unwrap().clone();
36+
// `EarlyUnsafePanic` runs in the final optimization strategy, i.e. on `PostBaseline` lowering
37+
// (after inlining). Use that stage so side-effecting externs like `debug::print` are visible.
38+
let before = db.lowered_body(function_id, LoweringStage::PostBaseline).unwrap().clone();
3739

3840
let lowering_diagnostics = db.module_lowering_diagnostics(test_function.module_id).unwrap();
3941
let mut after = before.clone();

‎crates/cairo-lang-lowering/src/optimizations/test_data/early_unsafe_panic‎

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,57 @@ End:
3131
blk1:
3232
Statements:
3333
End:
34-
Goto(blk3, {v1 -> v5})
34+
Return(v1)
3535

3636
blk2:
3737
Statements:
38-
(v3: core::felt252) <- 435711799154
39-
(v4: core::never) <- core::panic_with_felt252(v3)
38+
(v3: core::never) <- core::panic_with_const_felt252::<435711799154>()
4039
End:
41-
Match(match_enum(v4) {
40+
Match(match_enum(v3) {
4241
})
4342

44-
blk3:
43+
//! > after
44+
Parameters: v0: core::option::Option::<core::integer::u32>
45+
blk0 (root):
4546
Statements:
4647
End:
47-
Return(v5)
48+
Match(match_enum(v0) {
49+
Option::Some(v1) => blk1,
50+
Option::None(v2) => blk2,
51+
})
4852

49-
//! > after
53+
blk1:
54+
Statements:
55+
End:
56+
Return(v1)
57+
58+
blk2:
59+
Statements:
60+
End:
61+
Match(match core::panics::unsafe_panic() {
62+
})
63+
64+
//! > ==========================================================================
65+
66+
//! > Test panic in root block.
67+
68+
//! > test_runner_name
69+
test_early_unsafe_panic
70+
71+
//! > function_code
72+
fn foo(a: Option<u32>) -> u32 {
73+
a.unwrap();
74+
core::panic_with_felt252('error')
75+
}
76+
77+
//! > function_name
78+
foo
79+
80+
//! > semantic_diagnostics
81+
82+
//! > lowering_diagnostics
83+
84+
//! > before
5085
Parameters: v0: core::option::Option::<core::integer::u32>
5186
blk0 (root):
5287
Statements:
@@ -58,30 +93,52 @@ End:
5893

5994
blk1:
6095
Statements:
96+
(v3: core::never) <- core::panic_with_const_felt252::<435711799154>()
6197
End:
62-
Goto(blk3, {v1 -> v5})
98+
Match(match_enum(v3) {
99+
})
63100

64101
blk2:
102+
Statements:
103+
(v4: core::never) <- core::panic_with_const_felt252::<29721761890975875353235833581453094220424382983267374>()
104+
End:
105+
Match(match_enum(v4) {
106+
})
107+
108+
//! > after
109+
Parameters: v0: core::option::Option::<core::integer::u32>
110+
blk0 (root):
65111
Statements:
66112
End:
67113
Match(match core::panics::unsafe_panic() {
68114
})
69115

70-
blk3:
116+
blk1:
117+
Statements:
118+
(v3: core::never) <- core::panic_with_const_felt252::<435711799154>()
119+
End:
120+
Match(match_enum(v3) {
121+
})
122+
123+
blk2:
71124
Statements:
125+
(v4: core::never) <- core::panic_with_const_felt252::<29721761890975875353235833581453094220424382983267374>()
72126
End:
73-
Return(v5)
127+
Match(match_enum(v4) {
128+
})
74129

75130
//! > ==========================================================================
76131

77-
//! > Test panic in root block.
132+
//! > Test that a side-effecting statement (debug::print) before the dead tail is preserved.
78133

79134
//! > test_runner_name
80135
test_early_unsafe_panic
81136

82137
//! > function_code
138+
use core::debug::PrintTrait;
139+
83140
fn foo(a: Option<u32>) -> u32 {
84-
a.unwrap();
141+
'error'.print();
85142
core::panic_with_felt252('error')
86143
}
87144

@@ -96,17 +153,23 @@ foo
96153
Parameters: v0: core::option::Option::<core::integer::u32>
97154
blk0 (root):
98155
Statements:
99-
(v1: core::integer::u32) <- core::option::OptionTraitImpl::<core::integer::u32>::unwrap(v0)
100-
(v2: core::felt252) <- 435711799154
101-
(v3: core::never) <- core::panic_with_felt252(v2)
156+
(v1: core::felt252) <- 435711799154
157+
(v2: core::array::Array::<core::felt252>) <- core::array::array_new::<core::felt252>()
158+
(v3: core::array::Array::<core::felt252>) <- core::array::array_append::<core::felt252>(v2, v1)
159+
() <- core::debug::print(v3)
160+
(v4: core::never) <- core::panic_with_const_felt252::<435711799154>()
102161
End:
103-
Match(match_enum(v3) {
162+
Match(match_enum(v4) {
104163
})
105164

106165
//! > after
107166
Parameters: v0: core::option::Option::<core::integer::u32>
108167
blk0 (root):
109168
Statements:
169+
(v1: core::felt252) <- 435711799154
170+
(v2: core::array::Array::<core::felt252>) <- core::array::array_new::<core::felt252>()
171+
(v3: core::array::Array::<core::felt252>) <- core::array::array_append::<core::felt252>(v2, v1)
172+
() <- core::debug::print(v3)
110173
End:
111174
Match(match core::panics::unsafe_panic() {
112175
})

0 commit comments

Comments
 (0)