-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Folding/ReErased cleanups
#150861
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
Folding/ReErased cleanups
#150861
Conversation
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred to the CTFE machinery rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer |
This comment has been minimized.
This comment has been minimized.
| &self, | ||
| _dst: Ty<'db>, | ||
| _src: Ty<'db>, | ||
| _dst: Ty<'db>, |
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.
Nb.: we're currently using an older version of rustc_next_trait_solver (from crates.io), but this is fine because it's a no-op.
| assert!(!outlives_predicate.has_escaping_bound_vars()); | ||
| let erased_outlives_predicate = tcx.erase_and_anonymize_regions(outlives_predicate); | ||
| let outlives_ty = erased_outlives_predicate.skip_binder().0; | ||
| let outlives_ty = tcx.erase_and_anonymize_regions(outlives_predicate.skip_binder().0); |
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.
that impacts behavior. for<'a> Ty<'a, 'param> currently gets changed to Ty<'bound0, 'erased> but now stays as Ty<'a, 'erased> where bound0 and 'a are escaping bound region.
This doesn't matter as we need to special case the bound regions of the outlives predicate anyways and use a hashmap for them in MatchAgainstHigherRankedOutlives.
hmm. I am not too happy about this. I like having all regions in there anonymized, even if it doesn't matter. If we anonymize, we could use a SmallVec instead of a hashmap here 🤷 idk if this is in any way perf relevant, i'd guess not.
Please either revert this commit, or add a comment that we don't anonymize the escaping bound regions, but that this is fine as MatchAgainstHigherRankedOutlives handles them specially anyways
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.
I'll just remove the change. I wasn't intending to change behaviour.
| let ty = tcx.type_of(variant.tail().did).instantiate_identity(); | ||
| let ty = tcx.erase_and_anonymize_regions(ty); | ||
| assert!(!ty.has_infer()); | ||
| assert!(!ty.has_type_flags(TypeFlags::HAS_TY_INFER | TypeFlags::HAS_CT_INFER)); |
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.
!ty.has_non_region_infer()? though 🤷 that assert is unnecessary, ty is the output of a query. It never has infer vars
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.
Ok, I'll remove the assertion
| ) -> Result<Certainty, NoSolution> { | ||
| // Erase regions because we compute layouts in `rustc_transmute`, | ||
| // which will ICE for region vars. | ||
| let (dst, src) = self.tcx.erase_and_anonymize_regions((dst, src)); |
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.
it's not about looking at regions, we do call layout_of for types here though... surprised that removing this doesn't cause test failures.,
Do you encounter region vars here? if so, why does fn is_transmutable not ICE when calling Tree::from_ty
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.
I'll just remove this commit.
|
some thoughts, otherwise r=me |
This comment has been minimized.
This comment has been minimized.
b72108d to
c3ceba7
Compare
This comment has been minimized.
This comment has been minimized.
Because these folders only change regions. Note: `BottomUpFolder` folds all regions, while `fold_regions` skips some bound regions. But that's ok because these two folders only modify `ReVar`s.
The exact same call appears earlier in this function.
The only thing we do with the result is consult the `.def_id` field, which is unaffected by `erase_and_anonymize_regions`.
- Remove the vacuous `Types`, which provides extremely little value. - Make sure `src` comes before `dst` in all transmute-related functions. (Currently it's a mix: sometimes `src` is first, sometimes it is second`.)
It duplicates `has_erased_regions` from the compiler.
c3ceba7 to
e7036b1
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
I removed the commits mentioned in the reviews, and a couple more involving @bors r=lcnr |
Folding/`ReErased` cleanups Various cleanups I found while reading this code closely, mostly involving folding and the use of `ReErased`. r? @lcnr
Rollup of 5 pull requests Successful merges: - #150861 (Folding/`ReErased` cleanups) - #150941 (rustc_parse_format: improve diagnostics for unsupported python numeric grouping) - #150972 (Rename EII attributes slightly (being consistent in naming things foreign items, not extern items)) - #150980 (Use updated indexes to build reverse map for delegation generics) - #150986 (std: Fix size returned by UEFI tcp4 read operations) r? @ghost
Rollup of 8 pull requests Successful merges: - #150861 (Folding/`ReErased` cleanups) - #150869 (Emit error instead of delayed bug when meeting mismatch type for const tuple) - #150920 (Use a hook to decouple `rustc_mir_transform` from `rustc_mir_build`) - #150941 (rustc_parse_format: improve diagnostics for unsupported python numeric grouping) - #150972 (Rename EII attributes slightly (being consistent in naming things foreign items, not extern items)) - #150980 (Use updated indexes to build reverse map for delegation generics) - #150986 (std: Fix size returned by UEFI tcp4 read operations) - #150996 (Remove `S-waiting-on-bors` after a PR is merged) r? @ghost
Rollup of 8 pull requests Successful merges: - #150861 (Folding/`ReErased` cleanups) - #150869 (Emit error instead of delayed bug when meeting mismatch type for const tuple) - #150920 (Use a hook to decouple `rustc_mir_transform` from `rustc_mir_build`) - #150941 (rustc_parse_format: improve diagnostics for unsupported python numeric grouping) - #150972 (Rename EII attributes slightly (being consistent in naming things foreign items, not extern items)) - #150980 (Use updated indexes to build reverse map for delegation generics) - #150986 (std: Fix size returned by UEFI tcp4 read operations) - #150996 (Remove `S-waiting-on-bors` after a PR is merged) r? @ghost
Rollup of 8 pull requests Successful merges: - rust-lang/rust#150861 (Folding/`ReErased` cleanups) - rust-lang/rust#150869 (Emit error instead of delayed bug when meeting mismatch type for const tuple) - rust-lang/rust#150920 (Use a hook to decouple `rustc_mir_transform` from `rustc_mir_build`) - rust-lang/rust#150941 (rustc_parse_format: improve diagnostics for unsupported python numeric grouping) - rust-lang/rust#150972 (Rename EII attributes slightly (being consistent in naming things foreign items, not extern items)) - rust-lang/rust#150980 (Use updated indexes to build reverse map for delegation generics) - rust-lang/rust#150986 (std: Fix size returned by UEFI tcp4 read operations) - rust-lang/rust#150996 (Remove `S-waiting-on-bors` after a PR is merged) r? @ghost
Various cleanups I found while reading this code closely, mostly involving folding and the use of
ReErased.r? @lcnr