-
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
Implement Lift using interners instead of in_arena #67791
Conversation
@bors try @rust-timer queue |
Awaiting bors try build completion |
@@ -137,6 +137,20 @@ impl<K: Eq + Hash + Copy> ShardedHashMap<K, ()> { | |||
} | |||
} | |||
|
|||
pub unsafe trait IntoPointer { |
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.
Documenting the proof obligations would be good.
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 make it safe instead =P
☀️ Try build successful - checks-azure |
Queued 5071cca with parent 0ec3706, future comparison URL. |
Finished benchmarking try commit 5071cca, comparison URL. |
☔ The latest upstream changes (presumably #68115) made this pull request unmergeable. Please resolve the merge conflicts. |
@@ -123,6 +123,9 @@ macro_rules! arena_types { | |||
[few] inferred_outlives_crate: rustc::ty::CratePredicatesMap<'tcx>, | |||
[] upvars: rustc_data_structures::fx::FxIndexMap<rustc_hir::HirId, rustc_hir::Upvar>, | |||
|
|||
// Interned types | |||
[] tys: rustc::ty::TyS<$tcx>, |
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.
Huh, so we're not using the DroplessArena
for these anymore?
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 do, but TyS
is not Copy
, so we have to declare it.
assoc_name: ast::Ident, | ||
span: Span, | ||
is_equality: Option<String>, | ||
is_equality: impl Fn() -> Option<String>, |
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.
Am I missing something, or are the changes in this file unrelated?
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 stumbled into this since I broke Lift
while working on this PR. There's more cases still where we print types without using them.
r=me with #67791 (comment) explained or resolved |
@bors r+ |
📌 Commit f4968c8 has been approved by |
Rollup of 4 pull requests Successful merges: - #66564 (Document unsafe blocks in core::{cell, str, sync}) - #67791 (Implement Lift using interners instead of in_arena) - #68278 ([self-profiler] Add example to `-Z help` to turn on query key recording) - #68300 (Allow added string.insert benchmarks to compile) Failed merges: r? @ghost
r? @eddyb
cc @cjgillot