-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
modify alias-relate to also normalize ambiguous opaques #120549
Conversation
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
a9cdad2
to
80218a5
Compare
This comment has been minimized.
This comment has been minimized.
@@ -302,6 +302,8 @@ fn typeck_with_fallback<'tcx>( | |||
|
|||
let typeck_results = fcx.resolve_type_vars_in_body(body); | |||
|
|||
let _ = fcx.infcx.take_opaque_types(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Needs a comment at least.
@@ -562,7 +562,9 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { | |||
|
|||
#[instrument(skip(self), level = "debug")] | |||
fn visit_opaque_types(&mut self) { | |||
let opaque_types = self.fcx.infcx.take_opaque_types(); | |||
// We clone the opaques instead of stealing them here as they are still used for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's why lol
@bors r+ |
oh new solver @bors rollup |
modify alias-relate to also normalize ambiguous opaques allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable. It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless. r? `@compiler-errors`
I suspect that this failed here: |
fixed CI @bors r=compiler-errors |
modify alias-relate to also normalize ambiguous opaques allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable. It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless. r? `@compiler-errors`
modify alias-relate to also normalize ambiguous opaques allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable. It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless. r? ``@compiler-errors``
@bors r- |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#120584 (For a rigid projection, recursively look at the self type's item bounds to fix the `associated_type_bounds` feature) - rust-lang#120589 (std::thread::available_parallelism merging linux/android/freebsd version) - rust-lang#120596 ([rustdoc] Correctly generate path for non-local items in source code pages) - rust-lang#120629 (Move some test files) - rust-lang#120846 (Update jobserver-rs to 0.1.28) - rust-lang#120850 (ast_lowering: Fix regression in `use ::{}` imports.) - rust-lang#120853 (Avoid a collection and iteration on empty passes) Failed merges: - rust-lang#120549 (modify alias-relate to also normalize ambiguous opaques) r? `@ghost` `@rustbot` modify labels: rollup
☔ The latest upstream changes (presumably #120852) made this pull request unmergeable. Please resolve the merge conflicts. |
with this, alias-relate treats all aliases the same way and it can be used for structural normalization.
@rustbot ready |
@bors r=compiler-errors |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#114877 (unstable-book: add quick-edit link) - rust-lang#120548 (rustdoc: Fix handling of doc_auto_cfg feature for cfg attributes on glob reexport) - rust-lang#120549 (modify alias-relate to also normalize ambiguous opaques) - rust-lang#120959 (Remove good path delayed bugs) - rust-lang#120978 (match lowering: simplify block creation) - rust-lang#121019 (coverage: Simplify some parts of the coverage span refiner) - rust-lang#121021 (Extend intra-doc link chapter in the rustdoc book) - rust-lang#121031 (RustWrapper: adapt for coverage mapping API changes) Failed merges: - rust-lang#121014 (Remove `force_print_diagnostic`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#120549 - lcnr:errs-showcase, r=compiler-errors modify alias-relate to also normalize ambiguous opaques allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable. It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless. r? `@compiler-errors`
…lcnr No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates` We already fully normalize the self type before we reach `assemble_alias_bound_candidates`, so there's no reason to double check that a projection is truly rigid by checking param-env bounds. I think this is also blocked on us making sure to always normalize opaques: rust-lang#120549. r? lcnr
…lcnr No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates` We already fully normalize the self type before we reach `assemble_alias_bound_candidates`, so there's no reason to double check that a projection is truly rigid by checking param-env bounds. I think this is also blocked on us making sure to always normalize opaques: rust-lang#120549. r? lcnr
Rollup merge of rust-lang#120598 - compiler-errors:no-rigid-check, r=lcnr No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates` We already fully normalize the self type before we reach `assemble_alias_bound_candidates`, so there's no reason to double check that a projection is truly rigid by checking param-env bounds. I think this is also blocked on us making sure to always normalize opaques: rust-lang#120549. r? lcnr
allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the
(Alias, Infer)
branches in alias-relate, so removing the opaque type special case here is really valuable.It does worsen
deduce_closure_signature
and friends even more as they now receive an inference variable which is only constrained via anAliasRelate
goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless.r? @compiler-errors