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

Add non-panicking variants of pow for integer types #48321

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

milesand
Copy link
Contributor

@milesand milesand commented Feb 18, 2018

Currently, calling pow may panic in case of overflow, and the function does not have non-panicking counterparts. Thus, it would be beneficial to add those in.

Closes #48291.
Relevant tracking issue: #48320

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2018
@milesand milesand force-pushed the no_panic_pow branch 3 times, most recently from 767ada2 to a5bf3c9 Compare February 18, 2018 17:04
Currently, calling pow may panic in case of overflow, and the function
does not have non-panicking counterparts. Thus, it would be beneficial
to add those in.
@milesand
Copy link
Contributor Author

Squashed and rebased.

@dtolnay dtolnay added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 19, 2018
@alexcrichton
Copy link
Member

Thanks! Would it be possible perhaps to implement these methods in terms of one another? For example checked_pow can just call overflowing_pow, right?

@milesand
Copy link
Contributor Author

It would be possible, though that might incur some performace penalty(haven't benchmarked, so not sure). checked_pow implemented with overflowing_pow, for instance, would lack early return on overflow. I'm not quite sure about other cases, though.

@alexcrichton
Copy link
Member

Ah ok yeah that makes sense to me, the loop does indeed seem like it throws a wrench into performance here.

In that case...

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 26, 2018

📌 Commit b31ff95 has been approved by alexcrichton

@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 Feb 26, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Feb 27, 2018
Add non-panicking variants of pow for integer types

Currently, calling pow may panic in case of overflow, and the function does not have non-panicking counterparts. Thus, it would be beneficial to add those in.

Closes rust-lang#48291.
Relevant tracking issue: rust-lang#48320
@alexcrichton
Copy link
Member

@bors: rollup

kennytm added a commit to kennytm/rust that referenced this pull request Feb 28, 2018
Add non-panicking variants of pow for integer types

Currently, calling pow may panic in case of overflow, and the function does not have non-panicking counterparts. Thus, it would be beneficial to add those in.

Closes rust-lang#48291.
Relevant tracking issue: rust-lang#48320
bors added a commit that referenced this pull request Feb 28, 2018
Rollup of 15 pull requests

- Successful merges: #48266, #48321, #48365, #48381, #48450, #48473, #48479, #48484, #48488, #48497, #48541, #48548, #48558, #48560, #48565
- Failed merges:
@bors bors merged commit b31ff95 into rust-lang:master Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants