-
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
Split librustc::{traits,infer} to a separate crate rustc_infer #67953
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☔ The latest upstream changes (presumably #67770) made this pull request unmergeable. Please resolve the merge conflicts. |
Pushed a cleaner and rebased version. It contains some other PRs as merge commits. Those ought to be rebased out. |
☔ The latest upstream changes (presumably #68118) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #68286) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #68305) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #68405) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #68474) made this pull request unmergeable. Please resolve the merge conflicts. |
Nominating for @rust-lang/compiler. This PR mostly moves code around to enable splitting out |
☀️ Test successful - checks-azure |
📣 Toolstate changed by #67953! Tested on commit a643ee8. 💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra). |
Tested on commit rust-lang/rust@a643ee8. Direct link to PR: <rust-lang/rust#67953> 💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
Rustup to rust-lang/rust#67953 changelog: none
This seems to have regressed performance a bit: https://perf.rust-lang.org/compare.html?start=5e7af4669f80e5f682141f050193ab679afdb4b1&end=a643ee8d693b8100e6f54f2a01ff7cde05eb65c5&stat=instructions:u My guess is that something hot doesn't get inlined in type checking anymore. |
This comment has been minimized.
This comment has been minimized.
[WIP] traits/select: use global vs per-infcx caches more uniformly. ~~*Note: this is based on an older `master` to avoid perf interference before #67953 (comment) is resolved.*~~ **EDIT**: sadly not workable due #69294 (comment). So far this PR only has the first couple steps towards making the decision of which cache ("global" vs "per-`InferCtxt`") to use for trait evaluation/selection, only once (at the time of checking the cache). The goal here is to actually make per-`InferCtxt` caches not track `DepNode`s, and maybe even enforce that once `SelectionContext::in_task` is entered, the `InferCtxt` is effectively unused. My assumption is that if you *need* inference variables in your cache key (i.e. `ParamEnv` and/or `PolyTraitPredicate`) to ever end up doing anything "non-global", and you can't get there from a "global cache key" (which would still use `DepNode`s and `in_task` etc.). Perhaps another path forward is to redirect "global cache keys" to a query, but I'm not sure how that would handle cycles (badly?), and it feels like stepping on Chalk's toes. r? @nikomatsakis cc @rust-lang/wg-traits @Zoxc @michaelwoerister
@Zoxc maybe it's worth to open issue so this regression isn't forgotten. Seems like low hanging fruit. |
This is still very much work in progress.
Three functions are between dimensions (at the end of
rustc::traits
), waiting for some dependency breaking scheme.Please tell me if the approach seems sound, and how you would like to split this PR up.
The formatting is deliberately off, to ease rebasing.
cc #65031