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.

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

Define a dedicated error type for HandleOrNull and HandleOrInvalid. #96195

Merged
merged 11 commits into from
Apr 27, 2022
Merged

Define a dedicated error type for HandleOrNull and HandleOrInvalid. #96195

merged 11 commits into from
Apr 27, 2022

Conversation

sunfishcode
Copy link
Member

Define NullHandleError and InvalidHandleError types, that implement std::error::Error, and use them as the error types in HandleOrNull and HandleOrInvalid,

This addresses this concern.

This is the same as #95387.

r? @joshtriplett

@sunfishcode
Define a dedicated error type for HandleOrNull and HandleOrInvalid.
703a336 
Define a `NotHandle` type, that implements `std::error::Error`, and use
it as the error type in `HandleOrNull` and `HandleOrInvalid`.
@sunfishcode
Fix an incorrect word in a comment.
5b3023c
@sunfishcode
Move the Error impl for NotHandle out of platform-independent code.
67994b7
@sunfishcode
Split NotHandle into NullHandleError and InvalidHandleError.
f934043 
Also, make the display messages more specific, and remove the `Copy`
implementation.
@sunfishcode
Add a comment explaining the (()) idiom for empty structs.
890125d
@sunfishcode
Update the expected stderr for coerce-issue-49593-box-never.
b7ff103 
This test's expected stderr now includes a count of the number of types
that implment `Error`. This PR introduces two new types, so increment
the number by two.
@rust-highfive
Copy link
Collaborator

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 19, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 19, 2022
@sunfishcode
Copy link
Member Author

Following rust-highfive's instructions, since this PR changes an existing unstable library API:

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 19, 2022
This was referenced Apr 19, 2022
Closed
Closed
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 19, 2022

📌 Commit 19ef182 has been approved by joshtriplett

@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 19, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#96195 - sunfishcode:sunfishcode/handle-or-e…
c6114bb 
…rror-type, r=joshtriplett

 Define a dedicated error type for `HandleOrNull` and `HandleOrInvalid`.

Define `NullHandleError` and `InvalidHandleError` types, that implement std::error::Error, and use them as the error types in `HandleOrNull` and `HandleOrInvalid`,

This addresses [this concern](rust-lang#87074 (comment)).

This is the same as rust-lang#95387.

r? `@joshtriplett`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#96195 - sunfishcode:sunfishcode/handle-or-e…
3fbc4e2 
…rror-type, r=joshtriplett

 Define a dedicated error type for `HandleOrNull` and `HandleOrInvalid`.

Define `NullHandleError` and `InvalidHandleError` types, that implement std::error::Error, and use them as the error types in `HandleOrNull` and `HandleOrInvalid`,

This addresses [this concern](rust-lang#87074 (comment)).

This is the same as rust-lang#95387.

r? ``@joshtriplett``
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#96195 - sunfishcode:sunfishcode/handle-or-e…
e0a4afd 
…rror-type, r=joshtriplett

 Define a dedicated error type for `HandleOrNull` and `HandleOrInvalid`.

Define `NullHandleError` and `InvalidHandleError` types, that implement std::error::Error, and use them as the error types in `HandleOrNull` and `HandleOrInvalid`,

This addresses [this concern](rust-lang#87074 (comment)).

This is the same as rust-lang#95387.

r? ```@joshtriplett```
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Closed
@Dylan-DPC
Copy link
Member

failed in rollup

@bors r-

@sunfishcode sunfishcode added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 26, 2022
@sunfishcode
Copy link
Member Author

@joshtriplett I've now added a separate windows-only version of the test, to preserve the test coverage on Windows. How does that sound?

@sunfishcode
Copy link
Member Author

I discussed this with @joshtriplett offline.

r=@joshtriplett

@sunfishcode
Copy link
Member Author

@bors r=joshtriplett

@bors
Copy link
Contributor

bors commented Apr 26, 2022

@sunfishcode: 🔑 Insufficient privileges: Not in reviewers

@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 26, 2022

📌 Commit 839cd04 has been approved by joshtriplett

@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 26, 2022
@bors
Copy link
Contributor

bors commented Apr 26, 2022

⌛ Testing commit 839cd04 with merge cc13d213371516dcfbc3ea2d4c0f8394f98c0ca1...

@bors
Copy link
Contributor

bors commented Apr 26, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 26, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 26, 2022

📌 Commit 531c937 has been approved by joshtriplett

@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 26, 2022
@bors
Copy link
Contributor

bors commented Apr 27, 2022

⌛ Testing commit 531c937 with merge bb85bca...

@bors
Copy link
Contributor

bors commented Apr 27, 2022

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing bb85bca to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 27, 2022
@bors bors merged commit bb85bca into rust-lang:master Apr 27, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 27, 2022
@bors bors mentioned this pull request Apr 27, 2022
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (bb85bca): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@sunfishcode sunfishcode deleted the sunfishcode/handle-or-error-type branch April 27, 2022 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshtriplett joshtriplett Awaiting requested review from joshtriplett

Assignees

@joshtriplett joshtriplett

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.
8 participants
@sunfishcode @rust-highfive @joshtriplett @bors @Dylan-DPC @rust-log-analyzer @rust-timer @rustbot