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

Replace usages of ..i + 1 ranges with ..=i. #56516

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Replace usages of ..i + 1 ranges with ..=i. #56516

merged 1 commit into from
Dec 7, 2018

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Dec 4, 2018 •
edited
Loading

Before this change we were using old computer code techniques. After this change we use the new and improved computer code techniques.

@frewsxcv frewsxcv force-pushed the frewsxcv-eq branch 2 times, most recently from cfd8fdd to 33de72b Compare December 4, 2018 19:37
@rust-lang rust-lang deleted a comment from rust-highfive Dec 4, 2018
@rust-lang rust-lang deleted a comment from rust-highfive Dec 4, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
@leonardo-m
Copy link

Before merging this I suggest to take a look at the performance effects. Because closed intervals still are slower (but such lower performance is significant only in some situations).

@frewsxcv
Copy link
Member Author

frewsxcv commented Dec 5, 2018

@leonardo-m Is there an issue in this repo for tracking such performance issues?

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

kennytm commented Dec 5, 2018

@leonardo-m #45222 has been closed pretty long time ago.

@leonardo-m
Copy link

I know, and the problem isn't solved well yet. I'll have to reopen that issue or create a new one...

@leonardo-m leonardo-m mentioned this pull request Dec 6, 2018
Open
@leonardo-m
Copy link

I've added a little example in Issue #45222.

@kennytm
Copy link
Member

kennytm commented Dec 6, 2018

@bors try

@bors
Copy link
Contributor

bors commented Dec 6, 2018

⌛ Trying commit c025d61 with merge 941e2d7...

bors added a commit that referenced this pull request Dec 6, 2018
@bors
Auto merge of #56516 - frewsxcv:frewsxcv-eq, r=<try>
941e2d7 
Replace usages of `..i + 1` ranges with `..=i`.

Before this change we were using old computer code techniques. After this change we use the new and improved computer code techniques.
@bors
Copy link
Contributor

bors commented Dec 6, 2018

☀️ Test successful - status-travis
State: approved= try=True

@kennytm
Copy link
Member

kennytm commented Dec 6, 2018

@rust-timer build 941e2d7

@rust-timer
Copy link
Collaborator

Success: Queued 941e2d7 with parent 77a6a61, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 941e2d7

@frewsxcv
Copy link
Member Author

frewsxcv commented Dec 6, 2018

Doesn't like like there's much of a difference in performance, right?

@Mark-Simulacrum
Copy link
Member

Largely as expected -- ranges are almost definitively not where the compiler spends its time. I'm okay landing this from a performance perspective.

@Centril
Copy link
Contributor

Centril commented Dec 7, 2018

r? @Mark-Simulacrum

@estebank
Copy link
Contributor

estebank commented Dec 7, 2018

@bors r=Mark-Simulacrum (as there haven't been any updates to the PR since @Mark-Simulacrum last looked at it and it indeed looks fine to merge)

@bors
Copy link
Contributor

bors commented Dec 7, 2018

📌 Commit c025d61 has been approved by Mark-Simulacrum

@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 Dec 7, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Dec 7, 2018
@kennytm
Rollup merge of rust-lang#56516 - frewsxcv:frewsxcv-eq, r=Mark-Simula…
0e41ef1 
…crum

Replace usages of `..i + 1` ranges with `..=i`.

Before this change we were using old computer code techniques. After this change we use the new and improved computer code techniques.
@kennytm kennytm mentioned this pull request Dec 7, 2018
Merged
bors added a commit that referenced this pull request Dec 7, 2018
@bors
Auto merge of #56581 - kennytm:rollup, r=kennytm
fc84f5f 
Rollup of 7 pull requests

Successful merges:

 - #56000 (Add Armv8-M Mainline targets)
 - #56250 (Introduce ptr::hash for references)
 - #56434 (Improve query cycle errors for parallel queries)
 - #56516 (Replace usages of `..i + 1` ranges with `..=i`.)
 - #56555 (Send textual profile data to stderr, not stdout)
 - #56561 (Fix bug in from_key_hashed_nocheck)
 - #56574 (Fix a stutter in the docs for slice::exact_chunks)

Failed merges:

r? @ghost
@bors bors merged commit c025d61 into rust-lang:master Dec 7, 2018
@frewsxcv frewsxcv deleted the frewsxcv-eq branch December 10, 2018 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@frewsxcv @rust-highfive @leonardo-m @kennytm @bors @rust-timer @Mark-Simulacrum @Centril @estebank @Dylan-DPC-zz