Opportunistically resolve regions in new solver #112443
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
compiler-errors
commented
Jun 9, 2023
| } | ||
| ty::GenericArgKind::Const(ct) => { | ||
| matches!(ct.kind(), ty::ConstKind::Bound(ty::INNERMOST, bc) if bc.as_usize() == bv) | ||
| let mut var = ty::BoundVar::from_u32(0); |
There was a problem hiding this comment.
This essentially just checks that the type and const vars are monotonically increasing, ignoring regions (which may have been resolved to a concrete region or a different region var).
tests/ui/dyn-star/param-env-infer.rs
Outdated
| // incremental | ||
|
|
||
| //[next] compile-flags: -Ztrait-solver=next |
There was a problem hiding this comment.
This ends up ICEing like:
https://gist.github.com/compiler-errors/2660fdd9902ec2686f6288b697953294
Not sure why it's overflowing now, though. I'll look into it tomorrow.
lcnr
reviewed
Jun 9, 2023
aliemjay
requested changes
Jun 12, 2023
There was a problem hiding this comment.
I think region resolution should be limited to the query response. Resolving regions in the input side is bad for caching especially because we no longer "uniquify" input regions. It would also allow for trait selection bugs to easily rely on two input regions being equal.
rust-cloud-vms
bot
force-pushed
the
next-solver-opportunistically-resolve-regions
branch
from
5d0b44b
to
01377e8
Compare
|
Only opportunistically resolving regions on response (instead of on input), per @aliemjay's suggestion, actually also fixes that ICE. And I gave Think this PR is ready now. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 16, 2023
…unistically-resolve-regions, r=lcnr Opportunistically resolve regions in new solver Use `opportunistic_resolve_var` during canonicalization to collapse some regions. We have to start using `CanonicalVarValues::is_identity_modulo_regions`. We also have to modify that function to consider responses like `['static, ^0, '^1, ^2]` to be an "identity" response, since because we opportunistically resolve regions, there's no longer a 1:1 mapping between canonical var values and bound var indices in the response... There's one nasty side-effect -- one test (`tests/ui/dyn-star/param-env-infer.rs`) starts to ICE because the certainty goes from `Yes` to `Maybe(Overflow)`... Not exactly sure why, though? Putting this up for discussion/investigation. r? `@lcnr`
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Jun 16, 2023
…unistically-resolve-regions, r=lcnr Opportunistically resolve regions in new solver Use `opportunistic_resolve_var` during canonicalization to collapse some regions. We have to start using `CanonicalVarValues::is_identity_modulo_regions`. We also have to modify that function to consider responses like `['static, ^0, '^1, ^2]` to be an "identity" response, since because we opportunistically resolve regions, there's no longer a 1:1 mapping between canonical var values and bound var indices in the response... There's one nasty side-effect -- one test (`tests/ui/dyn-star/param-env-infer.rs`) starts to ICE because the certainty goes from `Yes` to `Maybe(Overflow)`... Not exactly sure why, though? Putting this up for discussion/investigation. r? ``@lcnr``
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 16, 2023
Rollup of 7 pull requests Successful merges: - rust-lang#112163 (fix: inline `predicate_may_hold_fatal` and remove expect call in it) - rust-lang#112399 (Instantiate closure synthetic substs in root universe) - rust-lang#112443 (Opportunistically resolve regions in new solver) - rust-lang#112535 (reorder attributes to make miri-test-libstd work again) - rust-lang#112579 (Fix building libstd documentation on FreeBSD.) - rust-lang#112639 (Fix `dead_code_cgu` computation) - rust-lang#112642 (Handle interpolated literal errors) r? `@ghost` `@rustbot` modify labels: rollup
compiler-errors
deleted the
next-solver-opportunistically-resolve-regions
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use
opportunistic_resolve_varduring canonicalization to collapse some regions.We have to start using
CanonicalVarValues::is_identity_modulo_regions. We also have to modify that function to consider responses like['static, ^0, '^1, ^2]to be an "identity" response, since because we opportunistically resolve regions, there's no longer a 1:1 mapping between canonical var values and bound var indices in the response...There's one nasty side-effect -- one test (
tests/ui/dyn-star/param-env-infer.rs) starts to ICE because the certainty goes fromYestoMaybe(Overflow)... Not exactly sure why, though? Putting this up for discussion/investigation.r? @lcnr