Skip to content

Commit 91f5965

Browse files
ShaharNavehShaharNaveh
committed
Split TestOperator inatruction
1 parent a819128 commit 91f5965

File tree

3 files changed

+88
-94
lines changed

3 files changed

+88
-94
lines changed

‎crates/codegen/src/compile.rs‎

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use rustpython_compiler_core::{
3636
Mode, OneIndexed, PositionEncoding, SourceFile, SourceLocation,
3737
bytecode::{
3838
self, Arg as OpArgMarker, BinaryOperator, CodeObject, ComparisonOperator, ConstantData,
39-
Instruction, OpArg, OpArgType, UnpackExArgs,
39+
Instruction, Invert, OpArg, OpArgType, UnpackExArgs,
4040
},
4141
};
4242
use rustpython_wtf8::Wtf8Buf;
@@ -2034,20 +2034,7 @@ impl Compiler {
20342034

20352035
// Check exception type:
20362036
self.compile_expression(exc_type)?;
2037-
emit!(
2038-
self,
2039-
Instruction::TestOperation {
2040-
op: bytecode::TestOperator::ExceptionMatch,
2041-
}
2042-
);
2043-
2044-
// We cannot handle this exception type:
2045-
emit!(
2046-
self,
2047-
Instruction::PopJumpIfFalse {
2048-
target: next_handler,
2049-
}
2050-
);
2037+
emit!(self, Instruction::JumpIfNotExcMatch(next_handler));
20512038

20522039
// We have a match, store in name (except x as y)
20532040
if let Some(alias) = name {
@@ -3477,12 +3464,7 @@ impl Compiler {
34773464
// 4. Load None.
34783465
self.emit_load_const(ConstantData::None);
34793466
// 5. Compare with IS_OP 1.
3480-
emit!(
3481-
self,
3482-
Instruction::TestOperation {
3483-
op: bytecode::TestOperator::IsNot
3484-
}
3485-
);
3467+
emit!(self, Instruction::IsOp(Invert::Yes));
34863468

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

36493631
// Check if copy is None (consumes the copy like POP_JUMP_IF_NONE)
36503632
self.emit_load_const(ConstantData::None);
3651-
emit!(
3652-
self,
3653-
Instruction::TestOperation {
3654-
op: bytecode::TestOperator::IsNot
3655-
}
3656-
);
3633+
emit!(self, Instruction::IsOp(Invert::Yes));
3634+
36573635
// Stack: [subject, keys_tuple, values_tuple, bool]
36583636
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
36593637
// Stack: [subject, keys_tuple, values_tuple]
@@ -3948,12 +3926,7 @@ impl Compiler {
39483926
Singleton::True => ConstantData::Boolean { value: true },
39493927
});
39503928
// Compare using the "Is" operator.
3951-
emit!(
3952-
self,
3953-
Instruction::TestOperation {
3954-
op: bytecode::TestOperator::Is
3955-
}
3956-
);
3929+
emit!(self, Instruction::IsOp(Invert::No));
39573930
// Jump to the failure label if the comparison is false.
39583931
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
39593932
Ok(())
@@ -4082,7 +4055,6 @@ impl Compiler {
40824055
let (last_val, mid_exprs) = exprs.split_last().unwrap();
40834056

40844057
use bytecode::ComparisonOperator::*;
4085-
use bytecode::TestOperator::*;
40864058
let compile_cmpop = |c: &mut Self, op: &CmpOp| match op {
40874059
CmpOp::Eq => emit!(c, Instruction::CompareOperation { op: Equal }),
40884060
CmpOp::NotEq => emit!(c, Instruction::CompareOperation { op: NotEqual }),
@@ -4092,10 +4064,10 @@ impl Compiler {
40924064
CmpOp::GtE => {
40934065
emit!(c, Instruction::CompareOperation { op: GreaterOrEqual })
40944066
}
4095-
CmpOp::In => emit!(c, Instruction::TestOperation { op: In }),
4096-
CmpOp::NotIn => emit!(c, Instruction::TestOperation { op: NotIn }),
4097-
CmpOp::Is => emit!(c, Instruction::TestOperation { op: Is }),
4098-
CmpOp::IsNot => emit!(c, Instruction::TestOperation { op: IsNot }),
4067+
CmpOp::In => emit!(c, Instruction::ContainsOp(Invert::No)),
4068+
CmpOp::NotIn => emit!(c, Instruction::ContainsOp(Invert::Yes)),
4069+
CmpOp::Is => emit!(c, Instruction::IsOp(Invert::No)),
4070+
CmpOp::IsNot => emit!(c, Instruction::IsOp(Invert::Yes)),
40994071
};
41004072

41014073
// a == b == c == d

‎crates/compiler-core/src/bytecode.rs‎

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,10 @@ pub enum Instruction {
577577
Subscript,
578578
StoreSubscript,
579579
DeleteSubscript,
580+
/// Performs `is` comparison, or `is not` if `invert` is 1.
581+
IsOp(Arg<Invert>),
582+
/// Performs `in` comparison, or `not in` if `invert` is 1.
583+
ContainsOp(Arg<Invert>),
580584
StoreAttr {
581585
idx: Arg<NameIdx>,
582586
},
@@ -600,9 +604,6 @@ pub enum Instruction {
600604
LoadAttr {
601605
idx: Arg<NameIdx>,
602606
},
603-
TestOperation {
604-
op: Arg<TestOperator>,
605-
},
606607
CompareOperation {
607608
op: Arg<ComparisonOperator>,
608609
},
@@ -628,6 +629,10 @@ pub enum Instruction {
628629
Break {
629630
target: Arg<Label>,
630631
},
632+
/// Performs exception matching for except.
633+
/// Tests whether the STACK[-2] is an exception matching STACK[-1].
634+
/// Pops STACK[-1] and pushes the boolean result of the test.
635+
JumpIfNotExcMatch(Arg<Label>),
631636
Jump {
632637
target: Arg<Label>,
633638
},
@@ -1088,19 +1093,6 @@ op_arg_enum!(
10881093
}
10891094
);
10901095

1091-
op_arg_enum!(
1092-
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1093-
#[repr(u8)]
1094-
pub enum TestOperator {
1095-
In = 0,
1096-
NotIn = 1,
1097-
Is = 2,
1098-
IsNot = 3,
1099-
/// two exceptions that match?
1100-
ExceptionMatch = 4,
1101-
}
1102-
);
1103-
11041096
op_arg_enum!(
11051097
/// The possible Binary operators
11061098
/// # Examples
@@ -1141,6 +1133,24 @@ op_arg_enum!(
11411133
}
11421134
);
11431135

1136+
op_arg_enum!(
1137+
/// Whether or not to invert the operation.
1138+
#[repr(u8)]
1139+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1140+
pub enum Invert {
1141+
/// ```py
1142+
/// foo is bar
1143+
/// x in lst
1144+
/// ```
1145+
No = 0,
1146+
/// ```py
1147+
/// foo is not bar
1148+
/// x not in lst
1149+
/// ```
1150+
Yes = 1,
1151+
}
1152+
);
1153+
11441154
#[derive(Copy, Clone)]
11451155
pub struct UnpackExArgs {
11461156
pub before: u8,
@@ -1462,10 +1472,7 @@ impl Instruction {
14621472
DeleteAttr { .. } => -1,
14631473
LoadConst { .. } => 1,
14641474
UnaryOperation { .. } => 0,
1465-
BinaryOperation { .. }
1466-
| BinaryOperationInplace { .. }
1467-
| TestOperation { .. }
1468-
| CompareOperation { .. } => -1,
1475+
BinaryOperation { .. } | BinaryOperationInplace { .. } | CompareOperation { .. } => -1,
14691476
BinarySubscript => -1,
14701477
CopyItem { .. } => 1,
14711478
Pop => -1,
@@ -1508,6 +1515,8 @@ impl Instruction {
15081515
1
15091516
}
15101517
}
1518+
IsOp(_) | ContainsOp(_) => -1,
1519+
JumpIfNotExcMatch(_) => -2,
15111520
ReturnValue => -1,
15121521
ReturnConst { .. } => 0,
15131522
Resume { .. } => 0,
@@ -1654,6 +1663,9 @@ impl Instruction {
16541663
DeleteGlobal(idx) => w!(DeleteGlobal, name = idx),
16551664
DeleteDeref(idx) => w!(DeleteDeref, cell_name = idx),
16561665
LoadClosure(i) => w!(LoadClosure, cell_name = i),
1666+
IsOp(inv) => w!(IS_OP, ?inv),
1667+
ContainsOp(inv) => w!(CONTAINS_OP, ?inv),
1668+
JumpIfNotExcMatch(target) => w!(JUMP_IF_NOT_EXC_MATCH, target),
16571669
Subscript => w!(Subscript),
16581670
StoreSubscript => w!(StoreSubscript),
16591671
DeleteSubscript => w!(DeleteSubscript),
@@ -1665,7 +1677,6 @@ impl Instruction {
16651677
BinaryOperationInplace { op } => w!(BinaryOperationInplace, ?op),
16661678
BinarySubscript => w!(BinarySubscript),
16671679
LoadAttr { idx } => w!(LoadAttr, name = idx),
1668-
TestOperation { op } => w!(TestOperation, ?op),
16691680
CompareOperation { op } => w!(CompareOperation, ?op),
16701681
CopyItem { index } => w!(CopyItem, index),
16711682
Pop => w!(Pop),

‎crates/vm/src/frame.rs‎

Lines changed: 46 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,52 @@ impl ExecutingFrame<'_> {
884884
bytecode::Instruction::StoreAttr { idx } => self.store_attr(vm, idx.get(arg)),
885885
bytecode::Instruction::DeleteAttr { idx } => self.delete_attr(vm, idx.get(arg)),
886886
bytecode::Instruction::UnaryOperation { op } => self.execute_unary_op(vm, op.get(arg)),
887-
bytecode::Instruction::TestOperation { op } => self.execute_test(vm, op.get(arg)),
887+
bytecode::Instruction::IsOp(invert) => {
888+
let b = self.pop_value();
889+
let a = self.pop_value();
890+
let res = a.is(&b);
891+
892+
let value = match invert.get(arg) {
893+
bytecode::Invert::No => res,
894+
bytecode::Invert::Yes => !res,
895+
};
896+
self.push_value(vm.ctx.new_bool(value).into());
897+
Ok(None)
898+
}
899+
bytecode::Instruction::ContainsOp(invert) => {
900+
let b = self.pop_value();
901+
let a = self.pop_value();
902+
903+
let value = match invert.get(arg) {
904+
bytecode::Invert::No => self._in(vm, &a, &b)?,
905+
bytecode::Invert::Yes => self._not_in(vm, &a, &b)?,
906+
};
907+
self.push_value(vm.ctx.new_bool(value).into());
908+
Ok(None)
909+
}
910+
bytecode::Instruction::JumpIfNotExcMatch(target) => {
911+
let b = self.pop_value();
912+
let a = self.pop_value();
913+
if let Some(tuple_of_exceptions) = b.downcast_ref::<PyTuple>() {
914+
for exception in tuple_of_exceptions {
915+
if !exception
916+
.is_subclass(vm.ctx.exceptions.base_exception_type.into(), vm)?
917+
{
918+
return Err(vm.new_type_error(
919+
"catching classes that do not inherit from BaseException is not allowed",
920+
));
921+
}
922+
}
923+
} else if !b.is_subclass(vm.ctx.exceptions.base_exception_type.into(), vm)? {
924+
return Err(vm.new_type_error(
925+
"catching classes that do not inherit from BaseException is not allowed",
926+
));
927+
}
928+
929+
let value = a.is_instance(&b, vm)?;
930+
self.push_value(vm.ctx.new_bool(value).into());
931+
self.pop_jump_if(vm, target.get(arg), false)
932+
}
888933
bytecode::Instruction::CompareOperation { op } => self.execute_compare(vm, op.get(arg)),
889934
bytecode::Instruction::ReturnValue => {
890935
let value = self.pop_value();
@@ -2234,40 +2279,6 @@ impl ExecutingFrame<'_> {
22342279
Ok(!self._in(vm, needle, haystack)?)
22352280
}
22362281

2237-
#[cfg_attr(feature = "flame-it", flame("Frame"))]
2238-
fn execute_test(&mut self, vm: &VirtualMachine, op: bytecode::TestOperator) -> FrameResult {
2239-
let b = self.pop_value();
2240-
let a = self.pop_value();
2241-
let value = match op {
2242-
bytecode::TestOperator::Is => a.is(&b),
2243-
bytecode::TestOperator::IsNot => !a.is(&b),
2244-
bytecode::TestOperator::In => self._in(vm, &a, &b)?,
2245-
bytecode::TestOperator::NotIn => self._not_in(vm, &a, &b)?,
2246-
bytecode::TestOperator::ExceptionMatch => {
2247-
if let Some(tuple_of_exceptions) = b.downcast_ref::<PyTuple>() {
2248-
for exception in tuple_of_exceptions {
2249-
if !exception
2250-
.is_subclass(vm.ctx.exceptions.base_exception_type.into(), vm)?
2251-
{
2252-
return Err(vm.new_type_error(
2253-
"catching classes that do not inherit from BaseException is not allowed",
2254-
));
2255-
}
2256-
}
2257-
} else if !b.is_subclass(vm.ctx.exceptions.base_exception_type.into(), vm)? {
2258-
return Err(vm.new_type_error(
2259-
"catching classes that do not inherit from BaseException is not allowed",
2260-
));
2261-
}
2262-
2263-
a.is_instance(&b, vm)?
2264-
}
2265-
};
2266-
2267-
self.push_value(vm.ctx.new_bool(value).into());
2268-
Ok(None)
2269-
}
2270-
22712282
#[cfg_attr(feature = "flame-it", flame("Frame"))]
22722283
fn execute_compare(
22732284
&mut self,

0 commit comments

Comments
 (0)