RustPython · youknowone · Mar 31, 2025 · Mar 2, 2025
diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs
@@ -2430,23 +2430,25 @@ impl Compiler<'_> {
            Expr::List(ExprList { elts, .. }) => {
                let (size, unpack) = self.gather_elements(0, elts)?;
                if unpack {
-                    emit!(self, Instruction::BuildListUnpack { size });
+                    emit!(self, Instruction::BuildListFromTuples { size });
                } else {
                    emit!(self, Instruction::BuildList { size });
                }
            }
            Expr::Tuple(ExprTuple { elts, .. }) => {
                let (size, unpack) = self.gather_elements(0, elts)?;
                if unpack {
-                    emit!(self, Instruction::BuildTupleUnpack { size });
+                    if size > 1 {
+                        emit!(self, Instruction::BuildTupleFromTuples { size });
+                    }
                } else {
                    emit!(self, Instruction::BuildTuple { size });
                }
            }
            Expr::Set(ExprSet { elts, .. }) => {
                let (size, unpack) = self.gather_elements(0, elts)?;
                if unpack {
-                    emit!(self, Instruction::BuildSetUnpack { size });
+                    emit!(self, Instruction::BuildSetFromTuples { size });
                } else {
                    emit!(self, Instruction::BuildSet { size });
                }
@@ -2819,7 +2821,7 @@ impl Compiler<'_> {
        let call = if unpack || has_double_star {
            // Create a tuple with positional args:
            if unpack {
-                emit!(self, Instruction::BuildTupleUnpack { size });
+                emit!(self, Instruction::BuildTupleFromTuples { size });
            } else {
                emit!(self, Instruction::BuildTuple { size });
            }
@@ -2863,34 +2865,31 @@ impl Compiler<'_> {

        let size = if has_stars {
            let mut size = 0;
+            let mut iter = elements.iter().peekable();
+            let mut run_size = before;

-            if before > 0 {
-                emit!(self, Instruction::BuildTuple { size: before });
-                size += 1;
-            }
+            loop {
+                if iter.peek().is_none_or(|e| matches!(e, Expr::Starred(_))) {
+                    emit!(self, Instruction::BuildTuple { size: run_size });
+                    run_size = 0;
+                    size += 1;
+                }

-            let groups = elements
-                .iter()
-                .map(|element| {
-                    if let Expr::Starred(ExprStarred { value, .. }) = &element {
-                        (true, value.as_ref())
-                    } else {
-                        (false, element)
+                match iter.next() {
+                    Some(Expr::Starred(ExprStarred { value, .. })) => {
+                        self.compile_expression(value)?;
+                        // We need to collect each unpacked element into a
+                        // tuple, since any side-effects during the conversion
+                        // should be made visible before evaluating remaining
+                        // expressions.
+                        emit!(self, Instruction::BuildTupleFromIter);
+                        size += 1;
                    }
-                })
-                .chunk_by(|(starred, _)| *starred);
-
-            for (starred, run) in &groups {
-                let mut run_size = 0;
-                for (_, value) in run {
-                    self.compile_expression(value)?;
-                    run_size += 1
-                }
-                if starred {
-                    size += run_size
-                } else {
-                    emit!(self, Instruction::BuildTuple { size: run_size });
-                    size += 1
+                    Some(element) => {
+                        self.compile_expression(element)?;
+                        run_size += 1;
+                    }
+                    None => break,
                }
            }


diff --git a/compiler/core/src/bytecode.rs b/compiler/core/src/bytecode.rs
@@ -542,19 +542,20 @@ pub enum Instruction {
    BuildTuple {
        size: Arg<u32>,
    },
-    BuildTupleUnpack {
+    BuildTupleFromTuples {
        size: Arg<u32>,
    },
+    BuildTupleFromIter,
    BuildList {
        size: Arg<u32>,
    },
-    BuildListUnpack {
+    BuildListFromTuples {
        size: Arg<u32>,
    },
    BuildSet {
        size: Arg<u32>,
    },
-    BuildSetUnpack {
+    BuildSetFromTuples {
        size: Arg<u32>,
    },
    BuildMap {
@@ -1260,11 +1261,12 @@ impl Instruction {
            Raise { kind } => -(kind.get(arg) as u8 as i32),
            BuildString { size }
            | BuildTuple { size, .. }
-            | BuildTupleUnpack { size, .. }
+            | BuildTupleFromTuples { size, .. }
            | BuildList { size, .. }
-            | BuildListUnpack { size, .. }
+            | BuildListFromTuples { size, .. }
            | BuildSet { size, .. }
-            | BuildSetUnpack { size, .. } => -(size.get(arg) as i32) + 1,
+            | BuildSetFromTuples { size, .. } => -(size.get(arg) as i32) + 1,
+            BuildTupleFromIter => 0,
            BuildMap { size } => {
                let nargs = size.get(arg) * 2;
                -(nargs as i32) + 1
@@ -1447,13 +1449,14 @@ impl Instruction {
            Raise { kind } => w!(Raise, ?kind),
            BuildString { size } => w!(BuildString, size),
            BuildTuple { size } => w!(BuildTuple, size),
-            BuildTupleUnpack { size } => w!(BuildTupleUnpack, size),
+            BuildTupleFromTuples { size } => w!(BuildTupleFromTuples, size),
+            BuildTupleFromIter => w!(BuildTupleFromIter),
            BuildList { size } => w!(BuildList, size),
-            BuildListUnpack { size } => w!(BuildListUnpack, size),
+            BuildListFromTuples { size } => w!(BuildListFromTuples, size),
            BuildSet { size } => w!(BuildSet, size),
-            BuildSetUnpack { size } => w!(BuildSetUnpack, size),
+            BuildSetFromTuples { size } => w!(BuildSetFromTuples, size),
            BuildMap { size } => w!(BuildMap, size),
-            BuildMapForCall { size } => w!(BuildMap, size),
+            BuildMapForCall { size } => w!(BuildMapForCall, size),
            DictUpdate => w!(DictUpdate),
            BuildSlice { step } => w!(BuildSlice, step),
            ListAppend { i } => w!(ListAppend, i),

diff --git a/extra_tests/snippets/builtin_list.py b/extra_tests/snippets/builtin_list.py
@@ -636,6 +636,27 @@ def iadd_slice():
 a = [*[1, 2], 3, *[4, 5]]
 assert a == [1, 2, 3, 4, 5]

+# Test for list unpacking evaluation order (https://github.com/RustPython/RustPython/issues/5566)
+a = [1, 2]
+b = [a.append(3), *a, a.append(4), *a]
+assert a == [1, 2, 3, 4]
+assert b == [None, 1, 2, 3, None, 1, 2, 3, 4]
+
+for base in object, list, tuple:
+    # do not assume that a type inherited from some sequence type behaves like
+    # that sequence type
+    class C(base):
+        def __iter__(self):
+            a.append(2)
+            def inner():
+                yield 3
+                a.append(4)
+            return inner()
+
+    a = [1]
+    b = [*a, *C(), *a.copy()]
+    assert b == [1, 3, 1, 2, 4]
+
 # Test for list entering daedlock or not (https://github.com/RustPython/RustPython/pull/2933)
 class MutatingCompare:
    def __eq__(self, other):

diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -710,27 +710,28 @@ impl ExecutingFrame<'_> {
                self.push_value(list_obj.into());
                Ok(None)
            }
-            bytecode::Instruction::BuildListUnpack { size } => {
-                let elements = self.unpack_elements(vm, size.get(arg) as usize)?;
+            bytecode::Instruction::BuildListFromTuples { size } => {
+                // SAFETY: compiler guarantees `size` tuples are on the stack
+                let elements = unsafe { self.flatten_tuples(size.get(arg) as usize) };
                let list_obj = vm.ctx.new_list(elements);
                self.push_value(list_obj.into());
                Ok(None)
            }
            bytecode::Instruction::BuildSet { size } => {
                let set = PySet::new_ref(&vm.ctx);
-                {
-                    for element in self.pop_multiple(size.get(arg) as usize) {
-                        set.add(element, vm)?;
-                    }
+                for element in self.pop_multiple(size.get(arg) as usize) {
+                    set.add(element, vm)?;
                }
                self.push_value(set.into());
                Ok(None)
            }
-            bytecode::Instruction::BuildSetUnpack { size } => {
+            bytecode::Instruction::BuildSetFromTuples { size } => {
                let set = PySet::new_ref(&vm.ctx);
-                {
-                    for element in self.pop_multiple(size.get(arg) as usize) {
-                        vm.map_iterable_object(&element, |x| set.add(x, vm))??;
+                for element in self.pop_multiple(size.get(arg) as usize) {
+                    // SAFETY: trust compiler
+                    let tup = unsafe { element.downcast_unchecked::<PyTuple>() };
+                    for item in tup.iter() {
+                        set.add(item.clone(), vm)?;
                    }
                }
                self.push_value(set.into());
@@ -742,12 +743,21 @@ impl ExecutingFrame<'_> {
                self.push_value(list_obj.into());
                Ok(None)
            }
-            bytecode::Instruction::BuildTupleUnpack { size } => {
-                let elements = self.unpack_elements(vm, size.get(arg) as usize)?;
+            bytecode::Instruction::BuildTupleFromTuples { size } => {
+                // SAFETY: compiler guarantees `size` tuples are on the stack
+                let elements = unsafe { self.flatten_tuples(size.get(arg) as usize) };
                let list_obj = vm.ctx.new_tuple(elements);
                self.push_value(list_obj.into());
                Ok(None)
            }
+            bytecode::Instruction::BuildTupleFromIter => {
+                if !self.top_value().class().is(vm.ctx.types.tuple_type) {
+                    let elements: Vec<_> = self.pop_value().try_to_value(vm)?;
+                    let list_obj = vm.ctx.new_tuple(elements);
+                    self.push_value(list_obj.into());
+                }
+                Ok(None)
+            }
            bytecode::Instruction::BuildMap { size } => self.execute_build_map(vm, size.get(arg)),
            bytecode::Instruction::BuildMapForCall { size } => {
                self.execute_build_map_for_call(vm, size.get(arg))
@@ -1234,14 +1244,14 @@ impl ExecutingFrame<'_> {
            })
    }

-    #[cfg_attr(feature = "flame-it", flame("Frame"))]
-    fn unpack_elements(&mut self, vm: &VirtualMachine, size: usize) -> PyResult<Vec<PyObjectRef>> {
-        let mut result = Vec::<PyObjectRef>::new();
-        for element in self.pop_multiple(size) {
-            let items: Vec<_> = element.try_to_value(vm)?;
-            result.extend(items);
+    unsafe fn flatten_tuples(&mut self, size: usize) -> Vec<PyObjectRef> {
+        let mut elements = Vec::new();
+        for tup in self.pop_multiple(size) {
+            // SAFETY: caller ensures that the elements are tuples
+            let tup = unsafe { tup.downcast_unchecked::<PyTuple>() };
+            elements.extend(tup.iter().cloned());
        }
-        Ok(result)
+        elements
    }

    #[cfg_attr(feature = "flame-it", flame("Frame"))]
@@ -1490,8 +1500,10 @@ impl ExecutingFrame<'_> {
        } else {
            IndexMap::new()
        };
-        let args = self.pop_value();
-        let args = args.try_to_value(vm)?;
+        // SAFETY: trust compiler
+        let args = unsafe { self.pop_value().downcast_unchecked::<PyTuple>() }
+            .as_slice()
+            .to_vec();
        Ok(FuncArgs { args, kwargs })
    }