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

Map clone #22276

Closed
wants to merge 4 commits into from
Closed

Map clone #22276

wants to merge 4 commits into from

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented Feb 13, 2015

This resolves #22243 for the single-letter variables that I could grep. Some occurences could not be replaced.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@nikomatsakis
Copy link
Contributor

@bors r+ c9ad0d1 rollup

@nikomatsakis
Copy link
Contributor

much nicer, thanks!

@steveklabnik
Copy link
Member

looks good to me

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Feb 13, 2015
This resolves rust-lang#22243 for the single-letter variables that I could grep. Some occurences could not be replaced.
@Gankra
Copy link
Contributor

Gankra commented Feb 13, 2015

I believe the collection ones you caught are done in #22242 as well.

Although yours got r'd first so I guess I'll go revert mine.

Unless Git is smart enough to deal with identical changes?

Regardless, it seems like you missed a lot of the occurrences in collections, perhaps there was a flaw in your search pattern? (I basically ctrl+f'd for iter().map in all the collections files since I'm bad at sed/grep).

Ah you didn't do any of map(|&x| x). Cool.

@ruuda
Copy link
Contributor Author

ruuda commented Feb 13, 2015

Oops, no I forgot those. (And maybe some other cases that are disguised better. I grepped for map(|.| \*.) and the others.) @Ryman: do you want to take care of those in #22287?

@Ryman
Copy link
Contributor

Ryman commented Feb 13, 2015

@ruud-v-a Yeah, I'll take care of them when rebasing once the rollup lands later :) I'll also try to avoid the ones @gankro has in their patch.

@Gankra
Copy link
Contributor

Gankra commented Feb 13, 2015

If you exclude any path that includes collections you should be fine.

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Feb 14, 2015
This resolves rust-lang#22243 for the single-letter variables that I could grep. Some occurences could not be replaced.
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Feb 15, 2015
This resolves rust-lang#22243 for the single-letter variables that I could grep. Some occurences could not be replaced.
@Manishearth
Copy link
Member

This was causing lots of failures in Steve's rollup: #22352

@Manishearth
Copy link
Member

Failed in the rollup again. Issue with Windows specifically.

http://buildbot.rust-lang.org/builders/auto-win-32-nopt-t/builds/3322/steps/compile/logs/stdio

I recommend rollup- on this PR, needs some individual shepherding.

@Manishearth
Copy link
Member

C:\bot\slave\auto-win-32-nopt-t\build\src\libstd\sys\windows\os.rs:307:41: 307:46 error: type `core::iter::Range<isize>` does not implement any method in scope named `len`
C:\bot\slave\auto-win-32-nopt-t\build\src\libstd\sys\windows\os.rs:307     fn len(&self) -> usize { self.range.len() }
                                                                                                               ^~~~~
C:\bot\slave\auto-win-32-nopt-t\build\src\libstd\sys\windows\thread_local.rs:247:42: 247:51 error: the trait `core::clone::Clone` is not implemented for the type `unsafe extern "C" fn(*mut u8)` [E0277]
C:\bot\slave\auto-win-32-nopt-t\build\src\libstd\sys\windows\thread_local.rs:247                 (*DTORS).iter().cloned().collect()
                                                                                                                          ^~~~~~~~~
error: aborting due to 2 previous errors
/c/bot/slave/auto-win-32-nopt-t/build/mk/target.mk:165: recipe for target 'i686-pc-windows-gnu/stage0/bin/rustlib/i686-pc-windows-gnu/lib/stamp.std' failed
make: *** [i686-pc-windows-gnu/stage0/bin/rustlib/i686-pc-windows-gnu/lib/stamp.std] Error 101

@pnkfelix
Copy link
Member

@bors rollup-

1 similar comment
@Gankra
Copy link
Contributor

Gankra commented Feb 17, 2015

@bors rollup-

@pnkfelix
Copy link
Member

@bors r-

@pnkfelix
Copy link
Member

(I agree with @Manishearth , this might need to go through runs on the try-server or something.)

@ruuda
Copy link
Contributor Author

ruuda commented Feb 17, 2015

My local test suite passed, is there anything I can do?

@pnkfelix
Copy link
Member

@ruud-v-a are you running on a Windows machine?

If not, then it might be good for you to remove the windows-related changes, since those were what caused problems here, from what I can tell.

@ruuda
Copy link
Contributor Author

ruuda commented Feb 17, 2015

I do have a Windows box, but I never succeeded in compiling Rust there, will try now.

@Ryman
Copy link
Contributor

Ryman commented Feb 17, 2015

This (including the fix for the failure) is covered in #22287, which seems to have gotten stuck in the buildqueue, so perhaps we can just close this one off?

@Gankra
Copy link
Contributor

Gankra commented Feb 17, 2015

Sounds good to me.

@Gankra Gankra closed this Feb 17, 2015
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 19, 2015
This overlaps with rust-lang#22276 (I left make check running overnight) but covers a number of additional cases and has a few rewrites where the clones are not even necessary.

This also implements `RandomAccessIterator` for `iter::Cloned`

cc @steveklabnik, you may want to glance at this before rust-lang#22281 gets the bors treatment
@ruuda ruuda deleted the map-clone branch November 30, 2016 09:59
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.

Purge map(|&x| x) et al
8 participants