From 11152d4b5f0df69eaa52d09dc99d22f4bf392fe1 Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Mon, 16 Feb 2026 00:21:05 +0900 Subject: [PATCH 1/9] Add compile_bool_op_inner and optimize nested opposite-operator BoolOps to avoid redundant __bool__ calls When a nested BoolOp has the opposite operator (e.g., `And` inside `Or`), the inner BoolOp's short-circuit exits are redirected to skip the outer BoolOp's redundant truth test. This avoids calling `__bool__()` twice on the same value (e.g., `Test() and False or False` previously called `Test().__bool__()` twice instead of once). Co-Authored-By: Claude Opus 4.6 --- crates/codegen/src/compile.rs | 86 +++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index a7672e9a472..032ebe211e8 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -6616,6 +6616,44 @@ impl Compiler { let (last_value, values) = values.split_last().unwrap(); for value in values { + // 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 let ast::Expr::BoolOp(ast::ExprBoolOp { + op: inner_op, + values: inner_values, + .. + }) = value + { + if inner_op != op { + let pop_block = self.new_block(); + self.compile_bool_op_inner(inner_op, inner_values, Some(pop_block))?; + // Test the inner result for the outer BoolOp + 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, + } + ); + } + } + self.switch_to_block(pop_block); + emit!(self, Instruction::PopTop); + continue; + } + } + self.compile_expression(value)?; emit!(self, Instruction::Copy { index: 1_u32 }); @@ -6647,6 +6685,54 @@ impl Compiler { Ok(()) } + /// 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_inner( + &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 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, + } + ); + } + ast::BoolOp::Or => { + emit!( + self, + Instruction::PopJumpIfTrue { + target, + } + ); + } + } + + emit!(self, Instruction::PopTop); + } + + // If all values did not qualify, take the value of the last value: + self.compile_expression(last_value)?; + self.switch_to_block(after_block); + Ok(()) + } + fn compile_dict(&mut self, items: &[ast::DictItem]) -> CompileResult<()> { let has_unpacking = items.iter().any(|item| item.key.is_none()); From 9a2d780903bcac329414b71a69e4365ecc388bdb Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Mon, 16 Feb 2026 00:24:18 +0900 Subject: [PATCH 2/9] Add snapshot test for nested BoolOp bytecode Co-Authored-By: Claude Opus 4.6 --- crates/codegen/src/compile.rs | 9 +++++++++ ...degen__compile__tests__nested_bool_op.snap | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 032ebe211e8..b0cd53d212b 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -9092,6 +9092,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 From d8ccc98fb5a47d7a7cbfcfe1dced1f1bf6307638 Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Mon, 16 Feb 2026 00:24:19 +0900 Subject: [PATCH 3/9] Add runtime test for redundant __bool__ check (issue #3567) Co-Authored-By: Claude Opus 4.6 --- extra_tests/snippets/builtin_bool.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/extra_tests/snippets/builtin_bool.py b/extra_tests/snippets/builtin_bool.py index 902ed0cced0..c52c2ad09d9 100644 --- a/extra_tests/snippets/builtin_bool.py +++ b/extra_tests/snippets/builtin_bool.py @@ -207,3 +207,13 @@ def __len__(self): with assert_raises(TypeError): bool(TestLenThrowError()) + +# Issue #3567: nested BoolOps should not call __bool__ redundantly +_n = 0 +class _BoolCounter: + def __bool__(self): + global _n + _n += 1 + return False +_BoolCounter() and False or False +assert _n == 1, f"__bool__ called {_n} times, expected 1" From 66cab2ca6bff97ff5b4e9e59c9dc1e45a5d3515a Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Mon, 16 Feb 2026 22:31:02 +0900 Subject: [PATCH 4/9] Apply clippy and rustfmt --- crates/codegen/src/compile.rs | 63 +++++++++++++++-------------------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index b0cd53d212b..681ce950671 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -6624,34 +6624,33 @@ impl Compiler { values: inner_values, .. }) = value + && inner_op != op { - if inner_op != op { - let pop_block = self.new_block(); - self.compile_bool_op_inner(inner_op, inner_values, Some(pop_block))?; - // Test the inner result for the outer BoolOp - 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, - } - ); - } + let pop_block = self.new_block(); + self.compile_bool_op_inner(inner_op, inner_values, Some(pop_block))?; + // Test the inner result for the outer BoolOp + 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, + } + ); } - self.switch_to_block(pop_block); - emit!(self, Instruction::PopTop); - continue; } + self.switch_to_block(pop_block); + emit!(self, Instruction::PopTop); + continue; } self.compile_expression(value)?; @@ -6707,20 +6706,10 @@ impl Compiler { emit!(self, Instruction::Copy { index: 1_u32 }); match op { ast::BoolOp::And => { - emit!( - self, - Instruction::PopJumpIfFalse { - target, - } - ); + emit!(self, Instruction::PopJumpIfFalse { target }); } ast::BoolOp::Or => { - emit!( - self, - Instruction::PopJumpIfTrue { - target, - } - ); + emit!(self, Instruction::PopJumpIfTrue { target }); } } From a500d54314fe934f9c9801d050c695889ee983e5 Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Tue, 17 Feb 2026 04:02:08 +0900 Subject: [PATCH 5/9] Apply ruff format --- extra_tests/snippets/builtin_bool.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extra_tests/snippets/builtin_bool.py b/extra_tests/snippets/builtin_bool.py index c52c2ad09d9..ad5f51d78ea 100644 --- a/extra_tests/snippets/builtin_bool.py +++ b/extra_tests/snippets/builtin_bool.py @@ -210,10 +210,14 @@ def __len__(self): # Issue #3567: nested BoolOps should not call __bool__ redundantly _n = 0 + + class _BoolCounter: def __bool__(self): global _n _n += 1 return False + + _BoolCounter() and False or False assert _n == 1, f"__bool__ called {_n} times, expected 1" From 35706f1661bc772d6d42d7a87b8a1d85cd5ee2db Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Tue, 17 Feb 2026 03:58:24 +0900 Subject: [PATCH 6/9] 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 --- crates/codegen/src/compile.rs | 115 ++++++++++------------------------ 1 file changed, 33 insertions(+), 82 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 681ce950671..205facd65bd 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -6611,70 +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 { // 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 let ast::Expr::BoolOp(ast::ExprBoolOp { - op: inner_op, - values: inner_values, - .. - }) = value + 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_inner(inner_op, inner_values, Some(pop_block))?; - // Test the inner result for the outer BoolOp - 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, - } - ); - } - } + 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)?; - - 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, - } - ); - } - } - + self.emit_short_circuit_test(op, jump_target); emit!(self, Instruction::PopTop); } @@ -6684,42 +6659,18 @@ impl Compiler { Ok(()) } - /// 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_inner( - &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 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 }); - } - ast::BoolOp::Or => { - emit!(self, Instruction::PopJumpIfTrue { target }); - } + /// 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 }); } - - emit!(self, Instruction::PopTop); } - - // If all values did not qualify, take the value of the last value: - self.compile_expression(last_value)?; - self.switch_to_block(after_block); - Ok(()) } fn compile_dict(&mut self, items: &[ast::DictItem]) -> CompileResult<()> { From e68c01824144b43b398e0c3b740acb2a77872240 Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Tue, 17 Feb 2026 13:22:56 +0900 Subject: [PATCH 7/9] Relocate redundant __bool__ check test snippet --- extra_tests/snippets/builtin_bool.py | 14 -------------- extra_tests/snippets/syntax_short_circuit_bool.py | 3 +++ 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/extra_tests/snippets/builtin_bool.py b/extra_tests/snippets/builtin_bool.py index ad5f51d78ea..902ed0cced0 100644 --- a/extra_tests/snippets/builtin_bool.py +++ b/extra_tests/snippets/builtin_bool.py @@ -207,17 +207,3 @@ def __len__(self): with assert_raises(TypeError): bool(TestLenThrowError()) - -# Issue #3567: nested BoolOps should not call __bool__ redundantly -_n = 0 - - -class _BoolCounter: - def __bool__(self): - global _n - _n += 1 - return False - - -_BoolCounter() and False or False -assert _n == 1, f"__bool__ called {_n} times, expected 1" diff --git a/extra_tests/snippets/syntax_short_circuit_bool.py b/extra_tests/snippets/syntax_short_circuit_bool.py index 76d89352cbb..5cf20fbc6dd 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 +ExplodingBool(False) and False or False From dbf95a321bc3c1790c3459c4b9d973d95f82dc6a Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Tue, 17 Feb 2026 21:36:56 +0900 Subject: [PATCH 8/9] Update extra_tests/snippets/syntax_short_circuit_bool.py --- extra_tests/snippets/syntax_short_circuit_bool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extra_tests/snippets/syntax_short_circuit_bool.py b/extra_tests/snippets/syntax_short_circuit_bool.py index 5cf20fbc6dd..edb9f15c662 100644 --- a/extra_tests/snippets/syntax_short_circuit_bool.py +++ b/extra_tests/snippets/syntax_short_circuit_bool.py @@ -33,4 +33,4 @@ def __bool__(self): # pass # Issue #3567: nested BoolOps should not call __bool__ redundantly -ExplodingBool(False) and False or False +assert ExplodingBool(False) and False or False From d6e48ff446117f4be494a1b44d39d7b9a61f456b Mon Sep 17 00:00:00 2001 From: Lee Dogeon Date: Tue, 17 Feb 2026 21:43:56 +0900 Subject: [PATCH 9/9] Fix assertion in syntax_short_circuit_bool --- extra_tests/snippets/syntax_short_circuit_bool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extra_tests/snippets/syntax_short_circuit_bool.py b/extra_tests/snippets/syntax_short_circuit_bool.py index edb9f15c662..6cbae190cae 100644 --- a/extra_tests/snippets/syntax_short_circuit_bool.py +++ b/extra_tests/snippets/syntax_short_circuit_bool.py @@ -33,4 +33,4 @@ def __bool__(self): # pass # Issue #3567: nested BoolOps should not call __bool__ redundantly -assert ExplodingBool(False) and False or False +assert (ExplodingBool(False) and False or False) == False