Don't delegate to default implementations of ExactSizeIterator by yotamofek · Pull Request #149384 · rust-lang/rust (original) (raw)

FWIW, if you try

#![feature(binary_heap_into_iter_sorted)] pub fn demo(it: &std::collections::binary_heap::IntoIterSorted) -> usize { let (low, high) = it.size_hint(); assert_eq!(Some(low), high); low }

in playground with "Show MIR" https://play.rust-lang.org/?version=nightly&mode=release&edition=2024&gist=a074bb1d7524e51ab5cb16746fa66505

Then you'll see that the assert actually optimizes out before we even get to LLVM

bb0: {
    _0 = copy ((((*_1).0: std::collections::BinaryHeap<i32>).0: std::vec::Vec<i32>).1: usize);
    StorageLive(_6);
    _6 = Le(copy _0, const <i32 as std::mem::SizedTypeProperties>::MAX_SLICE_LEN);
    assume(move _6);
    StorageDead(_6);
    _5 = Option::<usize>::Some(copy _0);
    StorageLive(_2);
    _2 = copy _5;
    // DBG: _3 = &_5;
    // DBG: _4 = &_2;
    StorageDead(_2);
    return;
}

(It still has some silly leftovers -- probably due to phase ordering -- but there's no assert any more, just the comparison to tell LLVM it's UB to have a too-high length so it can optimize based on that.)

So overall I don't know how much it's worth bothering adding a bunch of extra code in the library.

That said, at the same time the overrides that just call an existing self.len() seem kinda reasonable anyway, so I dunno.

An alternative: this is just a sanity check, so I wonder if we might say "hey, that all-the-time assert is silly" instead, and maybe (in a different PR that would probably need a team nomination) replace the existing

#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn len(&self) -> usize {
    let (lower, upper) = self.size_hint();
    // Note: This assertion is overly defensive, but it checks the invariant
    // guaranteed by the trait. If this trait were rust-internal,
    // we could use debug_assert!; assert_eq! will check all Rust user
    // implementations too.
    assert_eq!(upper, Some(lower));
    lower
}

with

#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rust_inherit_overflow_checks]
fn len(&self) -> usize {
    let (lower, upper) = self.size_hint();
    // If overflow checks are on, check that the size hint isn't malformed.
    // An incorrect `size_hint` isn't UB, so we don't *have* to check,
    // and thus in default release mode we don't to save time.
    if let Some(upper) = upper {
        _ = upper - lower;
    }
    lower
}

on the theory that in release mode it's just not ever worth bothering with this check.

Let me know how you're feeling,
@rustbot author