Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iterator reform broke reimplementations of methods now in IteratorExt #19435

Closed
skade opened this issue Dec 1, 2014 · 4 comments
Closed

Iterator reform broke reimplementations of methods now in IteratorExt #19435

skade opened this issue Dec 1, 2014 · 4 comments

Comments

@skade
Copy link
Contributor

skade commented Dec 1, 2014

Recently, Iterator was split into Iterator and IteratorExt, with an implementation impl<A, I: Iterator<A>> IteratorExt<A> for I.

I have an Iterator that implements last (as the underlying C-API has an efficient implementation for seeking), which now broke:

/home/vagrant/leveldb/src/database/iterator.rs:301:1: 306:2 error: conflicting implementations for trait `core::iter::IteratorExt` [E0119]
/home/vagrant/leveldb/src/database/iterator.rs:301 impl<'a, K: Key> iter::IteratorExt<Vec<u8>> for ValueIterator<'a,K,Vec<u8>> {
/home/vagrant/leveldb/src/database/iterator.rs:302   fn last(&mut self) -> Option<Vec<u8>> {
/home/vagrant/leveldb/src/database/iterator.rs:303      self.seek_last();
/home/vagrant/leveldb/src/database/iterator.rs:304      Some(self.value())
/home/vagrant/leveldb/src/database/iterator.rs:305   }
/home/vagrant/leveldb/src/database/iterator.rs:306 }
/home/vagrant/leveldb/src/database/iterator.rs:301:1: 306:2 note: conflicting implementation in crate `core`
/home/vagrant/leveldb/src/database/iterator.rs:301 impl<'a, K: Key> iter::IteratorExt<Vec<u8>> for ValueIterator<'a,K,Vec<u8>> {
/home/vagrant/leveldb/src/database/iterator.rs:302   fn last(&mut self) -> Option<Vec<u8>> {
/home/vagrant/leveldb/src/database/iterator.rs:303      self.seek_last();
/home/vagrant/leveldb/src/database/iterator.rs:304      Some(self.value())
/home/vagrant/leveldb/src/database/iterator.rs:305   }
/home/vagrant/leveldb/src/database/iterator.rs:306 }

The error is expected, but I'd expect that IteratorExt methods can be reimplemented like all others.

@Thiez
Copy link
Contributor

Thiez commented Dec 3, 2014

This problem is also in the stdlib. Using last() on, say, std::slice::Items will walk over the entire slice rather than just returning the last one right away.

@thestinger
Copy link
Contributor

The ability to override these was never an intended feature. It was a consequence of the implementation avoiding an extension trait. It doesn't make sense to do an ad-hoc override of a method like last dozens of times when random access iterators are a general purpose concept. It's also not a workable solution for methods like seek. There are better ways to approach this issue, and it should probably go through the RFC process.

@thestinger
Copy link
Contributor

The sane way to do it is providing reusable methods on the base iterator trait that can be used to build features like skip and last, and overridden by random-access iterators.

@skade
Copy link
Contributor Author

skade commented Dec 3, 2014

So how come IteratorExt still implements them as default methods? The library I am interfacing with (leveldb) also provides no way to interface with the iterator in any other fashion then skipping to "last", so I just cannot build reusable methods there.

This means that I would have to implement a non-standard iterator at that place, loosing convenient access to IteratorExt.

The underlying problem is that I cannot provide a more specific implementation taking priority over the implementation for A.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants