Skip to content

Split TestOperator instruction #6306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 3 commits into from
Nov 29, 2025
Merged

Split TestOperator instruction #6306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91f5965
Split `TestOperator` inatruction
ShaharNaveh Nov 28, 2025
625480d
Update snapshot
ShaharNaveh Nov 28, 2025
c323c6c
Set as have label
ShaharNaveh Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 10 additions & 38 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use rustpython_compiler_core::{
Mode, OneIndexed, PositionEncoding, SourceFile, SourceLocation,
bytecode::{
self, Arg as OpArgMarker, BinaryOperator, CodeObject, ComparisonOperator, ConstantData,
Instruction, OpArg, OpArgType, UnpackExArgs,
Instruction, Invert, OpArg, OpArgType, UnpackExArgs,
},
};
use rustpython_wtf8::Wtf8Buf;
Expand Down Expand Up @@ -2034,20 +2034,7 @@ impl Compiler {

// Check exception type:
self.compile_expression(exc_type)?;
emit!(
self,
Instruction::TestOperation {
op: bytecode::TestOperator::ExceptionMatch,
}
);

// We cannot handle this exception type:
emit!(
self,
Instruction::PopJumpIfFalse {
target: next_handler,
}
);
emit!(self, Instruction::JumpIfNotExcMatch(next_handler));

// We have a match, store in name (except x as y)
if let Some(alias) = name {
Expand Down Expand Up @@ -3477,12 +3464,7 @@ impl Compiler {
// 4. Load None.
self.emit_load_const(ConstantData::None);
// 5. Compare with IS_OP 1.
emit!(
self,
Instruction::TestOperation {
op: bytecode::TestOperator::IsNot
}
);
emit!(self, Instruction::IsOp(Invert::Yes));

// At this point the TOS is a tuple of (nargs + n_attrs) attributes (or None).
pc.on_top += 1;
Expand Down Expand Up @@ -3648,12 +3630,8 @@ impl Compiler {

// Check if copy is None (consumes the copy like POP_JUMP_IF_NONE)
self.emit_load_const(ConstantData::None);
emit!(
self,
Instruction::TestOperation {
op: bytecode::TestOperator::IsNot
}
);
emit!(self, Instruction::IsOp(Invert::Yes));

// Stack: [subject, keys_tuple, values_tuple, bool]
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
// Stack: [subject, keys_tuple, values_tuple]
Expand Down Expand Up @@ -3948,12 +3926,7 @@ impl Compiler {
Singleton::True => ConstantData::Boolean { value: true },
});
// Compare using the "Is" operator.
emit!(
self,
Instruction::TestOperation {
op: bytecode::TestOperator::Is
}
);
emit!(self, Instruction::IsOp(Invert::No));
// Jump to the failure label if the comparison is false.
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
Ok(())
Expand Down Expand Up @@ -4082,7 +4055,6 @@ impl Compiler {
let (last_val, mid_exprs) = exprs.split_last().unwrap();

use bytecode::ComparisonOperator::*;
use bytecode::TestOperator::*;
let compile_cmpop = |c: &mut Self, op: &CmpOp| match op {
CmpOp::Eq => emit!(c, Instruction::CompareOperation { op: Equal }),
CmpOp::NotEq => emit!(c, Instruction::CompareOperation { op: NotEqual }),
Expand All @@ -4092,10 +4064,10 @@ impl Compiler {
CmpOp::GtE => {
emit!(c, Instruction::CompareOperation { op: GreaterOrEqual })
}
CmpOp::In => emit!(c, Instruction::TestOperation { op: In }),
CmpOp::NotIn => emit!(c, Instruction::TestOperation { op: NotIn }),
CmpOp::Is => emit!(c, Instruction::TestOperation { op: Is }),
CmpOp::IsNot => emit!(c, Instruction::TestOperation { op: IsNot }),
CmpOp::In => emit!(c, Instruction::ContainsOp(Invert::No)),
CmpOp::NotIn => emit!(c, Instruction::ContainsOp(Invert::Yes)),
CmpOp::Is => emit!(c, Instruction::IsOp(Invert::No)),
CmpOp::IsNot => emit!(c, Instruction::IsOp(Invert::Yes)),
};

// a == b == c == d
Expand Down
69 changes: 34 additions & 35 deletions ...s/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 33 additions & 21 deletions crates/compiler-core/src/bytecode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,10 @@ pub enum Instruction {
Subscript,
StoreSubscript,
DeleteSubscript,
/// Performs `is` comparison, or `is not` if `invert` is 1.
IsOp(Arg<Invert>),
/// Performs `in` comparison, or `not in` if `invert` is 1.
ContainsOp(Arg<Invert>),
StoreAttr {
idx: Arg<NameIdx>,
},
Expand All @@ -600,9 +604,6 @@ pub enum Instruction {
LoadAttr {
idx: Arg<NameIdx>,
},
TestOperation {
op: Arg<TestOperator>,
},
CompareOperation {
op: Arg<ComparisonOperator>,
},
Expand All @@ -628,6 +629,10 @@ pub enum Instruction {
Break {
target: Arg<Label>,
},
/// Performs exception matching for except.
/// Tests whether the STACK[-2] is an exception matching STACK[-1].
/// Pops STACK[-1] and pushes the boolean result of the test.
JumpIfNotExcMatch(Arg<Label>),
Jump {
target: Arg<Label>,
},
Expand Down Expand Up @@ -1088,19 +1093,6 @@ op_arg_enum!(
}
);

op_arg_enum!(
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[repr(u8)]
pub enum TestOperator {
In = 0,
NotIn = 1,
Is = 2,
IsNot = 3,
/// two exceptions that match?
ExceptionMatch = 4,
}
);

op_arg_enum!(
/// The possible Binary operators
/// # Examples
Expand Down Expand Up @@ -1141,6 +1133,24 @@ op_arg_enum!(
}
);

op_arg_enum!(
/// Whether or not to invert the operation.
#[repr(u8)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum Invert {
/// ```py
/// foo is bar
/// x in lst
/// ```
No = 0,
/// ```py
/// foo is not bar
/// x not in lst
/// ```
Yes = 1,
}
);

#[derive(Copy, Clone)]
pub struct UnpackExArgs {
pub before: u8,
Expand Down Expand Up @@ -1395,6 +1405,7 @@ impl Instruction {
pub const fn label_arg(&self) -> Option<Arg<Label>> {
match self {
Jump { target: l }
| JumpIfNotExcMatch(l)
| PopJumpIfTrue { target: l }
| PopJumpIfFalse { target: l }
| JumpIfTrueOrPop { target: l }
Expand Down Expand Up @@ -1462,10 +1473,7 @@ impl Instruction {
DeleteAttr { .. } => -1,
LoadConst { .. } => 1,
UnaryOperation { .. } => 0,
BinaryOperation { .. }
| BinaryOperationInplace { .. }
| TestOperation { .. }
| CompareOperation { .. } => -1,
BinaryOperation { .. } | BinaryOperationInplace { .. } | CompareOperation { .. } => -1,
BinarySubscript => -1,
CopyItem { .. } => 1,
Pop => -1,
Expand Down Expand Up @@ -1508,6 +1516,8 @@ impl Instruction {
1
}
}
IsOp(_) | ContainsOp(_) => -1,
JumpIfNotExcMatch(_) => -2,
ReturnValue => -1,
ReturnConst { .. } => 0,
Resume { .. } => 0,
Expand Down Expand Up @@ -1654,6 +1664,9 @@ impl Instruction {
DeleteGlobal(idx) => w!(DeleteGlobal, name = idx),
DeleteDeref(idx) => w!(DeleteDeref, cell_name = idx),
LoadClosure(i) => w!(LoadClosure, cell_name = i),
IsOp(inv) => w!(IS_OP, ?inv),
ContainsOp(inv) => w!(CONTAINS_OP, ?inv),
JumpIfNotExcMatch(target) => w!(JUMP_IF_NOT_EXC_MATCH, target),
Subscript => w!(Subscript),
StoreSubscript => w!(StoreSubscript),
DeleteSubscript => w!(DeleteSubscript),
Expand All @@ -1665,7 +1678,6 @@ impl Instruction {
BinaryOperationInplace { op } => w!(BinaryOperationInplace, ?op),
BinarySubscript => w!(BinarySubscript),
LoadAttr { idx } => w!(LoadAttr, name = idx),
TestOperation { op } => w!(TestOperation, ?op),
CompareOperation { op } => w!(CompareOperation, ?op),
CopyItem { index } => w!(CopyItem, index),
Pop => w!(Pop),
Expand Down
Loading
Loading