search

Lymia / enumset

A library for compact bit sets containing enums.
Apache License 2.0
91 stars 35 forks source link

Ord implementation on EnumSetIter makes it difficult to use Iterator::max #12

Closed dmarcuse closed 4 years ago

dmarcuse commented 4 years ago

EnumSetIter derives an implementation of Ord, which makes it annoying to use Iterator::max due to the conflicting name with Ord::max. This can be worked around with iter.map(|v| v).max(), or by manually disambiguating the call with Iterator::max(iter), but both solutions feel a bit hacky.

I cloned and tested a fix for this by removing the Ord implementation, and the tests still compile and pass - I'm not sure why PartialOrd/Ord are implemented on an iterator in the first place. Unfortunately, removing the Ord implementation would be a breaking change according to semantic versioning.

Lymia commented 4 years ago

... yikes. I'll have to think about how to deal with this, I must have added those by accident via copy/paste.

dmarcuse commented 4 years ago

On second thought, it should be possible to add a feature flag that disables them, which would be a (technically) backwards-compatible change, right?

Lymia commented 4 years ago

No, a crate can enable the feature flag while another crate relies on it not being set. The same problem applies to introducing a new default feature flag that can be disabled.

dmarcuse commented 4 years ago

Oh well - I guess the only option would be to set it aside for the next major release?

Lymia commented 4 years ago

I believe so. One hack for now would be to add an inherent method that delegates to Iterator::max until the next major release.

Lymia commented 4 years ago

This will be fixed in the next major release.