Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions newsfragments/5830.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow non-`ExactSizeIterator` in `PyList::new`
86 changes: 32 additions & 54 deletions src/types/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,43 +36,6 @@ impl crate::impl_::pyclass::PyClassBaseType for PyList {
crate::impl_::pycell::PyVariableClassObject<T>;
}

#[inline]
#[track_caller]
fn try_new_from_iter<'py>(
py: Python<'py>,
mut elements: impl ExactSizeIterator<Item = PyResult<Bound<'py, PyAny>>>,
) -> PyResult<Bound<'py, PyList>> {
unsafe {
// PyList_New checks for overflow but has a bad error message, so we check ourselves
let len: Py_ssize_t = elements
.len()
.try_into()
.expect("out of range integral type conversion attempted on `elements.len()`");

let ptr = ffi::PyList_New(len);

// We create the `Bound` pointer here for two reasons:
// - panics if the ptr is null
// - its Drop cleans up the list if user code or the asserts panic.
let list = ptr.assume_owned(py).cast_into_unchecked();

let count = (&mut elements)
.take(len as usize)
.try_fold(0, |count, item| {
#[cfg(not(Py_LIMITED_API))]
ffi::PyList_SET_ITEM(ptr, count, item?.into_ptr());
#[cfg(Py_LIMITED_API)]
ffi::PyList_SetItem(ptr, count, item?.into_ptr());
Ok::<_, PyErr>(count + 1)
})?;

assert!(elements.next().is_none(), "Attempted to create PyList but `elements` was larger than reported by its `ExactSizeIterator` implementation.");
assert_eq!(len, count, "Attempted to create PyList but `elements` was smaller than reported by its `ExactSizeIterator` implementation.");

Ok(list)
}
}

impl PyList {
/// Constructs a new list with the given elements.
///
Expand Down Expand Up @@ -101,16 +64,38 @@ impl PyList {
/// All standard library structures implement this trait correctly, if they do, so calling this
/// function with (for example) [`Vec`]`<T>` or `&[T]` will always succeed.
#[track_caller]
pub fn new<'py, T, U>(
pub fn new<'py, T>(
py: Python<'py>,
elements: impl IntoIterator<Item = T, IntoIter = U>,
elements: impl IntoIterator<Item = T>,
) -> PyResult<Bound<'py, PyList>>
where
T: IntoPyObject<'py>,
U: ExactSizeIterator<Item = T>,
{
let iter = elements.into_iter().map(|e| e.into_bound_py_any(py));
try_new_from_iter(py, iter)
let mut elements = elements.into_iter().map(|e| e.into_bound_py_any(py));
let (min_len, _) = elements.size_hint();

// PyList_New checks for overflow but has a bad error message, so we check ourselves
let len: Py_ssize_t = min_len
.try_into()
.expect("out of range integral type conversion attempted on `elements.len()`");

let list = unsafe { ffi::PyList_New(len).assume_owned(py).cast_into_unchecked() };

let count = (&mut elements)
.take(len as usize)
.try_fold(0, |count, item| unsafe {
#[cfg(not(Py_LIMITED_API))]
ffi::PyList_SET_ITEM(list.as_ptr(), count, item?.into_ptr());
#[cfg(Py_LIMITED_API)]
ffi::PyList_SetItem(list.as_ptr(), count, item?.into_ptr());
Ok::<_, PyErr>(count + 1)
})?;

assert_eq!(len, count, "Attempted to create PyList but `elements` was smaller than reported by its `size_hint` implementation.");
Copy link
Member

@davidhewitt davidhewitt Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still trying to decide about this assertion. On one hand, if users have bad iterator implementations, this panic will help them clean up.

On the other hand, I still think it might be user-friendly to just truncate the list to count length.

Maybe for now we just document that we will panic if the iterator passes fewer elements than the size hint (and that we may change that in the future)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message about the panic should already be in the docs of this function


elements.try_for_each(|item| list.append(item?))?;

Ok(list)
}

/// Constructs a new empty list.
Expand Down Expand Up @@ -1516,6 +1501,10 @@ mod tests {
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}

fn size_hint(&self) -> (usize, Option<usize>) {
(self.1, Some(self.1))
}
}

impl ExactSizeIterator for FaultyIter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can just drop this impl?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Expand All @@ -1526,18 +1515,7 @@ mod tests {

#[test]
#[should_panic(
expected = "Attempted to create PyList but `elements` was larger than reported by its `ExactSizeIterator` implementation."
)]
fn too_long_iterator() {
Python::attach(|py| {
let iter = FaultyIter(0..usize::MAX, 73);
let _list = PyList::new(py, iter).unwrap();
})
}

#[test]
#[should_panic(
expected = "Attempted to create PyList but `elements` was smaller than reported by its `ExactSizeIterator` implementation."
expected = "Attempted to create PyList but `elements` was smaller than reported by its `size_hint` implementation."
)]
fn too_short_iterator() {
Python::attach(|py| {
Expand Down
Loading