Itertools 0.13.0 breaks strum derive macros if both are in scope at the same time.

[VorpalBlade]

// Uncomment this line and this fails to build (with itertools 0.13.0, works with 0.12.1)
// use itertools::Itertools;

#[derive(Debug, strum::EnumIter)]
enum A {
    B,
    C,
}

This leads to the very strange following error if the mentioned line is uncommented:

   Compiling itertools_strum v0.1.0 (/home/arvid/src/itertools_strum)
error[E0277]: the trait bound `usize: IteratorIndex<&mut AIter>` is not satisfied
   --> src/main.rs:4:17
    |
4   | #[derive(Debug, strum::EnumIter)]
    |                 ^^^^^^^^^^^^^^^ the trait `IteratorIndex<&mut AIter>` is not implemented for `usize`
    |
    = help: the following other types implement trait `IteratorIndex<I>`:
              RangeFull
              std::ops::Range<usize>
              RangeFrom<usize>
              RangeTo<usize>
              RangeInclusive<usize>
              RangeToInclusive<usize>
note: required by a bound in `itertools::Itertools::get`
   --> /home/arvid/.cargo/registry/src/index.crates.io-6f17d22bba15001f/itertools-0.13.0/src/lib.rs:554:12
    |
551 |     fn get<R>(self, index: R) -> R::Output
    |        --- required by a bound in this associated function
...
554 |         R: traits::IteratorIndex<Self>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Itertools::get`
    = note: this error originates in the derive macro `strum::EnumIter` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `itertools_strum` (bin "itertools_strum") due to 2 previous errors

I have no idea if this is a strum or itertools bug. As such I have reported this to both, see Peternator7/strum#358

Using rustc 1.78.0 (9b00956e5 2024-04-29).

[dependencies]
itertools = "0.13.0"
strum = { version = "0.26.2", features = ["derive"] }

[jswrenn]

It's a classic macro hygiene issue in strum's expansion. strum is generating an inherent method called get:

impl AIter {
    fn get(&self, idx: usize) -> ::core::option::Option<A> {
        match idx {
            0usize => ::core::option::Option::Some(A::B),
            1usize => ::core::option::Option::Some(A::C),
            _ => ::core::option::Option::None,
        }
    }
}

...and calling it without using the fully-qualified syntax; e.g.:

impl DoubleEndedIterator for AIter {
    fn next_back(&mut self) -> ::core::option::Option<<Self as Iterator>::Item> {
        let back_idx = self.back_idx + 1;
        if self.idx + back_idx > 2usize {
            self.back_idx = 2usize;
            ::core::option::Option::None
        } else {
            self.back_idx = back_idx;
            self.get(2usize - self.back_idx)
        }
    }
}

If my understanding of method resolution is correct, rustc first searches for an applicable get with a &mut self receiver in the inherent impl of AIter, then in the trait impls, and then — failing that — repeats the process for gets with a &self receiver. Consequently, the get(&mut self) from Itertools is resolved before the fn get(&self) inherent method.

The fix is for strum to use fully qualified invocations instead; e.g.:

AIter::get(self, 2usize - self.back_idx)

[VorpalBlade]

Fair enough, as a side note, get there is a very generic name. Wouldn't having prefixed names or at least somewhat less common words be better? Then you would also avoid users getting warnings like this:

warning: a method with this name may be added to the standard library in the future
  --> chezmoi_modify_manager/src/transforms.rs:62:18
   |
62 |             docs.intersperse("\n\n".to_string()).collect::<String>()
   |                  ^^^^^^^^^^^
   |
   = warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior!
   = note: for more information, see issue #48919 <https://github.com/rust-lang/rust/issues/48919>
   = help: call with fully qualified syntax `itertools::Itertools::intersperse(...)` to keep using the current method
   = note: `#[warn(unstable_name_collisions)]` on by default

The only way to get rid of that warning is to use a fully qualified form, which of course breaks method chaining (.a().b().c()...). With that in mind it seems a bit odd to add new functions that risks running into that.

[jswrenn]

We opted for consistency with slice::get. Since there is no risk of slices implementing Iterator, there's no risk of a name collision risk there. There is some risk of a name collision with Iterator::get (if it's ever added), but we did our due diligence by pitching the feature to the Rust libs team first (rust-lang/rust#73482). Even so, if they do someday decide to adopt Iterator::get, breakage there will be avoided by the implementation of rust-lang/rfcs#3624.

Especially given that there is a language-level solution to the problem of super trait method collisions on the horizon, I think it's very unlikely that we'd take the step of prefixing the names of all itertools methods. That would be a tremendous breaking change to pass on to our users.

[VorpalBlade]

Feel free to close this issue (or maybe you want to keep it open for others to find over the next few days?) as there is nothing to do on the itertools side of thing.

[Philippe-Cholet]

Itertools::get(self) (it takes ownership) and not get(&mut self) but that probably does not change the result of method resolution (I'm not familiar with it though).

It's a classic macro hygiene issue in strum's expansion.

It would be nice if there was a clippy lint that would help authors of libraries like strum with macro hygiene, suggesting to qualify calls (here to AIter::get). Might be tricky to do though.

[VorpalBlade]

It would be nice if there was a clippy lint that would help authors of libraries like strum with macro hygiene, suggesting to qualify calls (here to AIter::get). Might be tricky to do though.

Good idea, though this is the wrong place to pursue that. Either the rust issue tracker or https://internals.rust-lang.org would be appropriate places for this.

[fncnt]

I hope you don't mind me adding to this. I don't think the following warrants a separate issue but I wanted to document this somewhere and figured this would be a good place if people come across this quirk:

Range, RangeFrom and RangeInclusive implement Iterator (and therefore Itertools), as well as core::slice::SliceIndex.
The latter has an unstable trait method get().

So this example also requires qualified paths:

#![feature(slice_index_methods)]
use core::slice::SliceIndex;
use itertools::Itertools;

fn main() {
    let r = 0..10;
    r.get(1..3);
    // Disambiguation:
    //Itertools::get(r, 1..3);
}

I do think it's slightly unfortunate. However, slice_index_methods is unstable and my impression was that it's not really likely to get stabilized.

[Philippe-Cholet]

Yes it's a bit unfortunate. However, I heard that ranges are discussed to not be iterators anymore but only IntoIterator for them to be Copy in which case there would be no conflict in recent Rust.

---

EDIT: The fix has been merged in strum_macros 0.26.3. Unpinning this.