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.

Sign up for GitHub

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

Jump to bottom

Make double ended searchers use dependent fingers #47208

Merged
merged 2 commits into from
Jan 8, 2018
Merged

Make double ended searchers use dependent fingers #47208

merged 2 commits into from
Jan 8, 2018

Conversation

Manishearth
Copy link
Member

(fixes #47175)

r? @BurntSushi @alexcrichton

needs uplift to beta

@Manishearth Manishearth mentioned this pull request Jan 5, 2018
Closed
@eddyb eddyb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 5, 2018
kennytm
kennytm reviewed Jan 5, 2018
View reviewed changes
src/libcore/str/pattern.rs Outdated
@@ -304,7 +304,7 @@ unsafe impl<'a> Searcher<'a> for CharSearcher<'a> {
fn next_match(&mut self) -> Option<(usize, usize)> {
loop {
// get the haystack after the last character found
let bytes = if let Some(slice) = self.haystack.as_bytes().get(self.finger..) {
let bytes = if let Some(slice) = self.haystack.as_bytes().get(self.finger..self.finger_back) {
Copy link
Member

Choose a reason for hiding this comment

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

[00:03:59] tidy error: /checkout/src/libcore/str/pattern.rs:307: line longer than 100 chars

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2018
@kennytm
Copy link
Member

kennytm commented Jan 5, 2018

Please revert the clippy submodule update.

[00:01:29] error: the lock file needs to be updated but --locked was passed to prevent this
[00:01:29] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:29] Build completed unsuccessfully in 0:00:00
[00:01:29] make: *** [prepare] Error 1
[00:01:29] Makefile:76: recipe for target 'prepare' failed
[00:01:29] The command has failed after 5 attempts.

@Manishearth
Copy link
Member Author

done.

pnkfelix
pnkfelix reviewed Jan 5, 2018
View reviewed changes
src/libcore/str/pattern.rs Outdated
@@ -304,7 +304,8 @@ unsafe impl<'a> Searcher<'a> for CharSearcher<'a> {
fn next_match(&mut self) -> Option<(usize, usize)> {
loop {
// get the haystack after the last character found
let bytes = if let Some(slice) = self.haystack.as_bytes().get(self.finger..) {
let bytes = if let Some(slice) =self.haystack.as_bytes()
Copy link
Member

Choose a reason for hiding this comment

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

nit: missing space after the = in if let Some(slice) =self.haystack.as_bytes()

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@pnkfelix
Copy link
Member

pnkfelix commented Jan 5, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Jan 5, 2018

📌 Commit a50b2f1 has been approved by pnkfelix

@pnkfelix pnkfelix 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 5, 2018
@kennytm
Copy link
Member

kennytm commented Jan 5, 2018

@bors r-

A new test failed.

[01:14:17] failures:
[01:14:17] 
[01:14:17] ---- pattern::double_ended_regression_test stdout ----
[01:14:17] 	thread 'pattern::double_ended_regression_test' panicked at 'assertion failed: `(left == right)`
[01:14:17]   left: `[InRange(0, 1), Done, Done, Done]`,
[01:14:17]  right: `[InRange(0, 1), InRange(10, 11), InRange(5, 6), Done]`: alternating double ended search', /checkout/src/libcore/../libcore/tests/pattern.rs:270:5
[01:14:17] 
[01:14:17] 
[01:14:17] failures:
[01:14:17]     pattern::double_ended_regression_test
[01:14:17] 
[01:14:17] test result: FAILED. 704 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

@kennytm kennytm 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 5, 2018
@Manishearth
Copy link
Member Author

This is weird, the tests worked fine locally. Perhaps something changed with the merge commit?

@Manishearth
Copy link
Member Author

lol so it turns out if you write a crate using #[test], cargo test followed by cargo +otherrustc test will not recompile the tests even though the compiler changed.

Found the bug though.

@Manishearth
Copy link
Member Author

Should work now, I forgot to add self.finger to index in the reverse iterator.

@Manishearth
Copy link
Member Author

r? @pnkfelix

@rust-highfive rust-highfive assigned pnkfelix and unassigned BurntSushi Jan 6, 2018
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 6, 2018
@Manishearth
Copy link
Member Author

Tests pass.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 8, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Jan 8, 2018

📌 Commit 9066219 has been approved by pnkfelix

@pnkfelix pnkfelix 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 8, 2018
@kennytm
Copy link
Member

kennytm commented Jan 8, 2018

@bors p=2

@bors
Copy link
Contributor

bors commented Jan 8, 2018

⌛ Testing commit 9066219 with merge b5392f5...

bors added a commit that referenced this pull request Jan 8, 2018
@bors
Auto merge of #47208 - Manishearth:double-ended-searcher, r=pnkfelix
b5392f5 
Make double ended searchers use dependent fingers

(fixes #47175)

r? @BurntSushi @alexcrichton

needs uplift to beta
@bors
Copy link
Contributor

bors commented Jan 8, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing b5392f5 to master...

@bors bors merged commit 9066219 into rust-lang:master Jan 8, 2018
@Manishearth Manishearth deleted the double-ended-searcher branch January 9, 2018 08:08
@Manishearth
Copy link
Member Author

needs uplift

@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 10, 2018
@pnkfelix
Copy link
Member

(nominating just to ensure libs team looks at the beta-nomination.)

@alexcrichton alexcrichton added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed I-nominated labels Jan 10, 2018
@alexcrichton
Copy link
Member

Tagging as accepted since this fixes a regression

@MaloJaffre MaloJaffre mentioned this pull request Jan 10, 2018
Merged
bors added a commit that referenced this pull request Jan 10, 2018
@bors
Auto merge of #47327 - MaloJaffre:beta-backport, r=Mark-Simulacrum
a19122c 
[beta] Backports

Cherry-picked (cleanly) into beta:
- #46916
- #47161
- #47208
- #47269
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kennytm kennytm kennytm left review comments

@pnkfelix pnkfelix pnkfelix left review comments

Assignees

@pnkfelix pnkfelix

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Memory corruption (?) from tower-grpc-build on nightly
7 participants
@Manishearth @kennytm @pnkfelix @bors @alexcrichton @eddyb @BurntSushi