panic early when TrustedLen indicates a length > usize::MAX

[the8472]

Changes TrustedLen specializations to immediately panic when size_hint().1 == None.

As far as I can tell this is not a change a minimal change in observable behavior for anything except ZSTs because the fallback path would go through extend_desugared() which tries to reserve(lower_bound) which already is usize::MAX and that would also lead to a panic. Before it might have popped somewhere between zero and a few elements from the iterator before panicking while it now panics immediately.

Overall this should reduce codegen by eliminating the fallback paths.

While looking into the with_capacity() behavior I also noticed that its documentation didn't have a Panics section, so I added that.

[rust-highfive]

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

[the8472]

@rustbot label T-libs

since it also touches visible docs, not just implementation

[rustbot]

Error: Label since can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[JohnTitor]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit ad3a791 with merge fd6d76150a587f8fbfa740e956a61774c1e85edc...

[bors]

☀️ Try build successful - checks-actions
Build commit: fd6d76150a587f8fbfa740e956a61774c1e85edc (fd6d76150a587f8fbfa740e956a61774c1e85edc)

[rust-timer]

Queued fd6d76150a587f8fbfa740e956a61774c1e85edc with parent 4fdac23, future comparison URL.

[rust-timer]

Finished benchmarking try commit (fd6d76150a587f8fbfa740e956a61774c1e85edc): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[kennytm]

@bors r+

[bors]

📌 Commit ad3a791 has been approved by kennytm

[bors]

⌛ Testing commit ad3a791 with merge 803ddb8...

[bors]

☀️ Test successful - checks-actions
Approved by: kennytm
Pushing 803ddb8 to master...

[Mark-Simulacrum]

This.. surprisingly to me, at least, ended up being a pretty nice perf win, of up to 8%: https://perf.rust-lang.org/compare.html?start=49e1ec09952c5ab7798addd29532d44dc020283f&end=803ddb83598838fb9de308d283b759ba463e5e80&stat=instructions:u. Largely this looks to be driven by incremental benchmark improvements, but not completely.

[the8472]

Some iter.collect<{Arc, Rc}>() may have resulted in up to 3 separate monomorphizations of each iterator call chain. One for Rc::from_iter_exact, one for spec_extend and one for the fallback. For iter.collect<Vec>() there would have been two. Now there should only be one.

I haven't actually looked at the generated IR, but that's the theory that motivated this change.

[pickfire]

Why does the above code seemed to have the same condition written twice? See 10 lines above.

[the8472]

One checks that high == low. the other checks that it's finite.

[pickfire]

No, I mean

        if let Some(high_value) = high {
            debug_assert_eq!(
                low,
                high_value,
                "TrustedLen iterator's size hint is not exact: {:?}",
                (low, high)
            );
        }
        if let Some(additional) = high {
            self.reserve(additional);
...

[the8472]

idk, probably just an historic artifact of code being moved around and redundant conditions not being merged. Someone just has to clean it up.