-
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
Some minor dyn-related tweaks #133393
Some minor dyn-related tweaks #133393
Conversation
HIR ty lowering was modified cc @fmease changes to the core type system |
Some(predicate.with_self_ty(tcx, tcx.types.trait_object_dummy_self)) | ||
} else { | ||
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.
this relies on auto-traits not having any region bounds 🤔 I don't like the fact that we implicitly rely on that here, given that we may change it in the future, even if I can't think of any auto trait which does so.
Why don't we just pass in all ExistentialPredicates
here without any filtering? perf? the elaborator should filter duplicate predicates
nit, r=me on the other 2 commits |
565166d
to
bcfc8ab
Compare
Ok, I removed the filtering and squashed the two methods together. I also don't expect the filtering to affect perf. |
i guess unless this is blocking something @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Some minor dyn-related tweaks Each commit should be self-explanatory, but I'm happy to explain what's going on if not. These are tweaks I pulled out of rust-lang#133388, but they can be reviewed sooner than that. r? types
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (dee4e86): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 4.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -2.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 796.592s -> 798.676s (0.26%) |
@bors r=lcnr,spastorino |
☀️ Test successful - checks-actions |
Finished benchmarking commit (c322cd5): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (primary 1.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 793.948s -> 795.759s (0.23%) |
@@ -727,7 +727,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |||
let tcx = self.tcx(); | |||
// FIXME: Marked `mut` so that we can replace the spans further below with a more | |||
// appropriate one, but this should be handled earlier in the span assignment. | |||
let mut associated_types: FxIndexMap<Span, Vec<_>> = associated_types | |||
let associated_types: FxIndexMap<Span, Vec<_>> = associated_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.
Hasn't a FIXME
comment above became outdated after this?
Each commit should be self-explanatory, but I'm happy to explain what's going on if not. These are tweaks I pulled out of #133388, but they can be reviewed sooner than that.
r? types