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

Jump to bottom

VecDeque: Use power of two capacity even for zero sized types #28494

Merged
merged 1 commit into from Sep 19, 2015
Merged

VecDeque: Use power of two capacity even for zero sized types #28494

merged 1 commit into from Sep 19, 2015

Conversation

bluss
Copy link
Member

@bluss bluss commented Sep 18, 2015

VecDeque: Use power of two capacity even for zero sized types

VecDeque depends on using a power of two capacity. Use the largest
possible power of two capacity for ZSTs.

Fixes #28488

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@bluss
Copy link
Member Author

bluss commented Sep 18, 2015

WIP -- still running tests, they probably don't compile..

@Gankra
Copy link
Contributor

Gankra commented Sep 18, 2015

r=me when you get that to compile

@bluss
Copy link
Member Author

bluss commented Sep 18, 2015

It compiled with one fix.

@Gankra
Copy link
Contributor

Gankra commented Sep 18, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Sep 18, 2015

📌 Commit 838160c has been approved by Gankro

@bluss bluss added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 18, 2015
@bluss
Copy link
Member Author

bluss commented Sep 18, 2015

beta nominating as bug fix

@eddyb
Copy link
Member

eddyb commented Sep 19, 2015

We don't have an "is power of 2" method? IIRC x & (x - 1) == 0 works.

@bluss
Copy link
Member Author

bluss commented Sep 19, 2015

@eddyb Oh, we have is_power_of_two(). Didn't know that. It uses your method.

@bluss
VecDeque: Use power of two capacity even for zero sized types
66f5dc1 
VecDeque depends on using a power of two capacity. Use the largest
possible power of two capacity for ZSTs.
@bluss
Copy link
Member Author

bluss commented Sep 19, 2015

Updated to use is_power_of_two

@eddyb
Copy link
Member

eddyb commented Sep 19, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Sep 19, 2015

📌 Commit 66f5dc1 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Sep 19, 2015

⌛ Testing commit 66f5dc1 with merge 7e25e63...

@bors
Copy link
Contributor

bors commented Sep 19, 2015

💔 Test failed - auto-linux-64-x-android-t

@bluss
Copy link
Member Author

bluss commented Sep 19, 2015

I'm not sure why the debuginfo test failed on android, but I think it's safe to retry.

@bors retry

@bors
Copy link
Contributor

bors commented Sep 19, 2015

⌛ Testing commit 66f5dc1 with merge 0b5b029...

bors added a commit that referenced this pull request Sep 19, 2015
@bors
Auto merge of #28494 - bluss:vecdeque-zst, r=eddyb
0b5b029 
VecDeque: Use power of two capacity even for zero sized types

VecDeque depends on using a power of two capacity. Use the largest
possible power of two capacity for ZSTs.

Fixes #28488
@bors
Copy link
Contributor

bors commented Sep 19, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

@bors bors merged commit 66f5dc1 into rust-lang:master Sep 19, 2015
@bluss bluss deleted the vecdeque-zst branch September 19, 2015 16:31
@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 23, 2015
@alexcrichton
Copy link
Member

The libs team decided to accept this for a backport to beta

@alexcrichton alexcrichton added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 24, 2015
@bluss
Copy link
Member Author

bluss commented Sep 24, 2015

@alexcrichton Has a stable patch / 1.3.1 been considered? Maybe this is not so serious after all(?). I don't know, but I like fixing bugs.

@alexcrichton
Copy link
Member

No, but this doesn't seem serious enough to warrant the first patch release in Rust ever.

@bluss bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 24, 2015
@bluss
Copy link
Member Author

bluss commented Sep 24, 2015

Well maybe it's the worst regression in stable rust ever? We don't have much history to draw from, is my point.

@alexcrichton alexcrichton mentioned this pull request Oct 1, 2015
Merged
@brson brson removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. relnotes Marks issues that should be documented in the release notes of the next release. 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.

None yet
8 participants
@bluss @rust-highfive @Gankra @bors @eddyb @alexcrichton @brson @nikomatsakis