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

Debug-print error when using rtunwrap #60204

Merged
merged 1 commit into from
May 1, 2019

Conversation

jethrogb
Copy link
Contributor

@jethrogb jethrogb commented Apr 23, 2019

When I added this macro a while back I didn't have a way to make it print the failure for all types that you might want to unwrap. Now, I came up with a solution.

@rust-highfive
Copy link
Collaborator

r? @Kimundi

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 23, 2019
@Centril
Copy link
Contributor

Centril commented Apr 28, 2019

r? @alexcrichton

@alexcrichton
Copy link
Member

Could the PR description and/or commit be expanded with rationale for the change and/or how this came up? This function is often used to be a code-size-wise light alternative to panic!, so it may be best as well to only do this when debug assertions are enabled.

@jethrogb
Copy link
Contributor Author

Could the PR description and/or commit be expanded with rationale for the change and/or how this came up?

Done

code-size-wise light alternative to panic!

In my mind, it's more of an “abort immediately” alternative to panic:

  • panic! => rtabort!
  • assert! => rtassert!
  • unwrap => rtunwrap!

unwrap prints the error before panicking, therefore rtunwrap! should too.

only do this when debug assertions are enabled

rtabort! doesn't do this.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 30, 2019

📌 Commit 942831e 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 Apr 30, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 30, 2019
…=alexcrichton

Debug-print error when using rtunwrap

When I added this macro a while back I didn't have a way to make it print the failure for all types that you might want to unwrap. Now, I came up with a solution.
bors added a commit that referenced this pull request Apr 30, 2019
Debug-print error when using rtunwrap

When I added this macro a while back I didn't have a way to make it print the failure for all types that you might want to unwrap. Now, I came up with a solution.
@bors
Copy link
Contributor

bors commented Apr 30, 2019

⌛ Testing commit 942831e with merge 96ee0ba...

@bors
Copy link
Contributor

bors commented May 1, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 96ee0ba to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 1, 2019
@bors bors merged commit 942831e into rust-lang:master May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants