search

rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.72k stars 309 forks source link

Remove Clone requirement from unique #776

Closed gilhooleyd closed 1 year ago

gilhooleyd commented 1 year ago

This trait requirement appears to no longer be needed.

Philippe-Cholet commented 1 year ago

The item has to implement clone for Unique to be an iterator

jswrenn commented 1 year ago

@Philippe-Cholet's right. @gilhooleyd, could you submit a PR adding the Clone bound to the Unique definition: https://github.com/rust-itertools/itertools/blob/4d2bd4e0e98bbc9d506b83f6357787ad037ab12e/src/unique_impl.rs#L163-L170

gilhooleyd commented 1 year ago

Thanks for the quick feedback! I was surprised to see that cargo test passed with this change, but it looks like there's no test code that calls unique and then iterates over it. Adding that test and then trying to remove Clone further I see that the header comments are correct (Unique does clone each item to place it in the hashmap to look for duplicates).

I'll submit another PR to add a test and the trait