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

Expand .zip() specialization to .map() and .cloned() #37230

Merged
merged 1 commit into from Oct 19, 2016

Conversation

Projects
None yet
4 participants
@bluss
Copy link
Contributor

bluss commented Oct 17, 2016

Implement .zip() specialization for Map and Cloned.

The crucial thing for transparent specialization is that we want to
preserve the potential side effects.

The simplest example is that in this code snippet:

(0..6).map(f).zip((0..4).map(g)).count()

f will be called five times, and g four times. The last time for f
is when the other iterator is at its end, so this element is unused.
This side effect can be preserved without disturbing code generation for
simple uses of .map().

The Zip::next_back() case is even more complicated, unfortunately.

Expand .zip() specialization to .map() and .cloned()
Implement .zip() specialization for Map and Cloned.

The crucial thing for transparent specialization is that we want to
preserve the potential side effects.

The simplest example is that in this code snippet:

`(0..6).map(f).zip((0..4).map(g)).count()`

`f` will be called five times, and `g` four times. The last time for `f`
is when the other iterator is at its end, so this element is unused.
This side effect can be preserved without disturbing code generation for
simple uses of `.map()`.

The `Zip::next_back()` case is even more complicated, unfortunately.
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 17, 2016

r? @alexcrichton

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

@bluss

This comment has been minimized.

Copy link
Contributor

bluss commented Oct 17, 2016

I don't feel this is ready yet. @Stebalien, do you want to review it? Maybe there's a simpler way?

Extending this to .map() is worth a bit of complexity, but it also casts doubt on if we can make TrustedRandomAccess into a publically available trait (?). If it gets too complex to use, I mean. The bulk of the complexity here is not in the implementations of that specialization trait though, but instead in Zip itself.

The testcases should demonstrate why keeping track of the side effects is a bit ugly. One can't just specializal case .next() and let .next_back() be.

I thought back to the "next_unchecked()" proposal but I don't think it helps -- it still needs to keep track of the length and number of elements taken the same way.

#[no_mangle]
pub fn zip_copy_mapped(xs: &[u8], ys: &mut [u8]) {
// CHECK: memcpy
for (x, y) in xs.iter().map(|&x| x).zip(ys) {

This comment has been minimized.

@bluss

bluss Oct 17, 2016

Contributor

Before this PR, this loop copies one byte at a time. So it's a nice improvement.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 19, 2016

@bors: r+

Nice wins!

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 19, 2016

📌 Commit ed50159 has been approved by alexcrichton

eddyb added a commit to eddyb/rust that referenced this pull request Oct 19, 2016

Rollup merge of rust-lang#37230 - bluss:zip-specialization-for-map, r…
…=alexcrichton

Expand .zip() specialization to .map() and .cloned()

Implement .zip() specialization for Map and Cloned.

The crucial thing for transparent specialization is that we want to
preserve the potential side effects.

The simplest example is that in this code snippet:

`(0..6).map(f).zip((0..4).map(g)).count()`

`f` will be called five times, and `g` four times. The last time for `f`
is when the other iterator is at its end, so this element is unused.
This side effect can be preserved without disturbing code generation for
simple uses of `.map()`.

The `Zip::next_back()` case is even more complicated, unfortunately.

eddyb added a commit to eddyb/rust that referenced this pull request Oct 19, 2016

Rollup merge of rust-lang#37230 - bluss:zip-specialization-for-map, r…
…=alexcrichton

Expand .zip() specialization to .map() and .cloned()

Implement .zip() specialization for Map and Cloned.

The crucial thing for transparent specialization is that we want to
preserve the potential side effects.

The simplest example is that in this code snippet:

`(0..6).map(f).zip((0..4).map(g)).count()`

`f` will be called five times, and `g` four times. The last time for `f`
is when the other iterator is at its end, so this element is unused.
This side effect can be preserved without disturbing code generation for
simple uses of `.map()`.

The `Zip::next_back()` case is even more complicated, unfortunately.

@eddyb eddyb referenced this pull request Oct 19, 2016

Merged

Rollup of 23 pull requests #37269

bors added a commit that referenced this pull request Oct 19, 2016

@bors bors merged commit ed50159 into rust-lang:master Oct 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bluss bluss added the relnotes label Oct 19, 2016

@bluss bluss deleted the bluss:zip-specialization-for-map branch Oct 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment