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 new opaque type predicate #92046

Closed
wants to merge 1 commit into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Dec 17, 2021

It's not emitted yet and most logic for it doesn't exist.
This commit exists solely to make future changes smaller

r? @jackh726

this is one of the changes I mean in #92007 (comment)

Merging this will make further work easier to review and less bitrotty, but this PR has no logic changes to the actual execution of the compiler.

It's not emitted yet and most logic for it doesn't exist.
This commit exists solely to make future changes smaller
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 17, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 17, 2021
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking tracing-tree v0.1.9
    Checking rustdoc-json-types v0.1.0 (/checkout/src/rustdoc-json-types)
    Checking tera v1.10.0
    Checking rustdoc v0.0.0 (/checkout/src/librustdoc)
error[E0004]: non-exhaustive patterns: `OpaqueType(_, _, _)` not covered
   --> src/librustdoc/clean/mod.rs:278:15
    |
278 |         match bound_predicate.skip_binder() {
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pattern `OpaqueType(_, _, _)` not covered
   ::: /checkout/compiler/rustc_middle/src/ty/mod.rs:615:5
    |
    |
615 |     OpaqueType(Ty<'tcx>, Ty<'tcx>, bool),
    |
    = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
    = note: the matched value is of type `PredicateKind`

Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

Is there any documentation as to why this is needed? If there is, can you made add that here somewhere? If not, can you explain a bit?

/// The arguments may be in any order, but one of them must be an opaque type.
/// The order and the boolean is used for resolving which opaque type defines
/// the other if both are opaque types and for diagnostics.
OpaqueType(Ty<'tcx>, Ty<'tcx>, bool),
Copy link
Member

Choose a reason for hiding this comment

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

Why not just make the left be the opaque type being defined, and get rid of the bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I tried that but got messed up diagnostics, but I'll try again

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

oli-obk commented Jan 12, 2022

Closing in favour of merging #92007 in one go by allocating a pair-review session with niko and lcnr

@oli-obk oli-obk closed this Jan 12, 2022
@oli-obk oli-obk deleted the solve_opaque_types branch January 12, 2022 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler 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