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

Use more associated types in core::iter. #23025

Merged
merged 1 commit into from
Mar 7, 2015

Conversation

huonw
Copy link
Member

@huonw huonw commented Mar 4, 2015

This concretely improves type inference of some cases (see included
test). I assume the compiler struggles to reason about multiple layers
of generic type parameters (even with associated-type equalities) but
can understand pure associated types, since they are always directly
computable from the input types.

Thanks to @shepmaster for noticing the issue with Cloned (I took that example as a test case).

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -1400,11 +1396,11 @@ pub struct Chain<A, B> {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<T, A, B> Iterator for Chain<A, B> where A: Iterator<Item=T>, B: Iterator<Item=T> {
type Item = T;
impl<A: Iterator, B> Iterator for Chain<A, B> where B: Iterator<Item = A::Item> {
Copy link
Member

Choose a reason for hiding this comment

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

One bound inside <> and other inside where? Any reason why you did this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Moving one inside <>~ allows one to useA::Item`, but keeping as much as possible outside minimises the clutter.


fn next(&mut self) -> Option<T> {
fn next(&mut self) -> Option<<I::Item as Deref>::Target> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I've taken to using <Self as Iterator>::Item here when the actual expression for Item is complicated. More DRY.

@Gankra
Copy link
Contributor

Gankra commented Mar 4, 2015

r=me with stylistic nits addressed. (the first one I'll leave up to you)

@nikomatsakis
Copy link
Contributor

The fact that type inference failed is probably a bug but this seems fine in any case.

impl<I> Iterator for Cloned<I> where
I: Iterator,
I::Item: Deref,
<I::Item as Deref>::Target: Clone
Copy link
Member

Choose a reason for hiding this comment

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

I believe that resolution was fixed recently such that I::Item::Target: Clone should work. But perhaps the fix is not in a snapshot yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

:O really??

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately it doesn't seem to work: although, things did change recently, so that I::Item works even if the I: Iterator bound is in a where clause (which @nagisa thankfully pointed out on an earlier version of this PR, avoiding some unnecessary inconsistency).

This concretely improves type inference of some cases (see included
test). I assume the compiler struggles to reason about multiple layers
of generic type parameters (even with associated-type equalities) but
*can* understand pure associated types, since they are always directly
computable from the input types.
@huonw
Copy link
Member Author

huonw commented Mar 5, 2015

@bors r=Gankro 7bcf rollup

@huonw
Copy link
Member Author

huonw commented Mar 5, 2015

@nikomatsakis filed #23065.

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 6, 2015
 This concretely improves type inference of some cases (see included
test). I assume the compiler struggles to reason about multiple layers
of generic type parameters (even with associated-type equalities) but
*can* understand pure associated types, since they are always directly
computable from the input types.

Thanks to @shepmaster for noticing the issue with `Cloned` (I took that example as a test case).
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 6, 2015
 This concretely improves type inference of some cases (see included
test). I assume the compiler struggles to reason about multiple layers
of generic type parameters (even with associated-type equalities) but
*can* understand pure associated types, since they are always directly
computable from the input types.

Thanks to @shepmaster for noticing the issue with `Cloned` (I took that example as a test case).
@bors bors merged commit 7bcf7fb into rust-lang:master Mar 7, 2015
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

Successfully merging this pull request may close these issues.

7 participants