Skip to content

Commit 8c3b698

Browse files
committed
Replace custom opcodes with CPython standard sequences
Remove RustPython-specific opcodes (BuildListFromTuples, BuildMapForCall, BuildSetFromTuples, BuildTupleFromTuples) and replace their usage with CPython 3.14 standard opcode sequences: - BuildListFromTuples → BUILD_LIST + LIST_EXTEND loop - BuildSetFromTuples → BUILD_SET + SET_UPDATE loop - BuildTupleFromTuples → BUILD_LIST + LIST_EXTEND + CALL_INTRINSIC_1(ListToTuple) - BuildMapForCall → DICT_MERGE loop Implement missing opcodes: - ListExtend: Extend list with iterable elements - SetUpdate: Add iterable elements to set - DictMerge: Merge dict with duplicate key checking
1 parent 6a064aa commit 8c3b698

File tree

4 files changed

+115
-113
lines changed

4 files changed

+115
-113
lines changed

Lib/_opcode_metadata.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,7 @@
127127
'UNPACK_SEQUENCE': 117,
128128
'YIELD_VALUE': 118,
129129
'BREAK': 119,
130-
'BUILD_LIST_FROM_TUPLES': 120,
131-
'BUILD_MAP_FOR_CALL': 121,
132-
'BUILD_SET_FROM_TUPLES': 122,
133130
'BUILD_TUPLE_FROM_ITER': 123,
134-
'BUILD_TUPLE_FROM_TUPLES': 124,
135131
'BUILD_TEMPLATE': 125,
136132
'BUILD_INTERPOLATION': 126,
137133
'CONTINUE': 128,

crates/codegen/src/compile.rs

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -536,18 +536,38 @@ impl Compiler {
536536

537537
if unpack {
538538
// Has starred elements
539+
// Stack has `size` tuples: [tuple1, tuple2, ..., tupleN] (bottom to top)
540+
// We need to unpack from bottom to top
539541
match collection_type {
540542
CollectionType::Tuple => {
541543
if size > 1 || pushed > 0 {
542-
emit!(self, Instruction::BuildTupleFromTuples { size });
544+
// BUILD_LIST + LIST_EXTEND from bottom tuple to top + LIST_TO_TUPLE
545+
emit!(self, Instruction::BuildList { size: 0 });
546+
for i in (1..=size).rev() {
547+
emit!(self, Instruction::ListExtend { i });
548+
}
549+
emit!(
550+
self,
551+
Instruction::CallIntrinsic1 {
552+
func: bytecode::IntrinsicFunction1::ListToTuple
553+
}
554+
);
543555
}
544556
// If size == 1 and pushed == 0, the single tuple is already on the stack
545557
}
546558
CollectionType::List => {
547-
emit!(self, Instruction::BuildListFromTuples { size });
559+
// BUILD_LIST + LIST_EXTEND from bottom tuple to top
560+
emit!(self, Instruction::BuildList { size: 0 });
561+
for i in (1..=size).rev() {
562+
emit!(self, Instruction::ListExtend { i });
563+
}
548564
}
549565
CollectionType::Set => {
550-
emit!(self, Instruction::BuildSetFromTuples { size });
566+
// BUILD_SET + SET_UPDATE from bottom tuple to top
567+
emit!(self, Instruction::BuildSet { size: 0 });
568+
for i in (1..=size).rev() {
569+
emit!(self, Instruction::SetUpdate { i });
570+
}
551571
}
552572
}
553573
} else {
@@ -4415,7 +4435,17 @@ impl Compiler {
44154435
if unpack {
44164436
// Starred: gather_elements produced tuples on stack
44174437
emit!(self, Instruction::BuildTuple { size: 1 }); // (.generic_base,)
4418-
emit!(self, Instruction::BuildTupleFromTuples { size: size + 1 });
4438+
// BUILD_LIST + LIST_EXTEND from bottom to top + LIST_TO_TUPLE
4439+
emit!(self, Instruction::BuildList { size: 0 });
4440+
for i in (1..=(size + 1)).rev() {
4441+
emit!(self, Instruction::ListExtend { i });
4442+
}
4443+
emit!(
4444+
self,
4445+
Instruction::CallIntrinsic1 {
4446+
func: bytecode::IntrinsicFunction1::ListToTuple
4447+
}
4448+
);
44194449
} else {
44204450
// No starred: individual elements on stack
44214451
// size includes class_func + name + bases count, +1 for .generic_base
@@ -6895,7 +6925,12 @@ impl Compiler {
68956925
}
68966926
}
68976927
if size > 1 {
6898-
emit!(self, Instruction::BuildMapForCall { size });
6928+
// Merge all dicts: first dict is accumulator, merge rest into it
6929+
// Stack: [dict1] [dict2] [dict3] ... [dict_size]
6930+
// After each DICT_MERGE 1: merge TOS into dict below it
6931+
for _ in 1..size {
6932+
emit!(self, Instruction::DictMerge { index: 1 });
6933+
}
68996934
}
69006935
Ok(())
69016936
}
@@ -6956,7 +6991,17 @@ impl Compiler {
69566991
if unpack || has_double_star {
69576992
// Create a tuple with positional args:
69586993
if unpack {
6959-
emit!(self, Instruction::BuildTupleFromTuples { size });
6994+
// BUILD_LIST + LIST_EXTEND from bottom to top + LIST_TO_TUPLE
6995+
emit!(self, Instruction::BuildList { size: 0 });
6996+
for i in (1..=size).rev() {
6997+
emit!(self, Instruction::ListExtend { i });
6998+
}
6999+
emit!(
7000+
self,
7001+
Instruction::CallIntrinsic1 {
7002+
func: bytecode::IntrinsicFunction1::ListToTuple
7003+
}
7004+
);
69607005
} else {
69617006
emit!(self, Instruction::BuildTuple { size });
69627007
}

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

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -249,19 +249,7 @@ pub enum Instruction {
249249
Break {
250250
target: Arg<Label>,
251251
} = 119,
252-
BuildListFromTuples {
253-
size: Arg<u32>,
254-
} = 120,
255-
BuildMapForCall {
256-
size: Arg<u32>,
257-
} = 121,
258-
BuildSetFromTuples {
259-
size: Arg<u32>,
260-
} = 122,
261252
BuildTupleFromIter = 123,
262-
BuildTupleFromTuples {
263-
size: Arg<u32>,
264-
} = 124,
265253
/// Build a Template from strings tuple and interpolations tuple on stack.
266254
/// Stack: [strings_tuple, interpolations_tuple] -> [template]
267255
BuildTemplate = 125,
@@ -418,19 +406,7 @@ impl TryFrom<u8> for Instruction {
418406
u8::from(Self::Break {
419407
target: Arg::marker(),
420408
}),
421-
u8::from(Self::BuildListFromTuples {
422-
size: Arg::marker(),
423-
}),
424-
u8::from(Self::BuildMapForCall {
425-
size: Arg::marker(),
426-
}),
427-
u8::from(Self::BuildSetFromTuples {
428-
size: Arg::marker(),
429-
}),
430409
u8::from(Self::BuildTupleFromIter),
431-
u8::from(Self::BuildTupleFromTuples {
432-
size: Arg::marker(),
433-
}),
434410
u8::from(Self::BuildTemplate),
435411
u8::from(Self::BuildInterpolation {
436412
oparg: Arg::marker(),
@@ -615,20 +591,13 @@ impl InstructionMetadata for Instruction {
615591
}
616592
Self::BuildString { size } => -(size.get(arg) as i32) + 1,
617593
Self::BuildTuple { size, .. } => -(size.get(arg) as i32) + 1,
618-
Self::BuildTupleFromTuples { size, .. } => -(size.get(arg) as i32) + 1,
619594
Self::BuildList { size, .. } => -(size.get(arg) as i32) + 1,
620-
Self::BuildListFromTuples { size, .. } => -(size.get(arg) as i32) + 1,
621595
Self::BuildSet { size, .. } => -(size.get(arg) as i32) + 1,
622-
Self::BuildSetFromTuples { size, .. } => -(size.get(arg) as i32) + 1,
623596
Self::BuildTupleFromIter => 0,
624597
Self::BuildMap { size } => {
625598
let nargs = size.get(arg) * 2;
626599
-(nargs as i32) + 1
627600
}
628-
Self::BuildMapForCall { size } => {
629-
let nargs = size.get(arg);
630-
-(nargs as i32) + 1
631-
}
632601
Self::DictUpdate { .. } => -1,
633602
Self::BuildSlice { argc } => {
634603
// push 1
@@ -860,16 +829,12 @@ impl InstructionMetadata for Instruction {
860829
Self::BinarySubscr => w!(BINARY_SUBSCR),
861830
Self::Break { target } => w!(BREAK, target),
862831
Self::BuildList { size } => w!(BUILD_LIST, size),
863-
Self::BuildListFromTuples { size } => w!(BUILD_LIST_FROM_TUPLES, size),
864832
Self::BuildMap { size } => w!(BUILD_MAP, size),
865-
Self::BuildMapForCall { size } => w!(BUILD_MAP_FOR_CALL, size),
866833
Self::BuildSet { size } => w!(BUILD_SET, size),
867-
Self::BuildSetFromTuples { size } => w!(BUILD_SET_FROM_TUPLES, size),
868834
Self::BuildSlice { argc } => w!(BUILD_SLICE, ?argc),
869835
Self::BuildString { size } => w!(BUILD_STRING, size),
870836
Self::BuildTuple { size } => w!(BUILD_TUPLE, size),
871837
Self::BuildTupleFromIter => w!(BUILD_TUPLE_FROM_ITER),
872-
Self::BuildTupleFromTuples { size } => w!(BUILD_TUPLE_FROM_TUPLES, size),
873838
Self::Call { nargs } => w!(CALL, nargs),
874839
Self::CallFunctionEx => w!(CALL_FUNCTION_EX),
875840
Self::CallKw { nargs } => w!(CALL_KW, nargs),

crates/vm/src/frame.rs

Lines changed: 64 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -664,35 +664,13 @@ impl ExecutingFrame<'_> {
664664
target: target.get(arg),
665665
},
666666
),
667-
Instruction::BuildListFromTuples { size } => {
668-
// SAFETY: compiler guarantees `size` tuples are on the stack
669-
let elements = unsafe { self.flatten_tuples(size.get(arg) as usize) };
670-
let list_obj = vm.ctx.new_list(elements);
671-
self.push_value(list_obj.into());
672-
Ok(None)
673-
}
674667
Instruction::BuildList { size } => {
675668
let elements = self.pop_multiple(size.get(arg) as usize).collect();
676669
let list_obj = vm.ctx.new_list(elements);
677670
self.push_value(list_obj.into());
678671
Ok(None)
679672
}
680-
Instruction::BuildMapForCall { size } => {
681-
self.execute_build_map_for_call(vm, size.get(arg))
682-
}
683673
Instruction::BuildMap { size } => self.execute_build_map(vm, size.get(arg)),
684-
Instruction::BuildSetFromTuples { size } => {
685-
let set = PySet::default().into_ref(&vm.ctx);
686-
for element in self.pop_multiple(size.get(arg) as usize) {
687-
// SAFETY: trust compiler
688-
let tup = unsafe { element.downcast_unchecked::<PyTuple>() };
689-
for item in tup.iter() {
690-
set.add(item.clone(), vm)?;
691-
}
692-
}
693-
self.push_value(set.into());
694-
Ok(None)
695-
}
696674
Instruction::BuildSet { size } => {
697675
let set = PySet::default().into_ref(&vm.ctx);
698676
for element in self.pop_multiple(size.get(arg) as usize) {
@@ -728,13 +706,6 @@ impl ExecutingFrame<'_> {
728706
}
729707
Ok(None)
730708
}
731-
Instruction::BuildTupleFromTuples { size } => {
732-
// SAFETY: compiler guarantees `size` tuples are on the stack
733-
let elements = unsafe { self.flatten_tuples(size.get(arg) as usize) };
734-
let list_obj = vm.ctx.new_tuple(elements);
735-
self.push_value(list_obj.into());
736-
Ok(None)
737-
}
738709
Instruction::BuildTuple { size } => {
739710
let elements = self.pop_multiple(size.get(arg) as usize).collect();
740711
let list_obj = vm.ctx.new_tuple(elements);
@@ -947,6 +918,46 @@ impl ExecutingFrame<'_> {
947918
dict.merge_object(source, vm)?;
948919
Ok(None)
949920
}
921+
Instruction::DictMerge { index } => {
922+
let source = self.pop_value();
923+
let obj = self.nth_value(index.get(arg));
924+
let dict: &Py<PyDict> = unsafe {
925+
// SAFETY: trust compiler
926+
obj.downcast_unchecked_ref()
927+
};
928+
929+
// DICT_MERGE checks for duplicate keys (used for **kwargs in function calls)
930+
// Check if source has keys method (is a mapping)
931+
if vm
932+
.get_method(source.clone(), vm.ctx.intern_str("keys"))
933+
.is_none()
934+
{
935+
return Err(vm.new_type_error(format!(
936+
"'{}' object is not a mapping",
937+
source.class().name()
938+
)));
939+
}
940+
941+
// Get keys from source and check for duplicates
942+
let keys_iter = vm.call_method(&source, "keys", ())?;
943+
for key in keys_iter.try_to_value::<Vec<PyObjectRef>>(vm)? {
944+
// Check if keyword argument is a string
945+
if key.downcast_ref::<PyStr>().is_none() {
946+
return Err(vm.new_type_error("keywords must be strings".to_owned()));
947+
}
948+
// Check for duplicate keyword arguments
949+
if dict.contains_key(&*key, vm) {
950+
let key_repr = key.repr(vm)?;
951+
return Err(vm.new_type_error(format!(
952+
"got multiple values for keyword argument {}",
953+
key_repr.as_str()
954+
)));
955+
}
956+
let value = vm.call_method(&source, "__getitem__", (key.clone(),))?;
957+
dict.set_item(&*key, value, vm)?;
958+
}
959+
Ok(None)
960+
}
950961
Instruction::EndAsyncFor => {
951962
// END_ASYNC_FOR pops (awaitable, exc) from stack
952963
// Stack: [awaitable, exc] -> []
@@ -1184,6 +1195,16 @@ impl ExecutingFrame<'_> {
11841195
list.append(item);
11851196
Ok(None)
11861197
}
1198+
Instruction::ListExtend { i } => {
1199+
let iterable = self.pop_value();
1200+
let obj = self.nth_value(i.get(arg));
1201+
let list: &Py<PyList> = unsafe {
1202+
// SAFETY: trust compiler
1203+
obj.downcast_unchecked_ref()
1204+
};
1205+
list.extend(iterable, vm)?;
1206+
Ok(None)
1207+
}
11871208
Instruction::LoadAttr { idx } => self.load_attr(vm, idx.get(arg)),
11881209
Instruction::LoadSuperAttr { arg: idx } => self.load_super_attr(vm, idx.get(arg)),
11891210
Instruction::LoadBuildClass => {
@@ -1570,6 +1591,20 @@ impl ExecutingFrame<'_> {
15701591
set.add(item, vm)?;
15711592
Ok(None)
15721593
}
1594+
Instruction::SetUpdate { i } => {
1595+
let iterable = self.pop_value();
1596+
let obj = self.nth_value(i.get(arg));
1597+
let set: &Py<PySet> = unsafe {
1598+
// SAFETY: trust compiler
1599+
obj.downcast_unchecked_ref()
1600+
};
1601+
// Iterate and add each item to the set
1602+
let iter = PyIter::try_from_object(vm, iterable)?;
1603+
while let PyIterReturn::Return(item) = iter.next(vm)? {
1604+
set.add(item, vm)?;
1605+
}
1606+
Ok(None)
1607+
}
15731608
Instruction::SetExcInfo => {
15741609
// Set the current exception to TOS (for except* handlers)
15751610
// This updates sys.exc_info() so bare 'raise' will reraise the matched exception
@@ -1872,16 +1907,6 @@ impl ExecutingFrame<'_> {
18721907
})
18731908
}
18741909

1875-
unsafe fn flatten_tuples(&mut self, size: usize) -> Vec<PyObjectRef> {
1876-
let mut elements = Vec::new();
1877-
for tup in self.pop_multiple(size) {
1878-
// SAFETY: caller ensures that the elements are tuples
1879-
let tup = unsafe { tup.downcast_unchecked::<PyTuple>() };
1880-
elements.extend(tup.iter().cloned());
1881-
}
1882-
elements
1883-
}
1884-
18851910
#[cfg_attr(feature = "flame-it", flame("Frame"))]
18861911
fn import(&mut self, vm: &VirtualMachine, module_name: Option<&Py<PyStr>>) -> PyResult<()> {
18871912
let module_name = module_name.unwrap_or(vm.ctx.empty_str);
@@ -2089,35 +2114,6 @@ impl ExecutingFrame<'_> {
20892114
Ok(None)
20902115
}
20912116

2092-
fn execute_build_map_for_call(&mut self, vm: &VirtualMachine, size: u32) -> FrameResult {
2093-
let size = size as usize;
2094-
let map_obj = vm.ctx.new_dict();
2095-
for obj in self.pop_multiple(size) {
2096-
// Use keys() method for all mapping objects to preserve order
2097-
Self::iterate_mapping_keys(vm, &obj, "keyword argument", |key| {
2098-
// Check for keyword argument restrictions
2099-
if key.downcast_ref::<PyStr>().is_none() {
2100-
return Err(vm.new_type_error("keywords must be strings"));
2101-
}
2102-
if map_obj.contains_key(&*key, vm) {
2103-
let key_repr = &key.repr(vm)?;
2104-
let msg = format!(
2105-
"got multiple values for keyword argument {}",
2106-
key_repr.as_str()
2107-
);
2108-
return Err(vm.new_type_error(msg));
2109-
}
2110-
2111-
let value = obj.get_item(&*key, vm)?;
2112-
map_obj.set_item(&*key, value, vm)?;
2113-
Ok(())
2114-
})?;
2115-
}
2116-
2117-
self.push_value(map_obj.into());
2118-
Ok(None)
2119-
}
2120-
21212117
fn execute_build_slice(
21222118
&mut self,
21232119
vm: &VirtualMachine,

0 commit comments

Comments
 (0)