Skip to content

Commit 0ec4349

Browse files
committed
fixsing
1 parent 5293d8e commit 0ec4349

File tree

3 files changed

+162
-52
lines changed

3 files changed

+162
-52
lines changed

crates/codegen/src/compile.rs

Lines changed: 152 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,18 +1387,16 @@ impl Compiler {
13871387
}
13881388

13891389
FBlockType::FinallyEnd => {
1390-
// Stack when in FinallyEnd: [..., prev_exc, exc] or
1391-
// [..., prev_exc, exc, return_value] if preserve_tos
1392-
// Note: No lasti here - it's only pushed for cleanup handler exceptions
1393-
// We need to pop: exc, prev_exc (via PopExcept)
1390+
// codegen_unwind_fblock(FINALLY_END)
13941391
if preserve_tos {
13951392
emit!(self, Instruction::Swap { index: 2 });
13961393
}
1397-
emit!(self, Instruction::PopTop); // exc
1394+
emit!(self, Instruction::PopTop); // exc_value
13981395
if preserve_tos {
13991396
emit!(self, Instruction::Swap { index: 2 });
14001397
}
1401-
emit!(self, Instruction::PopExcept); // prev_exc is restored
1398+
emit!(self, PseudoInstruction::PopBlock);
1399+
emit!(self, Instruction::PopExcept);
14021400
}
14031401

14041402
FBlockType::With | FBlockType::AsyncWith => {
@@ -1435,9 +1433,16 @@ impl Compiler {
14351433
}
14361434

14371435
FBlockType::HandlerCleanup => {
1436+
// codegen_unwind_fblock(HANDLER_CLEANUP)
1437+
if let FBlockDatum::ExceptionName(_) = info.fb_datum {
1438+
// Named handler: PopBlock for inner SETUP_CLEANUP
1439+
emit!(self, PseudoInstruction::PopBlock);
1440+
}
14381441
if preserve_tos {
14391442
emit!(self, Instruction::Swap { index: 2 });
14401443
}
1444+
// PopBlock for outer SETUP_CLEANUP (ExceptionHandler)
1445+
emit!(self, PseudoInstruction::PopBlock);
14411446
emit!(self, Instruction::PopExcept);
14421447

14431448
// If there's an exception name, clean it up
@@ -1511,6 +1516,9 @@ impl Compiler {
15111516
self.unwind_fblock(&fblock_info, preserve_tos)?;
15121517
}
15131518
UnwindInfo::FinallyTry { body, fblock_idx } => {
1519+
// codegen_unwind_fblock(FINALLY_TRY)
1520+
emit!(self, PseudoInstruction::PopBlock);
1521+
15141522
// Temporarily remove the FinallyTry fblock so nested return/break/continue
15151523
// in the finally body won't see it again
15161524
let code = self.current_code_info();
@@ -2801,15 +2809,12 @@ impl Compiler {
28012809
}
28022810

28032811
self.switch_to_block(finally_except);
2804-
// PUSH_EXC_INFO first, THEN push FinallyEnd fblock
2805-
// Stack after unwind (no lasti): [exc] (depth = current_depth + 1)
2806-
// Stack after PUSH_EXC_INFO: [prev_exc, exc] (depth = current_depth + 2)
2807-
emit!(self, Instruction::PushExcInfo);
2812+
// SETUP_CLEANUP before PUSH_EXC_INFO
28082813
if let Some(cleanup) = finally_cleanup_block {
2809-
// FinallyEnd fblock must be pushed AFTER PUSH_EXC_INFO
2810-
// Depth = current_depth + 1 (only prev_exc remains after RERAISE pops exc)
2811-
// Exception table: L4 to L5 -> L6 [2] lasti (cleanup handler DOES push lasti)
28122814
emit!(self, PseudoInstruction::SetupCleanup { target: cleanup });
2815+
}
2816+
emit!(self, Instruction::PushExcInfo);
2817+
if let Some(cleanup) = finally_cleanup_block {
28132818
self.push_fblock(FBlockType::FinallyEnd, cleanup, cleanup)?;
28142819
}
28152820
self.compile_statements(finalbody)?;
@@ -3172,7 +3177,12 @@ impl Compiler {
31723177
let end_block = self.new_block();
31733178
let reraise_star_block = self.new_block();
31743179
let reraise_block = self.new_block();
3175-
let _cleanup_block = self.new_block();
3180+
let finally_cleanup_block = if !finalbody.is_empty() {
3181+
Some(self.new_block())
3182+
} else {
3183+
None
3184+
};
3185+
let exit_block = self.new_block();
31763186

31773187
// Push fblock with handler info for exception table generation
31783188
if !finalbody.is_empty() {
@@ -3182,7 +3192,12 @@ impl Compiler {
31823192
target: finally_block
31833193
}
31843194
);
3185-
self.push_fblock(FBlockType::FinallyTry, finally_block, finally_block)?;
3195+
self.push_fblock_full(
3196+
FBlockType::FinallyTry,
3197+
finally_block,
3198+
finally_block,
3199+
FBlockDatum::FinallyBody(finalbody.to_vec()),
3200+
)?;
31863201
}
31873202

31883203
// SETUP_FINALLY for try body
@@ -3211,7 +3226,26 @@ impl Compiler {
32113226
let eg_dummy2 = self.new_block();
32123227
self.push_fblock(FBlockType::ExceptionGroupHandler, eg_dummy1, eg_dummy2)?;
32133228

3229+
// Initialize handler stack before the loop
3230+
// BUILD_LIST 0 + COPY 2 to set up [prev_exc, orig, list, rest]
3231+
emit!(self, Instruction::BuildList { size: 0 });
3232+
// Stack: [prev_exc, exc, []]
3233+
emit!(self, Instruction::Copy { index: 2 });
3234+
// Stack: [prev_exc, orig, list, rest]
3235+
32143236
let n = handlers.len();
3237+
if n == 0 {
3238+
// Empty handlers (invalid AST) - append rest to list and proceed
3239+
// Stack: [prev_exc, orig, list, rest]
3240+
emit!(self, Instruction::ListAppend { i: 0 });
3241+
// Stack: [prev_exc, orig, list]
3242+
emit!(
3243+
self,
3244+
PseudoInstruction::Jump {
3245+
target: reraise_star_block
3246+
}
3247+
);
3248+
}
32153249
for (i, handler) in handlers.iter().enumerate() {
32163250
let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
32173251
type_,
@@ -3223,17 +3257,6 @@ impl Compiler {
32233257
let no_match_block = self.new_block();
32243258
let next_block = self.new_block();
32253259

3226-
// first handler creates list and copies exc
3227-
if i == 0 {
3228-
// ADDOP_I(c, loc, BUILD_LIST, 0);
3229-
emit!(self, Instruction::BuildList { size: 0 });
3230-
// Stack: [prev_exc, exc, []]
3231-
// ADDOP_I(c, loc, COPY, 2);
3232-
emit!(self, Instruction::Copy { index: 2 });
3233-
// Stack: [prev_exc, exc, [], exc_copy]
3234-
// Now stack is: [prev_exc, orig, list, rest]
3235-
}
3236-
32373260
// Compile exception type
32383261
if let Some(exc_type) = type_ {
32393262
// Check for unparenthesized tuple
@@ -3438,7 +3461,12 @@ impl Compiler {
34383461
target: finally_block
34393462
}
34403463
);
3441-
self.push_fblock(FBlockType::FinallyTry, finally_block, finally_block)?;
3464+
self.push_fblock_full(
3465+
FBlockType::FinallyTry,
3466+
finally_block,
3467+
finally_block,
3468+
FBlockDatum::FinallyBody(finalbody.to_vec()),
3469+
)?;
34423470
}
34433471
self.switch_to_block(else_block);
34443472
self.compile_statements(orelse)?;
@@ -3453,11 +3481,60 @@ impl Compiler {
34533481

34543482
self.switch_to_block(end_block);
34553483
if !finalbody.is_empty() {
3484+
// Snapshot sub_tables before first finally compilation
3485+
let sub_table_cursor = self.symbol_table_stack.last().map(|t| t.next_sub_table);
3486+
3487+
// Compile finally body inline for normal path
3488+
self.compile_statements(finalbody)?;
3489+
emit!(self, PseudoInstruction::Jump { target: exit_block });
3490+
3491+
// Restore sub_tables for exception path compilation
3492+
if let Some(cursor) = sub_table_cursor
3493+
&& let Some(current_table) = self.symbol_table_stack.last_mut()
3494+
{
3495+
current_table.next_sub_table = cursor;
3496+
}
3497+
3498+
// Exception handler path
34563499
self.switch_to_block(finally_block);
3500+
emit!(self, Instruction::PushExcInfo);
3501+
3502+
if let Some(cleanup) = finally_cleanup_block {
3503+
emit!(self, PseudoInstruction::SetupCleanup { target: cleanup });
3504+
self.push_fblock(FBlockType::FinallyEnd, cleanup, cleanup)?;
3505+
}
3506+
34573507
self.compile_statements(finalbody)?;
3458-
// No EndFinally emit - exception table handles this
3508+
3509+
if finally_cleanup_block.is_some() {
3510+
emit!(self, PseudoInstruction::PopBlock);
3511+
self.pop_fblock(FBlockType::FinallyEnd);
3512+
}
3513+
3514+
emit!(self, Instruction::Copy { index: 2_u32 });
3515+
emit!(self, Instruction::PopExcept);
3516+
emit!(
3517+
self,
3518+
Instruction::RaiseVarargs {
3519+
kind: bytecode::RaiseKind::ReraiseFromStack
3520+
}
3521+
);
3522+
3523+
if let Some(cleanup) = finally_cleanup_block {
3524+
self.switch_to_block(cleanup);
3525+
emit!(self, Instruction::Copy { index: 3_u32 });
3526+
emit!(self, Instruction::PopExcept);
3527+
emit!(
3528+
self,
3529+
Instruction::RaiseVarargs {
3530+
kind: bytecode::RaiseKind::ReraiseFromStack
3531+
}
3532+
);
3533+
}
34593534
}
34603535

3536+
self.switch_to_block(exit_block);
3537+
34613538
Ok(())
34623539
}
34633540

@@ -4866,16 +4943,19 @@ impl Compiler {
48664943

48674944
self.switch_to_block(for_block);
48684945

4869-
// Push fblock for async for loop
4870-
// Note: SetupExcept is no longer emitted (exception table handles StopAsyncIteration)
4871-
emit!(self, PseudoInstruction::SetupFinally { target: else_block });
4946+
// codegen_async_for: push fblock BEFORE SETUP_FINALLY
48724947
self.push_fblock(FBlockType::ForLoop, for_block, after_block)?;
48734948

4949+
// SETUP_FINALLY to guard the __anext__ call
4950+
emit!(self, PseudoInstruction::SetupFinally { target: else_block });
48744951
emit!(self, Instruction::GetANext);
48754952
self.emit_load_const(ConstantData::None);
48764953
self.compile_yield_from_sequence(true)?;
4954+
// POP_BLOCK for SETUP_FINALLY - only GetANext/yield_from are protected
4955+
emit!(self, PseudoInstruction::PopBlock);
4956+
4957+
// Success block for __anext__
48774958
self.compile_store(target)?;
4878-
// Note: PopBlock is no longer emitted (exception table handles this)
48794959
} else {
48804960
// Retrieve Iterator
48814961
emit!(self, Instruction::GetIter);
@@ -4898,10 +4978,8 @@ impl Compiler {
48984978

48994979
self.switch_to_block(else_block);
49004980

4901-
// Pop fblock (PopBlock only for async for which has SETUP_FINALLY)
4902-
if is_async {
4903-
emit!(self, PseudoInstruction::PopBlock);
4904-
}
4981+
// Except block for __anext__ / end of sync for
4982+
// No PopBlock here - for async, POP_BLOCK is already in for_block
49054983
self.pop_fblock(FBlockType::ForLoop);
49064984

49074985
if is_async {
@@ -7798,7 +7876,10 @@ impl Compiler {
77987876
With {
77997877
is_async: bool,
78007878
},
7801-
HandlerCleanup,
7879+
HandlerCleanup {
7880+
name: Option<String>,
7881+
},
7882+
TryExcept,
78027883
FinallyTry {
78037884
body: Vec<ruff_python_ast::Stmt>,
78047885
fblock_idx: usize,
@@ -7819,7 +7900,14 @@ impl Compiler {
78197900
unwind_actions.push(UnwindAction::With { is_async: true });
78207901
}
78217902
FBlockType::HandlerCleanup => {
7822-
unwind_actions.push(UnwindAction::HandlerCleanup);
7903+
let name = match &code.fblock[i].fb_datum {
7904+
FBlockDatum::ExceptionName(name) => Some(name.clone()),
7905+
_ => None,
7906+
};
7907+
unwind_actions.push(UnwindAction::HandlerCleanup { name });
7908+
}
7909+
FBlockType::TryExcept => {
7910+
unwind_actions.push(UnwindAction::TryExcept);
78237911
}
78247912
FBlockType::FinallyTry => {
78257913
// Need to execute finally body before break/continue
@@ -7847,6 +7935,8 @@ impl Compiler {
78477935
for action in unwind_actions {
78487936
match action {
78497937
UnwindAction::With { is_async } => {
7938+
// codegen_unwind_fblock(WITH/ASYNC_WITH)
7939+
emit!(self, PseudoInstruction::PopBlock);
78507940
// compiler_call_exit_with_nones
78517941
emit!(self, Instruction::PushNull);
78527942
self.emit_load_const(ConstantData::None);
@@ -7862,10 +7952,29 @@ impl Compiler {
78627952

78637953
emit!(self, Instruction::PopTop);
78647954
}
7865-
UnwindAction::HandlerCleanup => {
7955+
UnwindAction::HandlerCleanup { ref name } => {
7956+
// codegen_unwind_fblock(HANDLER_CLEANUP)
7957+
if name.is_some() {
7958+
// Named handler: PopBlock for inner SETUP_CLEANUP
7959+
emit!(self, PseudoInstruction::PopBlock);
7960+
}
7961+
// PopBlock for outer SETUP_CLEANUP (ExceptionHandler)
7962+
emit!(self, PseudoInstruction::PopBlock);
78667963
emit!(self, Instruction::PopExcept);
7964+
if let Some(name) = name {
7965+
self.emit_load_const(ConstantData::None);
7966+
self.store_name(name)?;
7967+
self.compile_name(name, NameUsage::Delete)?;
7968+
}
7969+
}
7970+
UnwindAction::TryExcept => {
7971+
// codegen_unwind_fblock(TRY_EXCEPT)
7972+
emit!(self, PseudoInstruction::PopBlock);
78677973
}
78687974
UnwindAction::FinallyTry { body, fblock_idx } => {
7975+
// codegen_unwind_fblock(FINALLY_TRY)
7976+
emit!(self, PseudoInstruction::PopBlock);
7977+
78697978
// compile finally body inline
78707979
// Temporarily pop the FinallyTry fblock so nested break/continue
78717980
// in the finally body won't see it again.
@@ -7880,11 +7989,10 @@ impl Compiler {
78807989
code.fblock.insert(fblock_idx, saved_fblock);
78817990
}
78827991
UnwindAction::FinallyEnd => {
7883-
// Stack when in FinallyEnd: [..., prev_exc, exc]
7884-
// Note: No lasti here - it's only pushed for cleanup handler exceptions
7885-
// We need to pop: exc, prev_exc (via PopExcept)
7886-
emit!(self, Instruction::PopTop); // exc
7887-
emit!(self, Instruction::PopExcept); // prev_exc is restored
7992+
// codegen_unwind_fblock(FINALLY_END)
7993+
emit!(self, Instruction::PopTop); // exc_value
7994+
emit!(self, PseudoInstruction::PopBlock);
7995+
emit!(self, Instruction::PopExcept);
78887996
}
78897997
UnwindAction::PopValue => {
78907998
// Pop the return value - continue/break cancels the pending return

crates/codegen/src/ir.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,8 +1342,12 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
13421342
| PseudoInstruction::SetupWith { .. } => {
13431343
info.instr = Instruction::Nop.into();
13441344
}
1345-
// POP_BLOCK → NOP (should already be NOP from label_exception_targets)
1345+
// POP_BLOCK should already be converted to NOP by label_exception_targets
13461346
PseudoInstruction::PopBlock => {
1347+
debug_assert!(
1348+
false,
1349+
"PopBlock should have been converted to NOP by label_exception_targets"
1350+
);
13471351
info.instr = Instruction::Nop.into();
13481352
}
13491353
// LOAD_CLOSURE → LOAD_FAST (with varnames offset)
@@ -1359,9 +1363,7 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
13591363
| PseudoInstruction::JumpIfFalse { .. }
13601364
| PseudoInstruction::JumpIfTrue { .. }
13611365
| PseudoInstruction::StoreFastMaybeNull(_) => {
1362-
unimplemented!(
1363-
"Unexpected pseudo instruction in convert_pseudo_ops: {pseudo:?}"
1364-
)
1366+
unreachable!("Unexpected pseudo instruction in convert_pseudo_ops: {pseudo:?}")
13651367
}
13661368
}
13671369
}

crates/compiler-core/src/bytecode/instruction.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,9 +1019,9 @@ impl InstructionMetadata for PseudoInstruction {
10191019
Self::JumpNoInterrupt { .. } => (0, 0),
10201020
Self::LoadClosure(_) => (1, 0),
10211021
Self::PopBlock => (0, 0),
1022-
Self::SetupCleanup => (2, 0),
1023-
Self::SetupFinally => (1, 0),
1024-
Self::SetupWith => (1, 0),
1022+
Self::SetupCleanup { .. } => (2, 0),
1023+
Self::SetupFinally { .. } => (1, 0),
1024+
Self::SetupWith { .. } => (1, 0),
10251025
Self::StoreFastMaybeNull(_) => (0, 1),
10261026
};
10271027

@@ -1034,7 +1034,7 @@ impl InstructionMetadata for PseudoInstruction {
10341034
fn is_unconditional_jump(&self) -> bool {
10351035
matches!(self, Self::Jump { .. } | Self::JumpNoInterrupt { .. })
10361036
}
1037-
fn stack_effect(&self, _arg: OpArg) -> i32 {
1037+
fn stack_effect(&self, _arg: u32) -> i32 {
10381038
match self {
10391039
Self::AnnotationsPlaceholder => 0,
10401040
Self::Jump { .. } => 0,

0 commit comments

Comments
 (0)