-
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
Validate AggregateKind types in MIR #120137
Conversation
r? @davidtwco (rustbot has picked a reviewer for you, use r? to override) |
If perf is bad, I'll put this behind a debug cfg I guess @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
… r=<try> Validate AggregateKind types in MIR Would have helped me catch some bugs when writing shims for async closures
This comment was marked as duplicate.
This comment was marked as duplicate.
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (3dba895): 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 a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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: 664.714s -> 664.328s (-0.06%) |
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.
More checking is good. r=me, I'll let you decide how to address the suggestions.
@bors delegate=compiler-errors rollup
AggregateKind::Tuple => {} | ||
AggregateKind::Array(dest) => { | ||
for src in fields { | ||
if !self.mir_assign_valid_types(src.ty(self.body, self.tcx), dest) { |
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.
Note: this is a weird name for a method returning bool
.
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.
(But pre-existing, so no need to change anything in this PR.)
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 will not change it in that case 😅
} | ||
} | ||
} | ||
AggregateKind::Coroutine(_, args) => { |
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.
Seems like the Closure
and Coroutine
branches could be merged, the types are the same. Only the error message differs, but it could be changed to "closure/coroutine field has the wrong type".
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.
No, they differ with how they handle their args. See the as_coroutine
and as_closure
.
@bors delegate=compiler-errors rollup |
✌️ @compiler-errors, you can now approve this pull request! If @nnethercote told you to " |
I don't need delegation since I already have bors rights 😉 @bors r=nnethercote |
…s, r=nnethercote Validate AggregateKind types in MIR Would have helped me catch some bugs when writing shims for async closures
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#117910 (Refactor uses of `objc_msgSend` to no longer have clashing definitions) - rust-lang#118639 (Undeprecate lint `unstable_features` and make use of it in the compiler) - rust-lang#119801 (Fix deallocation with wrong allocator in (A)Rc::from_box_in) - rust-lang#120058 (bootstrap: improvements for compiler builds) - rust-lang#120059 (Make generic const type mismatches not hide trait impls from the trait solver) - rust-lang#120097 (Report unreachable subpatterns consistently) - rust-lang#120137 (Validate AggregateKind types in MIR) - rust-lang#120164 (`maybe_lint_impl_trait`: separate `is_downgradable` from `is_object_safe`) - rust-lang#120181 (Allow any `const` expression blocks in `thread_local!`) - rust-lang#120204 (Builtin macros effectively have implicit #[collapse_debuginfo(yes)]) - rust-lang#120218 (rustfmt: Check that a token can begin a nonterminal kind before parsing it as a macro arg) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#117910 (Refactor uses of `objc_msgSend` to no longer have clashing definitions) - rust-lang#118639 (Undeprecate lint `unstable_features` and make use of it in the compiler) - rust-lang#119801 (Fix deallocation with wrong allocator in (A)Rc::from_box_in) - rust-lang#120058 (bootstrap: improvements for compiler builds) - rust-lang#120059 (Make generic const type mismatches not hide trait impls from the trait solver) - rust-lang#120097 (Report unreachable subpatterns consistently) - rust-lang#120137 (Validate AggregateKind types in MIR) - rust-lang#120164 (`maybe_lint_impl_trait`: separate `is_downgradable` from `is_object_safe`) - rust-lang#120181 (Allow any `const` expression blocks in `thread_local!`) - rust-lang#120218 (rustfmt: Check that a token can begin a nonterminal kind before parsing it as a macro arg) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#120137 - compiler-errors:validate-aggregates, r=nnethercote Validate AggregateKind types in MIR Would have helped me catch some bugs when writing shims for async closures
Would have helped me catch some bugs when writing shims for async closures