Repository navigation
Stabilizing Iterator::intersperse breaks a large number of crates #88967
Description
The 1.56 crater run shows that there are at least 59 root crates (or repositories) broken by this change, and 260 crates.io crates along with 278 github repositories which depend on those.
See https://crater-reports.s3.amazonaws.com/beta-1.56-2/index.html > "regressed: root results" > E0034 for the root crates.
cc some threads with partial discussion:
- https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/possible.20semi-breaking.20change.20in.20next.20release
- Recent PRs break lots of nightly builds #88878
Nominating for T-libs-api discussion as the FCP on #79524 seems to have been made without commentary about the expected level of breakage, so I suspect it was not discussed.
Activity
the8472 commentedon Sep 15, 2021
Given #48552, rust-itertools/itertools#576 might solve this assuming it gets merged and released before 1.56 reaches stable.
Edit, wait... was the check for deprecated candidates in #48552 (comment) implemented? Doesn't look like it.
yaahc commentedon Oct 7, 2021
During yesterday's libs-api team meeting we discussed how we can stabilize intersperse without causing large amounts of breakage across the ecosystem. The plan we arrived upon is to first revert the stabilization and then plan on stabilizing the feature in a future release, @Mark-Simulacrum suggested 1.65.
Whenever we end up stabilizing it we need to have a plan for how we will avoid these same breakages. Suggestions so far include using build scripts or
#[cfg(accessible(...)]in itertools to detect the presence of intersperse instdand disable Itertools::intersperse when there would be a conflict.In a previous meeting we also discussed more long term solutions we could implement to avoid ambiguity breakages caused by introducing new methods like this. Suggestions in that meeting included:
jswrenn commentedon Oct 7, 2021
I believe the accepted supertrait_item_shadowing RFC might provide a long-term solution to this kind of breakage.
Auto merge of rust-lang#89638 - rust-lang:revert-88548-intersperse, r…
Mark-Simulacrum commentedon Oct 14, 2021
Dropping from the 1.56 milestone -- no longer release sensitive.
m-ou-se commentedon Oct 27, 2021
In the libs api meeting right now, @dtolnay suggested to investigate the possibility of prioritizing subtraits when resolving names. So then the
Itertoolstrait would be picked over theIteratortrait with such an ambiguity.ranile commentedon Aug 23, 2022
How exactly is this method a breaking change? I looked through the comments here and in the tracking issue and couldn't find a minimal example showing the breakage of the API
yaahc commentedon Aug 23, 2022
@hamza1311 it causes ambiguous name resolution errors on files that already use the itertools intersperse method because now both the itertools and iterator traits are in scope and have a method with the same name, so the compiler goes from having one option to two options and produces an error because it can't pick arbitrarily from two options for which intersperse method to use.
The crater results mark linked in the top comment are where all the examples live. Here's one, scroll to the end to see the error
https://crater-reports.s3.amazonaws.com/beta-1.56-2/beta-2021-09-12/gh/Centril.consensusbot/log.txt
ckaran commentedon Sep 27, 2022
@yaahc wrote:
Is this still the plan?
cdstanford commentedon Nov 10, 2022
If
.intersperse()causes name resolution conflicts, the obvious thing to do is to rename. But why not also offer.join()which combines intersperse with collect (i.e., what users of this API actually want to do)?jswrenn commentedon Nov 10, 2022
Renaming is a somewhat unsatisfying solution. That approach sets us up to have our own little smooshgate potentially every time a new method is added to the standard library. I am considering renaming every method in itertools to start with
it_, but that'll only be a partial solution. A considerable number of itertools users aren't on the latest release.RFC 2845 establishes a mechanism for reducing the breakage resulting from most of these situations, but requires implementation.
cdstanford commentedon Nov 10, 2022
I don't see how it's analogous to smooshgate, because monkeypatching doesn't exist in Rust. itertools isn't using bad practice here.
There are plenty of reasonable alternative names like
.weaveor.interleave_item. How does choosing one of these set a bad precedent?the8472 commentedon Nov 10, 2022
It is analogous in the sense that the respective standard library should not base its naming decisions on what downstream libs do. It's a violation of namespacing. That not fully qualified method resolution can lead to such conflicts is a known issue but it's better to resolve that rather than working around it.
cdstanford commentedon Nov 10, 2022
I agree, but as a short-term solution renaming prevents this being blocked on what is a much bigger issue with Rust's namespacing. Otherwise it seems like this (and many similar stdlib improvements) are blocked for the near-term forseeable future, if I'm understanding the discussion correctly.
the8472 commentedon Nov 11, 2022
The RFC fixing the issue is already accepted and only needs to be implemented. Itertools provides that API for people that need it today, so there's no reason to rush.