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

Add Iterator::intersperse_with #80567

Merged
merged 4 commits into from
Jan 14, 2021
Merged

Conversation

lukaslueg
Copy link
Contributor

@lukaslueg lukaslueg commented Dec 31, 2020

This is a follow-up to #79479, tracking in #79524, as discussed #79479 (comment).

Note that I had to manually implement Clone and Debug because derive insists on placing a Clone-bound on the struct-definition, which is too narrow. There is a long-standing issue # for this somewhere around here :-)

Also, note that I refactored the guts of Intersperse into private functions and re-used them in IntersperseWith, so I also went light on duplicating all the tests.

If this is suitable to be merged, the tracking issue should be updated, since it only mentions intersperse.

Happy New Year!

r? @m-ou-se

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2020
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.................................................................................................... 9000/11231
.................................................................................................... 9100/11231
.................................................................................................... 9200/11231
..................i......i.......................................................................... 9300/11231
.........................................................iiiiii..iiiiii.i........................... 9400/11231
.................................................................................................... 9600/11231
.................................................................................................... 9700/11231
.................................................................................................... 9800/11231
.................................................................................................... 9900/11231
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 27 tests
iiiiiiiiiiiiiiiiiiiiiiiiiii

 finished in 0.096 seconds
Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii.....ii....ii..........iiii.........i.....i....i......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.51s

 finished in 2.601 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
     |
3605 |     };
     |      ^ help: remove this semicolon
     |
     = note: `-D redundant-semicolons` implied by `-D warnings`
error: aborting due to previous error

error: could not compile `core`


To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "-p" "core" "--" "--quiet"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:21:48

@lukaslueg
Copy link
Contributor Author

Removes the FIXME-notes due to this advisory.

@lukaslueg
Copy link
Contributor Author

@m-ou-se poke :-)

Copy link
Member

@m-ou-se m-ou-se left a comment

Choose a reason for hiding this comment

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

This looks great!

I have a small nit and a question:

library/core/src/iter/adapters/intersperse.rs Outdated Show resolved Hide resolved
library/core/src/iter/adapters/intersperse.rs Show resolved Hide resolved
library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved
@m-ou-se m-ou-se added A-iterators Area: Iterators T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 13, 2021
@lukaslueg
Copy link
Contributor Author

lukaslueg commented Jan 13, 2021

All addressed

Copy link
Member

@m-ou-se m-ou-se left a comment

Choose a reason for hiding this comment

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

Thanks! This is ready to merge then, with two typos fixed:

library/core/src/iter/adapters/intersperse.rs Outdated Show resolved Hide resolved
library/core/src/iter/adapters/intersperse.rs Outdated Show resolved Hide resolved
library/core/src/iter/traits/iterator.rs Outdated Show resolved Hide resolved
library/core/src/iter/adapters/intersperse.rs Show resolved Hide resolved
@lukaslueg
Copy link
Contributor Author

All fixed, addressed the example a bit. gtg imo

@m-ou-se
Copy link
Member

m-ou-se commented Jan 14, 2021

Thanks for working on this!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 14, 2021

📌 Commit 9b2f085 has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2021
@lukaslueg
Copy link
Contributor Author

Thanks for working on this!

@m-ou-se Thanks for your review!

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 14, 2021
Add Iterator::intersperse_with

This is a follow-up to rust-lang#79479, tracking in rust-lang#79524, as discussed rust-lang#79479 (comment).

~~Note that I had to manually implement `Clone` and `Debug` because `derive` insists on placing a `Clone`-bound on the struct-definition, which is too narrow. There is a long-standing issue # for this somewhere around here :-)~~

Also, note that I refactored the guts of `Intersperse` into private functions and re-used them in `IntersperseWith`, so I also went light on duplicating all the tests.

If this is suitable to be merged, the tracking issue should be updated, since it only mentions `intersperse`.

Happy New Year!

r? `@m-ou-se`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2021
Rollup of 17 pull requests

Successful merges:

 - rust-lang#79982 (Add missing methods to unix ExitStatusExt)
 - rust-lang#80017 (Suggest `_` and `..` if a pattern has too few fields)
 - rust-lang#80169 (Recommend panic::resume_unwind instead of panicking.)
 - rust-lang#80217 (Add a `std::io::read_to_string` function)
 - rust-lang#80444 (Add as_ref and as_mut methods for Bound)
 - rust-lang#80567 (Add Iterator::intersperse_with)
 - rust-lang#80829 (Get rid of `DepConstructor`)
 - rust-lang#80895 (Fix handling of malicious Readers in read_to_end)
 - rust-lang#80966 (Deprecate atomic::spin_loop_hint in favour of hint::spin_loop)
 - rust-lang#80969 (Use better ICE message when no MIR is available)
 - rust-lang#80972 (Remove unstable deprecated Vec::remove_item)
 - rust-lang#80973 (Update books)
 - rust-lang#80980 (Fixed incorrect doc comment)
 - rust-lang#80981 (Fix -Cpasses=list and llvm version print with -vV)
 - rust-lang#80985 (Fix stabilisation version of slice_strip)
 - rust-lang#80990 (llvm: Remove the unused context from CreateDebugLocation)
 - rust-lang#80991 (Fix formatting specifiers doc links)

Failed merges:

 - rust-lang#80944 (Use Option::map_or instead of `.map(..).unwrap_or(..)`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 446ed77 into rust-lang:master Jan 14, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 14, 2021
@lukaslueg lukaslueg deleted the intersperse_with branch January 15, 2021 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-iterators Area: Iterators S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants