Skip to content

Commit 0fe34f5

Browse files
morealclaude
andcommitted
Refactor compile_bool_op: extract emit_short_circuit_test and unify with compile_bool_op_inner
Reduce code duplication by: - Extracting the repeated Copy + conditional jump pattern into emit_short_circuit_test - Merging compile_bool_op and compile_bool_op_inner into a single compile_bool_op_with_target with an optional short_circuit_target parameter - Keeping compile_bool_op as a thin wrapper for the public interface Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 20001b0 commit 0fe34f5

1 file changed

Lines changed: 36 additions & 84 deletions

File tree

‎crates/codegen/src/compile.rs‎

Lines changed: 36 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -6611,108 +6611,46 @@ impl Compiler {
66116611
/// Compile a boolean operation as an expression.
66126612
/// This means, that the last value remains on the stack.
66136613
fn compile_bool_op(&mut self, op: &ast::BoolOp, values: &[ast::Expr]) -> CompileResult<()> {
6614-
let after_block = self.new_block();
6615-
6616-
let (last_value, values) = values.split_last().unwrap();
6617-
6618-
for value in values {
6619-
// Optimization: when a non-last value is a BoolOp with the opposite
6620-
// operator, redirect its short-circuit exits to skip the outer's
6621-
// redundant __bool__ test (jump threading).
6622-
if let ast::Expr::BoolOp(ast::ExprBoolOp {
6623-
op: inner_op,
6624-
values: inner_values,
6625-
..
6626-
}) = value
6627-
&& inner_op != op
6628-
{
6629-
let pop_block = self.new_block();
6630-
self.compile_bool_op_inner(inner_op, inner_values, Some(pop_block))?;
6631-
// Test the inner result for the outer BoolOp
6632-
emit!(self, Instruction::Copy { index: 1_u32 });
6633-
match op {
6634-
ast::BoolOp::And => {
6635-
emit!(
6636-
self,
6637-
Instruction::PopJumpIfFalse {
6638-
target: after_block,
6639-
}
6640-
);
6641-
}
6642-
ast::BoolOp::Or => {
6643-
emit!(
6644-
self,
6645-
Instruction::PopJumpIfTrue {
6646-
target: after_block,
6647-
}
6648-
);
6649-
}
6650-
}
6651-
self.switch_to_block(pop_block);
6652-
emit!(self, Instruction::PopTop);
6653-
continue;
6654-
}
6655-
6656-
self.compile_expression(value)?;
6657-
6658-
emit!(self, Instruction::Copy { index: 1_u32 });
6659-
match op {
6660-
ast::BoolOp::And => {
6661-
emit!(
6662-
self,
6663-
Instruction::PopJumpIfFalse {
6664-
target: after_block,
6665-
}
6666-
);
6667-
}
6668-
ast::BoolOp::Or => {
6669-
emit!(
6670-
self,
6671-
Instruction::PopJumpIfTrue {
6672-
target: after_block,
6673-
}
6674-
);
6675-
}
6676-
}
6677-
6678-
emit!(self, Instruction::PopTop);
6679-
}
6680-
6681-
// If all values did not qualify, take the value of the last value:
6682-
self.compile_expression(last_value)?;
6683-
self.switch_to_block(after_block);
6684-
Ok(())
6614+
self.compile_bool_op_with_target(op, values, None)
66856615
}
66866616

66876617
/// Compile a boolean operation as an expression, with an optional
66886618
/// short-circuit target override. When `short_circuit_target` is `Some`,
66896619
/// the short-circuit jumps go to that block instead of the default
66906620
/// `after_block`, enabling jump threading to avoid redundant `__bool__` calls.
6691-
fn compile_bool_op_inner(
6621+
fn compile_bool_op_with_target(
66926622
&mut self,
66936623
op: &ast::BoolOp,
66946624
values: &[ast::Expr],
66956625
short_circuit_target: Option<BlockIdx>,
66966626
) -> CompileResult<()> {
66976627
let after_block = self.new_block();
6698-
66996628
let (last_value, values) = values.split_last().unwrap();
6700-
6701-
let class=pl-kos>.unwrap_or(after_block);
6629+
let jump_target = short_circuit_target.unwrap_or(after_block);
67026630

67036631
for value in values {
6704-
self.compile_expression(value)?;
6705-
6706-
emit!(self, Instruction::Copy { index: 1_u32 });
6707-
match op {
6708-
ast::BoolOp::And => {
6709-
emit!(self, Instruction::PopJumpIfFalse { target });
6710-
}
6711-
ast::BoolOp::Or => {
6712-
emit!(self, Instruction::PopJumpIfTrue { target });
6632+
// Optimization: when a non-last value is a BoolOp with the opposite
6633+
// operator, redirect its short-circuit exits to skip the outer's
6634+
// redundant __bool__ test (jump threading).
6635+
if short_circuit_target.is_none() {
6636+
if let ast::Expr::BoolOp(ast::ExprBoolOp {
6637+
op: inner_op,
6638+
values: inner_values,
6639+
..
6640+
}) = value
6641+
&& inner_op != op
6642+
{
6643+
let pop_block = self.new_block();
6644+
self.compile_bool_op_with_target(inner_op, inner_values, Some(pop_block))?;
6645+
self.emit_short_circuit_test(op, after_block);
6646+
self.switch_to_block(pop_block);
6647+
emit!(self, Instruction::PopTop);
6648+
continue;
67136649
}
67146650
}
67156651

6652+
self.compile_expression(value)?;
6653+
self.emit_short_circuit_test(op, jump_target);
67166654
emit!(self, Instruction::PopTop);
67176655
}
67186656

@@ -6722,6 +6660,20 @@ impl Compiler {
67226660
Ok(())
67236661
}
67246662

6663+
/// Emit `Copy 1` + conditional jump for short-circuit evaluation.
6664+
/// For `And`, emits `PopJumpIfFalse`; for `Or`, emits `PopJumpIfTrue`.
6665+
fn emit_short_circuit_test(&mut self, op: &ast::BoolOp, target: BlockIdx) {
6666+
emit!(self, Instruction::Copy { index: 1_u32 });
6667+
match op {
6668+
ast::BoolOp::And => {
6669+
emit!(self, Instruction::PopJumpIfFalse { target });
6670+
}
6671+
ast::BoolOp::Or => {
6672+
emit!(self, Instruction::PopJumpIfTrue { target });
6673+
}
6674+
}
6675+
}
6676+
67256677
fn compile_dict(&mut self, items: &[ast::DictItem]) -> CompileResult<()> {
67266678
let has_unpacking = items.iter().any(|item| item.key.is_none());
67276679

0 commit comments

Comments
 (0)