Specialize internal state of Fuse depending on underlying iterator

[AnthonyMikh]

This PR pushes #70366 further using approach outlined in comment by @cuviper. After this PR Fuse holds not Option<I> but Result<I, State> where State is () for regular iterators and Infallible for fused iterators. This enables niche optimisations as well as eliminating excessive code branches for fused iterators in predictable way without relying on unsafe. Also it concentrates all the specialization in one place and thus removes the need to reimplement methods of Iterator and DoubleEndedIterator for I: FusedIterator.

All the caveats listed in #70374 apply here as well.

[rust-highfive]

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

[cuviper]

This looks great to me! And the nice thing about the Infallible case is that this goes further than niche optimization -- an uninhabited Err means the Result will add no size even if the iterator has no niche.

The remaining question is whether the compiler team wants to let std use specialization on associated types at all.

[AnthonyMikh]

This looks great to me! And the nice thing about the Infallible case is that this goes further than niche optimization -- an uninhabited Err means the Result will add no size even if the iterator has no niche.

That was probably unclear from my comment, but I consider this a part of niche optimization.

[cuviper]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit 4ea75af with merge 1a2c08e6d58bb6fd8aef01982c4e673069aeca85...

[bors]

☀️ Try build successful - checks-azure
Build commit: 1a2c08e6d58bb6fd8aef01982c4e673069aeca85 (1a2c08e6d58bb6fd8aef01982c4e673069aeca85)

[rust-timer]

Queued 1a2c08e6d58bb6fd8aef01982c4e673069aeca85 with parent b9d5ee5, future comparison URL.

[rust-timer]

Finished benchmarking try commit 1a2c08e6d58bb6fd8aef01982c4e673069aeca85, comparison URL.

[cuviper]

Perf results look like a clear win across the board!

I'm actually a little surprised, since fuse is not often used directly in the compiler. However, the iterator inside FlatMap/Flatten is fused, so I'm guessing that's where this PR comes into play.

[cuviper]

In #70374 (comment), @timvermeulen noted that Zip type specialization was reverted in #36490 because it regressed on variance. I decided to try extending that test case for Fuse, and unfortunately this PR fails:

error[E0308]: mismatched types
  --> /home/jistone/rust/rust/src/test/ui/variance-iterators-in-libcore.rs:7:67
   |
LL | fn fuse_covariant<'a, I>(iter: Fuse<&'static I>) -> Fuse<&'a I> { iter }
   |                                                                   ^^^^ lifetime mismatch
   |
   = note: expected struct `std::iter::Fuse<&'a I>`
              found struct `std::iter::Fuse<&'static I>`
note: the lifetime `'a` as defined on the function body at 7:19...
  --> /home/jistone/rust/rust/src/test/ui/variance-iterators-in-libcore.rs:7:19
   |
LL | fn fuse_covariant<'a, I>(iter: Fuse<&'static I>) -> Fuse<&'a I> { iter }
   |                   ^^
   = note: ...does not necessarily outlive the static lifetime

Fuse does pass here on current master. I think I'll go ahead and extend that test for more iterator types, as it was intended long ago... #36490 (comment)

[AnthonyMikh]

It looks like I found the case where the compiler is simply not smart enough. I expected that putting 'static bound on StopState::State is enough to assert that lifetime of that type doesn't depend on lifetime of I, but it didn't help :/

[llogiq]

Could a Copy bound help? I found that rustc will sometimes avoid lifetime checks if something isn't Dropped anywhere while working on compact_arena.

[AnthonyMikh]

Could a Copy bound help? I found that rustc will sometimes avoid lifetime checks if something isn't Dropped anywhere while working on compact_arena.

@llogiq I tried it and it didn't help

[Centril]

r? @matthewjasper

[llogiq]

@AnthonyMikh it appears the type system won't allow any reasoning on associated types. I checked various simplified forms of covariant and contravariant types, but their association still stayed invariant.

[cuviper]

Somewhat amusingly, I just found that type specialization was also attempted here when FusedIterator was first added!
#35656 (diff)

We probably need a warning comment, at least...

[crlf0710]

Ping from triage, So, it seems this PR here is blocked, right? It would be nice to actually add the fore-mentioned failing unit test. And maybe create an separate issue for the lifetime check, though. Can @AnthonyMikh help here?

[AnthonyMikh]

Ping from triage, So, it seems this PR here is blocked, right? It would be nice to actually add the fore-mentioned failing unit test.

@crlf0710 it would be nice, though it is unclear where it should be located. So far all the tests are located at libcore/tests/iter.rs, but I am not sure if it belongs there: it contains unit tests rather than "compile pass" tests.

And maybe create an separate issue for the lifetime check, though. Can @AnthonyMikh help here?

It turns out there are several issues for that: #68375, #35852, #71547 and possibly some other issues. The first one though contains a comment by RustyYato with a possible workaround which would make Fuse have two generic parameters with the second one always being defaulted. However, it is unclear if this workaround is applicable here.

[spastorino]

This is really T-libs, but I think under our new scheme this is really libs-impl so the compiler team can take care of it. Going to label as such, if not appropriate please feel free to correct me.

[Mark-Simulacrum]

I'm going to relabel as S-blocked, as I think specialization is not going to be changing anytime soon to incorporate more things such as associated types; once that changes we can reopen this PR.

Is it accurate to say that this PR cannot proceed then? If so I think we should close it for the time being.

[AnthonyMikh]

@Mark-Simulacrum I agree with closing. On the one hand, the compiler is unlikely to resolve the issue with specialization any soon. On the other hand, it is even more unlikely that a PR that makes a covariant structure invariant will be accepted. Thereby, it's better to close this PR until the better times.