-
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
do not rerun trait solver cycles on ambiguity #125981
Conversation
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
As an example of other cases where this PR helps: it improves the new solver's current simili-hang on #125994 a lot. |
@rustbot ready looked at rust-lang/chalk#48 and to my understanding their reasoning was about an implementation detail of chalk. I still don't fully get why they got hangs when rerunning goals, but it seems unrelated as we do "apply the constraints" from ambiguous results and the added test works with the new solver (after changing it to |
☔ The latest upstream changes (presumably #126614) made this pull request unmergeable. Please resolve the merge conflicts. |
@rustbot author |
@jimblandy are you sure you're commenting on the correct PR? This PR is for the new solver, and shouldn't help building docs for wgpu-core. |
Yes, I think I am commenting on the right PR:
Honestly, I'm as surprised as you are, but here we are. |
To clear things up, #114891 is due to something going wrong in the old solver (it can't be the next solver because it's not yet enabled by default). First, I minimized the issue here: #114891 (comment). Then, I profiled rustdoc and realized it had something to do with the (old) trait solver. However, I'm not particularly well-versed in either trait solver much less in the gnarly details around cycles & caching which seems to be the root cause. So instead I decided to experimentally migrate the problematic rustdoc 'routine' (blanket impl synthesis) from the old to the next solver in #126196 to check if that might solve the problem. However, it turns out that the "rustc equivalent" of #114891 currently also hangs in the next solver (coincidentally I presume). For that, I opened #126196. This brings us to this PR which might unblock #126196 which in turn might fix #114891. Getting slightly off-topic but unfortunately I don't know when and why #114891 regressed (since it's hard to bisect a compiletime or hang issue). |
merged as part of #128828 |
See the comment in
fn reached_fixpoint
.Fixes a minimization of the hang in fuchsia (even if it doesn't fix fuchsia itself) and may impact other hangs as well 🤷 doesn't fix the regression in diesel by itself
r? @compiler-errors