-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Implement selection for Unsize
for better coercion behavior
#113353
Conversation
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
Unsize
for better coercion behavior
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.
r=me after nits
ty::TraitRef::new(tcx, goal.predicate.def_id(), [*a_last_ty, *b_last_ty]), | ||
)); | ||
} | ||
_ => return Ok(None), |
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.
please ICE here to detect mismatches
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.
We cannot ICE here because this goal is selected during coercion, so we legitimately may see, e.g. [T; N]: Unsize<_>
during selection.
One more point towards having a typeck and codegen mode for selection...
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.
Left a fixme anyways.
☔ The latest upstream changes (presumably #113308) made this pull request unmergeable. Please resolve the merge conflicts. |
772b9fe
to
df22cc2
Compare
df22cc2
to
a071044
Compare
You said r=me after nits, which I think I have addressed. @bors r=lcnr rollup (new solver) |
Implement selection for `Unsize` for better coercion behavior In order for much of coercion to succeed, we need to be able to deal with partial ambiguity of `Unsize` traits during selection. However, I pessimistically implemented selection in the new trait solver to just bail out with ambiguity if it was a built-in impl: https://github.com/rust-lang/rust/blob/9227ff28aff55b252314076fcf21c9a66f10ac1e/compiler/rustc_trait_selection/src/solve/eval_ctxt/select.rs#L126 This implements a proper "rematch" procedure for dealing with built-in `Unsize` goals, so that even if the goal is ambiguous, we are able to get nested obligations which are used in the coercion selection-like loop: https://github.com/rust-lang/rust/blob/9227ff28aff55b252314076fcf21c9a66f10ac1e/compiler/rustc_hir_typeck/src/coercion.rs#L702 Second commit just moves a `resolve_vars_if_possible` call to fix a bug where we weren't detecting a trait upcasting to occur. r? `@lcnr`
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#113353 (Implement selection for `Unsize` for better coercion behavior) - rust-lang#113553 (Make Placeholder, GeneratorWitness*, Infer and Error unreachable on SMIR rustc_ty_to_ty) - rust-lang#113598 (Update cargo) - rust-lang#113603 (Test simd-wide-sum for codegen error) - rust-lang#113613 (Allow to have `-` in rustdoc-json test file name) - rust-lang#113615 (llvm-wrapper: adapt for LLVM API change) - rust-lang#113616 (Fix bootstrap.py uname error) r? `@ghost` `@rustbot` modify labels: rollup
In order for much of coercion to succeed, we need to be able to deal with partial ambiguity of
Unsize
traits during selection. However, I pessimistically implemented selection in the new trait solver to just bail out with ambiguity if it was a built-in impl:rust/compiler/rustc_trait_selection/src/solve/eval_ctxt/select.rs
Line 126 in 9227ff2
This implements a proper "rematch" procedure for dealing with built-in
Unsize
goals, so that even if the goal is ambiguous, we are able to get nested obligations which are used in the coercion selection-like loop:rust/compiler/rustc_hir_typeck/src/coercion.rs
Line 702 in 9227ff2
Second commit just moves a
resolve_vars_if_possible
call to fix a bug where we weren't detecting a trait upcasting to occur.r? @lcnr