libs-team icon indicating copy to clipboard operation
libs-team copied to clipboard

implement Default for more iterators

Open programmerjake opened this issue 4 months ago • 10 comments

Proposal

Problem statement

Sometimes it's useful to be able to create empty iterators without needing to first obtain an empty container to then iterate over, some of the existing iterator types implement Default for this #77 but there are still quite a few missing.

Motivating examples or use cases

I'm currently writing a parser for parsing templates for command lines that are already split into a slice of arguments (written using the Default impls I'd like to add):

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
enum Token {
    Char(char),
    ArgSeparator,
}

#[derive(Clone, Debug, Default)]
struct Tokens<'a> {
    current: std::str::Chars<'a>,
    rest: std::slice::Iter<'a, &'a str>,
}

impl<'a> Tokens<'a> {
    fn new(args: &'a [&'a str]) -> Self {
        Self { rest: args, ..Self::default() }
    }
}

impl Iterator for Tokens<'_> {
    type Item = Token;

    fn next(&mut self) -> Option<Self::Item> {
        match self.current.next() {
            Some(c) => Some(Token::Char(c)),
            None => {
                self.current = self.rest.next()?.chars();
                Some(Token::ArgSeparator)
            }
        }
    }
}

// code that uses Tokens...

Solution sketch

implement Default for all iterators where the iterator is constructed from some type that has an obvious default value that's empty. So std::str::Chars should be Default since "" is obviously empty. std::array::IntoIter should not be Default since the array type's default wouldn't be empty unless it has zero length. std::env::Args should not be Default since an obvious default value is the program's arguments which isn't usually empty.

The full list of iterator types: https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html#implementors

Iterator implementations I found:

iterator implements Default already

For list, see https://github.com/rust-lang/libs-team/issues/656#issuecomment-3306065908

iterator should implement Default

iterator could implement Default but it might make the rest of the iterator slower (e.g. adding a conditional in Drop)

iterator shouldn't implement Default

iterator could implement Default but we maybe shouldn't

Alternatives

Manually construct iterators from a manually-obtained empty source type and have to implement Default manually.

Links and related work

We did some of this before in #77 but didn't end up handling all the iterator types.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

programmerjake avatar Sep 18 '25 05:09 programmerjake

seems like my list scraped from https://doc.rust-lang.org/nightly/trait.impl/core/iter/traits/iterator/trait.Iterator.js is still missing some

programmerjake avatar Sep 18 '25 05:09 programmerjake

I'm writing a program to actually find all the impls using rustdoc-types and just noticed std::array::IntoIter<T, N>: Default is stable, which is unexpected given the discussion in #77 that we'd specifically not want that.

programmerjake avatar Sep 18 '25 07:09 programmerjake

Ok, I finished writing that program: https://github.com/programmerjake/find_iterators_without_default/tree/e77e64d9469d3915a3500699b002688f3fb8e9bb (edit: I added the full list of types without Default impls to the top comment) Here's the output for 1.92.0-nightly (4645a7988 2025-09-17):

Types that implement both Iterator and Default

<details>

</details>

Types that implement Iterator but not Default

programmerjake avatar Sep 18 '25 08:09 programmerjake

The policy as stated in the solution sketch makes sense to me, but a bunch of entries in your list seem dubious, or perhaps require a very loose definition of "constructed from some type that has an obvious default value that's empty":

  • While char implements Default, that's not an "empty" value for iterator purpose. For example, char::default().escape_debug() is not an empty iterator nor an obvious default value for char::EscapeDebug.
  • result::IntoIter<T> is an empty iterator when constructed from Result::Err, but Result<T, E> doesn't implement Default at all, and if it did implement Default returning Err, that wouldn't be an "empty value" in general (only for the specific purpose of iterating over the result).
  • Iterator adaptors like Cycle can be empty with the right underlying iterator, but since the adapters are parameterized by the underlying iterator, I guess we'd need impls like impl<I: Iterator + Default> for Cycle<I>. That doesn't seem in line with the proposed policy because those bounds don't guarantee the default value of I is empty (cc https://github.com/rust-lang/rust/pull/140985).
  • slice::Split and friends requires picking a FnMut type for the predicate, and even if you pick one, [].split(|_| todo!()) yields an empty slice once, so it's not an empty iterator. Similar problems apply to string splitting iterators.
  • error::Source as currently defined is never an empty iterator when it's constructed from any error. That aside, while there are some rare error types that implement Default and don't yield and (further) sources (e.g., fmt::Error), but it seems odd to use that to justify an impl for error::Sources in general.

Perhaps the policy needs to be amended to say (1) it must also be obvious which type is the one whose obvious empty default value is used to create the iterator, and (2) the iterator value constructed this way must be empty.

hanna-kruppe avatar Sep 18 '25 16:09 hanna-kruppe

yeah, after some point I kinda forgot the policy I wrote down and went more by which iterators I think being empty is a reasonable default (going off the core::array::IntoIter precedent I guess), regardless of how you can construct them.

for Cycle<T>, I think it's reasonable to assume T: Default generally means the inner iterator's default is empty, but even if it isn't empty, the default should just be T::default().cycle()

programmerjake avatar Sep 18 '25 20:09 programmerjake

I'm writing a program to actually find all the impls using rustdoc-types and just noticed std::array::IntoIter<T, N>: Default is stable, which is unexpected given the discussion in #77 that we'd specifically not want that.

Some discussion in the PR.

jdahlstrom avatar Sep 23 '25 11:09 jdahlstrom

I'm planning on adjusting this ACP to propose that all iterators by convention should default to an empty iterator, unless the type comes with a strong expectation of the default being non-empty (e.g. env::Args), in which case they shouldn't implement Default.

I may take a while to get around to doing that, sorry in advance.

programmerjake avatar Sep 23 '25 12:09 programmerjake

One question:

ExtractIf has an F type param that represents the filter, which may not have a Default. What does a proper Default for it look like?

tisonkun avatar Oct 09 '25 13:10 tisonkun

I was assuming it'd just implement Default where F: Default -- that also seems like a good motivation for having more function items and capture-less lambdas implement Default

programmerjake avatar Oct 09 '25 20:10 programmerjake

We had a preliminary discussion about this in today's @rust-lang/libs-api meeting.

We observed some additional cases where we don't want to implement Default. For example, we don't want to implement Default for Matches or MatchIndices, because that would require synthesizing the generic Pattern type. Likewise for Map or Split which would require synthesizing a function.

Please drop any cases that would require synthesizing a user-controlled Pattern or function or other generic type. That'll make the remaining list easier to review.

(Cases that might require a std-specified type internally are okay; for instance, Lines should be fine.)

Can you also separate out the cases that are wrappers around another iterator (which will require I: Default) from the cases that aren't?

Once that's done we'll take another look at the remaining items. Thank you!

joshtriplett avatar Oct 28 '25 18:10 joshtriplett