-
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
Add some extra information to opaque type cycle errors #113320
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Everything looks reasonable (although I may be missing some context). |
📌 Commit 3278316ac08c310eee3f472dd0b524c48fbcbc66 has been approved by It is now in the queue for this repository. |
☔ The latest upstream changes (presumably #113330) made this pull request unmergeable. Please resolve the merge conflicts. |
3278316
to
43c01ba
Compare
43c01ba
to
9e98feb
Compare
cc @rust-lang/initiative-trait-system-refactor opinions on the last commit? 9e98feb It adds a new |
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.
didn't look at actual diagnostic changes only the parts where the solver was changed. doesn't seem like it would have any bad interactions with new solver
@bors r=petrochenkov,BoxyUwU |
…etrochenkov,BoxyUwU Add some extra information to opaque type cycle errors Plus a bunch of cleanups. This should help users debug query cycles due to auto trait checking. We'll probably want to fix cycle errors in most (or all?) cases by looking at the current item's hidden types (new solver does this), and by delaying the auto trait checks to after typeck.
…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#113010 (rust-installer & rls: remove exclusion from rustfmt & tidy ) - rust-lang#113317 ( -Ztrait-solver=next: stop depending on old solver) - rust-lang#113319 (`TypeParameterDefinition` always require a `DefId`) - rust-lang#113320 (Add some extra information to opaque type cycle errors) - rust-lang#113321 (Move `ty::ConstKind` to `rustc_type_ir`) - rust-lang#113337 (Winnow specialized impls during selection in new solver) - rust-lang#113355 (Move most coverage code out of `rustc_codegen_ssa`) - rust-lang#113356 (Add support for NetBSD/riscv64 aka. riscv64gc-unknown-netbsd.) r? `@ghost` `@rustbot` modify labels: rollup
@rust-timer build a469b5fc80c0d4250147c756dc5c44516090467f |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (a469b5fc80c0d4250147c756dc5c44516090467f): comparison URL. Overall result: ❌ regressions - 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. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @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: 655.622s -> 654.779s (-0.13%) |
The grammar regression looks like inliner noise I think?
|
The regex regression has incr_comp_encode_dep_graph take more total time, even though there are fewer executions of it. I'm very confused 😆 |
More precisely detect cycle errors from type_of on opaque Not sure if this still needs work. Just putting it up for initial impressions, since it seems that a few people are frustrated with the increased error verbosity due to rust-lang#113320. Essentially we introduce a new sub-query for `type_of` specifically for opaques which returns a value that is able to distinguish "has errors" from "due to cycle recovery". Fixes rust-lang#115188 r? `@oli-obk`
Plus a bunch of cleanups.
This should help users debug query cycles due to auto trait checking. We'll probably want to fix cycle errors in most (or all?) cases by looking at the current item's hidden types (new solver does this), and by delaying the auto trait checks to after typeck.