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.

Sign up for GitHub

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

Jump to bottom

WIP move ordered iterators to associated items #78

Merged
merged 4 commits into from
Mar 7, 2015
Merged

WIP move ordered iterators to associated items #78

merged 4 commits into from
Mar 7, 2015

Conversation

Gankra
Copy link
Owner

@Gankra Gankra commented Jan 10, 2015

Doesn't work because of this ICE: rust-lang/rust#20220

But I think this is the right approach.

@ghost
Copy link

ghost commented Jan 10, 2015

:o this is much nicer.

@apasel422 apasel422 mentioned this pull request Feb 8, 2015
Closed
@Gankra
Copy link
Owner Author

Gankra commented Feb 20, 2015

Bah, the ICE persists

apasel422
apasel422 reviewed Feb 26, 2015
View reviewed changes
src/iter/ordered_iter.rs
#[cfg(feature="trie_map")]
impl<'a> OrderedSetIterator<usize> for trie_set::Iter<'a> {}
impl<'a, K, V> OrderedMapIterator<Key=&'a K, Val=&'a V> for tree_map::Iter<'a, K, V> {}
#[cfg(feature="tree_map")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/tree_map/trie_map/

@Gankra
Copy link
Owner Author

Gankra commented Feb 26, 2015

Current roadblock:

pub trait OrderedMapIterator: Iterator<Item=(<Self as OrderedMapIterator>::Key, <Self as OrderedMapIterator>::Val)> + Sized {
    type Key;
    type Val;

Illegal recursive type.

@apasel422
Copy link
Contributor

Oh, yeah. That is rust-lang/rust#20825.

@apasel422
Copy link
Contributor

If you rebase on master, this will build now. You can add

[profile.dev]
debug = false

to Cargo.toml to avoid the issue I referenced earlier. While you're at it, we can remove #![feature(old_impl_check)] from lib.rs.

@Gankra
Copy link
Owner Author

Gankra commented Mar 7, 2015

I'm still seeing the error. Do you mean rebase on rust master? As in the next nightly?

@apasel422
Copy link
Contributor

Using today's nightly (rustc 1.0.0-nightly (b0746ff19 2015-03-05) (built 2015-03-06)), I rebased your commits on collect-rs/master and it built and tested successfully after I modified Cargo.toml as above.

@Gankra
Copy link
Owner Author

Gankra commented Mar 7, 2015

Ah I'm a nightly back. On a bad internet connection, want to finish this PR up?

@apasel422
Copy link
Contributor

Sure. Let me take a look.

On Fri, Mar 6, 2015 at 7:16 PM, Alexis Beingessner <[email protected]

wrote:

Ah I'm a nightly back. On a bad internet connection, want to finish this
PR up?

—
Reply to this email directly or view it on GitHub
#78 (comment).

apasel422 added a commit that referenced this pull request Mar 7, 2015
@apasel422
Merge pull request #78 from Gankro/order-iter
f259281 
WIP move ordered iterators to associated items
@apasel422 apasel422 merged commit f259281 into master Mar 7, 2015
@Gankra
Copy link
Owner Author

Gankra commented Mar 7, 2015

🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gankra @apasel422