diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index a7672e9a472..205facd65bd 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -6611,33 +6611,45 @@ impl Compiler { /// Compile a boolean operation as an expression. /// This means, that the last value remains on the stack. fn compile_bool_op(&mut self, op: &ast::BoolOp, values: &[ast::Expr]) -> CompileResult<()> { - let after_block = self.new_block(); + self.compile_bool_op_with_target(op, values, None) + } + /// Compile a boolean operation as an expression, with an optional + /// short-circuit target override. When `short_circuit_target` is `Some`, + /// the short-circuit jumps go to that block instead of the default + /// `after_block`, enabling jump threading to avoid redundant `__bool__` calls. + fn compile_bool_op_with_target( + &mut self, + op: &ast::BoolOp, + values: &[ast::Expr], + short_circuit_target: Option, + ) -> CompileResult<()> { + let after_block = self.new_block(); let (last_value, values) = values.split_last().unwrap(); + let jump_target = short_circuit_target.unwrap_or(after_block); for value in values { - self.compile_expression(value)?; - - emit!(self, Instruction::Copy { index: 1_u32 }); - match op { - ast::BoolOp::And => { - emit!( - self, - Instruction::PopJumpIfFalse { - target: after_block, - } - ); - } - ast::BoolOp::Or => { - emit!( - self, - Instruction::PopJumpIfTrue { - target: after_block, - } - ); - } + // Optimization: when a non-last value is a BoolOp with the opposite + // operator, redirect its short-circuit exits to skip the outer's + // redundant __bool__ test (jump threading). + if short_circuit_target.is_none() + && let ast::Expr::BoolOp(ast::ExprBoolOp { + op: inner_op, + values: inner_values, + .. + }) = value + && inner_op != op + { + let pop_block = self.new_block(); + self.compile_bool_op_with_target(inner_op, inner_values, Some(pop_block))?; + self.emit_short_circuit_test(op, after_block); + self.switch_to_block(pop_block); + emit!(self, Instruction::PopTop); + continue; } + self.compile_expression(value)?; + self.emit_short_circuit_test(op, jump_target); emit!(self, Instruction::PopTop); } @@ -6647,6 +6659,20 @@ impl Compiler { Ok(()) } + /// Emit `Copy 1` + conditional jump for short-circuit evaluation. + /// For `And`, emits `PopJumpIfFalse`; for `Or`, emits `PopJumpIfTrue`. + fn emit_short_circuit_test(&mut self, op: &ast::BoolOp, target: BlockIdx) { + emit!(self, Instruction::Copy { index: 1_u32 }); + match op { + ast::BoolOp::And => { + emit!(self, Instruction::PopJumpIfFalse { target }); + } + ast::BoolOp::Or => { + emit!(self, Instruction::PopJumpIfTrue { target }); + } + } + } + fn compile_dict(&mut self, items: &[ast::DictItem]) -> CompileResult<()> { let has_unpacking = items.iter().any(|item| item.key.is_none()); @@ -9006,6 +9032,15 @@ if (True and False) or (False and True): )); } + #[test] + fn test_nested_bool_op() { + assert_dis_snapshot!(compile_exec( + "\ +x = Test() and False or False +" + )); + } + #[test] fn test_nested_double_async_with() { assert_dis_snapshot!(compile_exec( diff --git a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap new file mode 100644 index 00000000000..5b9a2182bdf --- /dev/null +++ b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap @@ -0,0 +1,20 @@ +--- +source: crates/codegen/src/compile.rs +assertion_line: 9071 +expression: "compile_exec(\"\\\nx = Test() and False or False\n\")" +--- + 1 0 RESUME (0) + 1 LOAD_NAME (0, Test) + 2 PUSH_NULL + 3 CALL (0) + 4 COPY (1) + 5 POP_JUMP_IF_FALSE (10) + 6 POP_TOP + 7 LOAD_CONST (False) + 8 COPY (1) + 9 POP_JUMP_IF_TRUE (12) + >> 10 POP_TOP + 11 LOAD_CONST (False) + >> 12 STORE_NAME (1, x) + 13 LOAD_CONST (None) + 14 RETURN_VALUE diff --git a/extra_tests/snippets/syntax_short_circuit_bool.py b/extra_tests/snippets/syntax_short_circuit_bool.py index 76d89352cbb..6cbae190cae 100644 --- a/extra_tests/snippets/syntax_short_circuit_bool.py +++ b/extra_tests/snippets/syntax_short_circuit_bool.py @@ -31,3 +31,6 @@ def __bool__(self): # if ExplodingBool(False) and False and True and False: # pass + +# Issue #3567: nested BoolOps should not call __bool__ redundantly +assert (ExplodingBool(False) and False or False) == False