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

Make split_whitespace use split_terminator #49818

Closed
wants to merge 16 commits into from

Conversation

clarfonthey
Copy link
Contributor

Presumably, this method didn't exist when split_whitspace was originally written, but this helps reduce code complexity a little. This is rebased on top of #49698 to avoid merge conflicts; when reviewing this, specifically look at the changes made in clarfonthey/rust@8b8ae47a to avoid the noise.

@rust-highfive
Copy link
Collaborator

r? @cramertj

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 9, 2018
@TimNN
Copy link
Contributor

TimNN commented Apr 9, 2018

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
Resolving deltas: 100% (613524/613524), completed with 4886 local objects.
---
[00:00:45] configure: rust.quiet-tests     := True
---
[00:02:58] error: expected `,`, or `}`, found `>`
[00:02:58]     --> libcore/str/mod.rs:2608:45
[00:02:58]      |
[00:02:58] 2608 |     inner: SplitTerminator<'a, IsWhitespace>>,
[00:02:58]      |                                             ^
[00:02:58]      |
[00:02:58]      = help: struct fields should be separated by commas
[00:02:58]
[00:03:00]    Compiling std v0.0.0 (file:///checkout/src/libstd)
[00:03:01] error: unused import: `Filter`
[00:03:01]   --> libcore/str/mod.rs:22:52
[00:03:01]    |
[00:03:01] 22 | use iter::{Map, Cloned, FusedIterator, TrustedLen, Filter};
---
[00:03:01]    = note: #[deny(unused_imports)] implied by #[deny(warnings)]
[00:03:01]
[00:03:04]    Compiling compiler_builtins v0.0.0 (file:///checkout/src/rustc/compiler_builtins_shim)
[00:03:04]    Compiling cmake v0.1.29
[00:03:04]    Compiling alloc_jemalloc v0.0.0 (file:///checkout/src/liballoc_jemalloc)
[00:03:06] error[E0392]: parameter `'a` is never used
[00:03:06]     --> libcore/str/mod.rs:2607:28
[00:03:06]      |
[00:03:06] 2607 | pub struct SplitWhitespace<'a> {
[00:03:06]      |                            ^^ unused type parameter
[00:03:06]      |
[00:03:06]      = help: consider removing `'a` or using a marker such as `marker::PhantomData`
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:1ccc4ac5:start=1523307667211641695,finish=1523307667217541214,duration=5899519
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:0589dbd0
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:0589dbd0:start=1523307667222799634,finish=1523307667228358331,duration=5558697
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:12d92bde
$ dmesg | grep -i kill
[   10.595163] init: failsafe main process (1094) killed by TERM signal

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@ollie27
Copy link
Member

ollie27 commented Apr 10, 2018

split_whitespace() and split_terminator(char::is_whitespace) don't do the same thing:

println!("{:?}", "  a  b  c  ".split_whitespace().collect::<Vec<&str>>());
println!("{:?}", "  a  b  c  ".split_terminator(char::is_whitespace).collect::<Vec<&str>>());

prints:

["a", "b", "c"]
["", "", "a", "", "b", "", "c", ""]

@cramertj
Copy link
Member

r? @SimonSapin

@rust-highfive rust-highfive assigned SimonSapin and unassigned cramertj Apr 10, 2018
@SimonSapin
Copy link
Contributor

As @ollie27 said, it looks like the premise of this change is not valid. @clarcharr, are we missing something?

@SimonSapin SimonSapin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 10, 2018
@bors
Copy link
Contributor

bors commented Apr 11, 2018

☔ The latest upstream changes (presumably #49715) made this pull request unmergeable. Please resolve the merge conflicts.

@clarfonthey
Copy link
Contributor Author

I honestly don't know why I misread the usage of split_terminator, so, I'm just going to close this.

@clarfonthey clarfonthey deleted the split_term branch April 12, 2018 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants