Skip to content

Fix #18604: next_power_of_two should panic on overflow #45754

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
Merged
merged 3 commits into from
Nov 5, 2017
Merged

Fix #18604: next_power_of_two should panic on overflow #45754

merged 3 commits into from
Nov 5, 2017

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Nov 4, 2017

Fixes #18604

Is it possible to write a test for this? My experiments showed x.py test running in release mode, so my attempt at a #[should_panic] didn't work.

@rust-highfive
Copy link
Contributor

r? @dtolnay

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

dtolnay
dtolnay reviewed Nov 4, 2017
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Check out the test in #36372. Looks like there is a compile-flags magic comment.

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

kennytm commented Nov 4, 2017

Considering #36372, is it possible to use Add::add instead of #[rustc_inherit_overflow_checks]?

@dtolnay
Copy link
Member

dtolnay commented Nov 5, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 5, 2017

📌 Commit 0d745af has been approved by dtolnay

@bors
Copy link
Collaborator

bors commented Nov 5, 2017

⌛ Testing commit 0d745af with merge 4efcc66...

bors added a commit that referenced this pull request Nov 5, 2017
@bors
Auto merge of #45754 - scottmcm:checked-npot, r=dtolnay
4efcc66 
Fix #18604: next_power_of_two should panic on overflow

Fixes #18604

Is it possible to write a test for this?  My experiments showed `x.py test` running in release mode, so my attempt at a `#[should_panic]` didn't work.
@kennytm kennytm 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 Nov 5, 2017
@bors
Copy link
Collaborator

bors commented Nov 5, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: dtolnay
Pushing 4efcc66 to master...

@bors bors merged commit 0d745af into rust-lang:master Nov 5, 2017
@scottmcm scottmcm deleted the checked-npot branch November 6, 2017 01:43
@ecstatic-morse ecstatic-morse mentioned this pull request Feb 9, 2020
Merged
@scottmcm scottmcm mentioned this pull request Feb 3, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay left review comments

Assignees

@dtolnay dtolnay

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.
5 participants
@scottmcm @rust-highfive @kennytm @dtolnay @bors