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

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

Merged
merged 1 commit into from Sep 19, 2015

Conversation

Projects
None yet
8 participants
@bluss
Copy link
Contributor

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

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Sep 18, 2015

r? @nikomatsakis

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

@bluss

This comment has been minimized.

Copy link
Contributor Author

bluss commented Sep 18, 2015

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

@bluss bluss force-pushed the bluss:vecdeque-zst branch from f31314c to 838160c Sep 18, 2015

@Gankro

This comment has been minimized.

Copy link
Contributor

Gankro commented Sep 18, 2015

r=me when you get that to compile

@bluss

This comment has been minimized.

Copy link
Contributor Author

bluss commented Sep 18, 2015

It compiled with one fix.

@Gankro

This comment has been minimized.

Copy link
Contributor

Gankro commented Sep 18, 2015

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 18, 2015

📌 Commit 838160c has been approved by Gankro

@bluss

This comment has been minimized.

Copy link
Contributor Author

bluss commented Sep 18, 2015

beta nominating as bug fix

@eddyb

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

bluss commented Sep 19, 2015

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

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.

@bluss bluss force-pushed the bluss:vecdeque-zst branch from 838160c to 66f5dc1 Sep 19, 2015

@bluss

This comment has been minimized.

Copy link
Contributor Author

bluss commented Sep 19, 2015

Updated to use is_power_of_two

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Sep 19, 2015

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 19, 2015

📌 Commit 66f5dc1 has been approved by eddyb

bors added a commit that referenced this pull request Sep 19, 2015

Auto merge of #28494 - bluss:vecdeque-zst, r=eddyb
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

This comment has been minimized.

Copy link
Contributor

bors commented Sep 19, 2015

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

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 19, 2015

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

@bluss

This comment has been minimized.

Copy link
Contributor 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

This comment has been minimized.

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

Auto merge of #28494 - bluss:vecdeque-zst, r=eddyb
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 bors merged commit 66f5dc1 into rust-lang:master Sep 19, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@bluss bluss deleted the bluss:vecdeque-zst branch Sep 19, 2015

@alexcrichton alexcrichton added the T-libs label Sep 23, 2015

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 24, 2015

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

@bluss

This comment has been minimized.

Copy link
Contributor 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

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 24, 2015

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

@bluss bluss added the relnotes label Sep 24, 2015

@bluss

This comment has been minimized.

Copy link
Contributor 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.