-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
exhaustively check ty::Kind
during structural match checking
#72153
Conversation
Best way to check that: make them
If it's not possible, best to
Yeah... |
This comment has been minimized.
This comment has been minimized.
@bors r+ rollup |
📌 Commit 60d9df2 has been approved by |
exhaustively check `ty::Kind` during structural match checking This was prone to errors as we may forget new kinds in the future. I am also not yet sure about some kinds. `ty::GeneratorWitness(..) | ty::Infer(_) | ty::Placeholder(_) | ty::UnnormalizedProjection(..) | ty::Bound(..)` might be unreachable here. We may want to forbid `ty::Projection`, similar to `ty::Param`. `ty::Opaque` seems fine afaict, should not be possible in a match atm. I believe `ty::Foreign` should not be structurally match, as I don't even know what that would actually mean. r? @pnkfelix cc @eddyb
@bors r- |
rebased |
@pnkfelix Can you once again review/r+ this? |
@bors r=pnkfelix |
📌 Commit a5a4ec9 has been approved by |
Rollup of 4 pull requests Successful merges: - rust-lang#72153 (exhaustively check `ty::Kind` during structural match checking) - rust-lang#72308 (Emit a better diagnostic when function actually has a 'self' parameter) - rust-lang#72560 (Enable `glacier` command via triagebot) - rust-lang#72567 (Clean up E0608 explanation) Failed merges: r? @ghost
This was prone to errors as we may forget new kinds in the future.
I am also not yet sure about some kinds.
ty::GeneratorWitness(..) | ty::Infer(_) | ty::Placeholder(_) | ty::UnnormalizedProjection(..) | ty::Bound(..)
might be unreachable here.We may want to forbid
ty::Projection
, similar toty::Param
.ty::Opaque
seems fine afaict, should not be possible in a match atm.I believe
ty::Foreign
should not be structurally match, as I don't even know whatthat would actually mean.
r? @pnkfelix cc @eddyb