Skip to content
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

Merged
merged 11 commits into from
Feb 17, 2020

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Jan 6, 2020

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

@cjgillot
Copy link
Contributor Author

cjgillot commented Jan 7, 2020

Once #67797 is merged, the resolve_instance query will be movable to a downstream crate. When that is done and #67780 is merged, this PR will be ready.

bors added a commit that referenced this pull request Jan 7, 2020
Minimize dependencies on trait and infer inside librustc

Split from #67953

All commits should pass check on their own.

r? @Centril
@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-07T21:28:13.0465822Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-07T21:28:13.0549482Z ##[command]git config gc.auto 0
2020-01-07T21:28:13.0627284Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-07T21:28:13.0686062Z ##[command]git config --get-all http.proxy
2020-01-07T21:28:13.0831017Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67953/merge:refs/remotes/pull/67953/merge
---
2020-01-07T21:37:48.4269745Z configure: build.locked-deps    := True
2020-01-07T21:37:48.4269792Z configure: llvm.ccache          := sccache
2020-01-07T21:37:48.4270006Z configure: build.cargo-native-static := True
2020-01-07T21:37:48.4270230Z configure: dist.missing-tools   := True
2020-01-07T21:37:48.4270496Z configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
2020-01-07T21:37:48.4270607Z configure: writing `config.toml` in current directory
2020-01-07T21:37:48.4270650Z configure: 
2020-01-07T21:37:48.4270867Z configure: run `python /checkout/x.py --help`
2020-01-07T21:37:48.4271129Z configure: 
---
2020-01-07T21:42:26.8802246Z 
2020-01-07T21:42:26.8868390Z error: could not compile `rustc_infer`.
2020-01-07T21:42:26.8868804Z warning: build failed, waiting for other jobs to finish...
2020-01-07T21:42:28.1497172Z error: build failed
2020-01-07T21:42:28.1593299Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "-Zconfig-profile" "--target" "i686-pc-windows-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--locked" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-01-07T21:42:28.1594301Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu
2020-01-07T21:42:28.1594392Z Build completed unsuccessfully in 0:03:08
2020-01-07T21:42:28.1632261Z == clock drift check ==
2020-01-07T21:42:28.1644394Z   local time: Tue Jan  7 21:42:28 UTC 2020
2020-01-07T21:42:28.1644394Z   local time: Tue Jan  7 21:42:28 UTC 2020
2020-01-07T21:42:28.3257950Z   network time: Tue, 07 Jan 2020 21:42:28 GMT
2020-01-07T21:42:28.3259041Z == end clock drift check ==
2020-01-07T21:42:29.0788166Z 
2020-01-07T21:42:29.0896282Z ##[error]Bash exited with code '1'.
2020-01-07T21:42:29.0930116Z ##[section]Starting: Checkout
2020-01-07T21:42:29.0932238Z ==============================================================================
2020-01-07T21:42:29.0932295Z Task         : Get sources
2020-01-07T21:42:29.0932362Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 @TimNN. (Feature Requests)

@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-08T08:32:03.4336844Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-08T08:32:03.4439409Z ##[command]git config gc.auto 0
2020-01-08T08:32:03.4501733Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-08T08:32:03.4559485Z ##[command]git config --get-all http.proxy
2020-01-08T08:32:03.4686107Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67953/merge:refs/remotes/pull/67953/merge
---
2020-01-08T08:40:56.8844388Z configure: build.locked-deps    := True
2020-01-08T08:40:56.8844452Z configure: llvm.ccache          := sccache
2020-01-08T08:40:56.8844650Z configure: build.cargo-native-static := True
2020-01-08T08:40:56.8844841Z configure: dist.missing-tools   := True
2020-01-08T08:40:56.8845092Z configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
2020-01-08T08:40:56.8845187Z configure: writing `config.toml` in current directory
2020-01-08T08:40:56.8845245Z configure: 
2020-01-08T08:40:56.8845468Z configure: run `python /checkout/x.py --help`
2020-01-08T08:40:56.8845513Z configure: 
---
2020-01-08T08:45:10.2078966Z 
2020-01-08T08:45:10.2134805Z error: could not compile `rustc_infer`.
2020-01-08T08:45:10.2135110Z warning: build failed, waiting for other jobs to finish...
2020-01-08T08:45:11.4125358Z error: build failed
2020-01-08T08:45:11.4148725Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "-Zconfig-profile" "--target" "i686-pc-windows-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--locked" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-01-08T08:45:11.4191284Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu
2020-01-08T08:45:11.4191431Z Build completed unsuccessfully in 0:02:47
2020-01-08T08:45:11.4242627Z == clock drift check ==
2020-01-08T08:45:11.4242707Z   local time: Wed Jan  8 08:45:11 UTC 2020
2020-01-08T08:45:11.4242707Z   local time: Wed Jan  8 08:45:11 UTC 2020
2020-01-08T08:45:11.5723667Z   network time: Wed, 08 Jan 2020 08:45:11 GMT
2020-01-08T08:45:11.5729432Z == end clock drift check ==
2020-01-08T08:45:12.2410426Z 
2020-01-08T08:45:12.2507829Z ##[error]Bash exited with code '1'.
2020-01-08T08:45:12.2677653Z ##[section]Starting: Checkout
2020-01-08T08:45:12.2680288Z ==============================================================================
2020-01-08T08:45:12.2680567Z Task         : Get sources
2020-01-08T08:45:12.2680613Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 @TimNN. (Feature Requests)

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-08T09:19:36.7144124Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-08T09:19:36.7229492Z ##[command]git config gc.auto 0
2020-01-08T09:19:36.7286044Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-08T09:19:36.7337708Z ##[command]git config --get-all http.proxy
2020-01-08T09:19:36.7478781Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67953/merge:refs/remotes/pull/67953/merge
---
2020-01-08T09:44:55.6117821Z Assembling stage1 compiler (x86_64-unknown-linux-gnu)
2020-01-08T09:44:55.6136473Z Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-08T09:44:55.9805317Z    Compiling cc v1.0.49
2020-01-08T09:44:55.9807278Z    Compiling core v0.0.0 (/checkout/src/libcore)
2020-01-08T09:45:00.8239503Z thread 'rustc' panicked at 'not implemented', src/librustc/traits/mod.rs:779:5
2020-01-08T09:45:00.8241044Z 
2020-01-08T09:45:00.8241602Z error: internal compiler error: unexpected panic
2020-01-08T09:45:00.8241692Z 
2020-01-08T09:45:00.8241807Z note: the compiler unexpectedly panicked. this is a bug.
2020-01-08T09:45:00.8241807Z note: the compiler unexpectedly panicked. this is a bug.
2020-01-08T09:45:00.8241888Z 
2020-01-08T09:45:00.8242569Z note: we would appreciate a bug report: ***/blob/master/CONTRIBUTING.md#bug-reports
2020-01-08T09:45:00.8242612Z 
2020-01-08T09:45:00.8242951Z note: rustc 1.42.0-nightly (80066bcb4 2020-01-08) running on x86_64-unknown-linux-gnu
2020-01-08T09:45:00.8242988Z 
2020-01-08T09:45:00.8243470Z note: compiler flags: -Z external-macro-backtrace -Z binary-dep-depinfo -Z force-unstable-if-unmarked -C opt-level=2 -C codegen-units=1 -C debuginfo=0 -C link-args=-Wl,-rpath,$ORIGIN/../lib -C prefer-dynamic -C llvm-args=-import-instr-limit=10 -C debug-assertions=n --crate-type lib
2020-01-08T09:45:00.8243590Z note: some of the compiler flags provided by cargo are hidden
2020-01-08T09:45:00.8243622Z 
2020-01-08T09:45:00.8678416Z error: could not compile `core`.
2020-01-08T09:45:00.8679194Z warning: build failed, waiting for other jobs to finish...
---
2020-01-08T09:45:01.8525770Z   local time: Wed Jan  8 09:45:01 UTC 2020
2020-01-08T09:45:02.0117512Z   network time: Wed, 08 Jan 2020 09:45:02 GMT
2020-01-08T09:45:02.0121866Z == end clock drift check ==
2020-01-08T09:45:03.2515208Z 
2020-01-08T09:45:03.2611755Z ##[error]Bash exited with code '1'.
2020-01-08T09:45:03.2643896Z ##[section]Starting: Checkout
2020-01-08T09:45:03.2645545Z ==============================================================================
2020-01-08T09:45:03.2645610Z Task         : Get sources
2020-01-08T09:45:03.2645648Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Jan 8, 2020

☔ The latest upstream changes (presumably #67770) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2020
src/librustc_infer/Cargo.toml Outdated Show resolved Hide resolved
@cjgillot
Copy link
Contributor Author

Pushed a cleaner and rebased version. It contains some other PRs as merge commits. Those ought to be rebased out.

@bors
Copy link
Contributor

bors commented Jan 15, 2020

☔ The latest upstream changes (presumably #68118) made this pull request unmergeable. Please resolve the merge conflicts.

@cjgillot cjgillot changed the title [WIP] Split librustc::{traits,infer} to a separate crate rustc_infer Split librustc::{traits,infer} to a separate crate rustc_infer Jan 16, 2020
@bors
Copy link
Contributor

bors commented Jan 16, 2020

☔ The latest upstream changes (presumably #68286) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jan 17, 2020

☔ The latest upstream changes (presumably #68305) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jan 21, 2020

☔ The latest upstream changes (presumably #68405) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jan 23, 2020

☔ The latest upstream changes (presumably #68474) made this pull request unmergeable. Please resolve the merge conflicts.

@Zoxc Zoxc added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 27, 2020
@Zoxc
Copy link
Contributor

Zoxc commented Jan 27, 2020

Nominating for @rust-lang/compiler. This PR mostly moves code around to enable splitting out rustc::traits and rustc::infer to a new crate rustc_infer. Some methods are converted to trait extensions methods, but I think quite a few of those could be made free standing functions. This would help quite a bit towards cutting down the size of librustc and it was done relatively cleanly. I was expecting it to be harder. Just checking in to see if anyone has some concerns about doing this.

@bors
Copy link
Contributor

bors commented Feb 17, 2020

☀️ Test successful - checks-azure
Approved by: Zoxc
Pushing a643ee8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 17, 2020
@bors bors merged commit e88500b into rust-lang:master Feb 17, 2020
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #67953!

Tested on commit a643ee8.
Direct link to PR: #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).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Feb 17, 2020
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).
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Feb 17, 2020
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 17, 2020
@cjgillot cjgillot deleted the split_infer branch February 17, 2020 07:31
@Zoxc
Copy link
Contributor

Zoxc commented Feb 17, 2020

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.

@eddyb

This comment has been minimized.

bors added a commit that referenced this pull request Feb 19, 2020
[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
@mati865
Copy link
Contributor

mati865 commented Feb 22, 2020

@Zoxc maybe it's worth to open issue so this regression isn't forgotten. Seems like low hanging fruit.

bors added a commit that referenced this pull request Mar 13, 2020
Split librustc::{traits,infer} to their respective crates

Followup on #67953.

I tried to follow the existing module structures.

cc @eddyb
r? @Zoxc
bors added a commit that referenced this pull request Mar 13, 2020
Split librustc::{traits,infer} to their respective crates

Followup on #67953.

I tried to follow the existing module structures.

cc @eddyb
r? @Zoxc
bors added a commit that referenced this pull request Mar 14, 2020
Split librustc::{traits,infer} to their respective crates

Followup on #67953.

I tried to follow the existing module structures.

cc @eddyb
r? @Zoxc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants