mirror of
https://github.com/RustPython/RustPython.git
synced 2026-06-02 19:39:49 +09:00
Optimize redundant bool check (#7176)
* 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 <noreply@anthropic.com> * Add snapshot test for nested BoolOp bytecode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add runtime test for redundant __bool__ check (issue #3567) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Apply clippy and rustfmt * Apply ruff format * 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> * Relocate redundant __bool__ check test snippet * Update extra_tests/snippets/syntax_short_circuit_bool.py * Fix assertion in syntax_short_circuit_bool --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jeong, YunWon <69878+youknowone@users.noreply.github.com>
This commit is contained in:
@@ -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<BlockIdx>,
|
||||
) -> 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(
|
||||
|
||||
20
crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap
generated
Normal file
20
crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap
generated
Normal file
@@ -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
|
||||
@@ -31,3 +31,6 @@ while ExplodingBool(False) and False:
|
||||
|
||||
# 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
|
||||
|
||||
Reference in New Issue
Block a user