Move the dep_graph construction to a dedicated crate. #67761

cjgillot · 2019-12-31T16:46:19Z

The interface for librustc consists in two traits: DepKind and DepContext.

The DepKind is the main interface. It allows to probe properties of the dependency.
As before, DepNode is the pair of a DepKind object and a hash fingerprint.

The DepContext takes the place of the TyCtxt, and handles communication with the query engine.

The use of the ImplicitCtxt through ty::tls is done through the DepKind trait.
This may not be the best choice, but it seemed like the simplest.

rust-highfive · 2019-12-31T16:46:23Z

r? @zackmdavis

(rust_highfive has picked a reviewer for you, use r? to override)

cjgillot · 2019-12-31T17:00:04Z

cc #65031

src/librustc_dep_graph/lib.rs

Centril · 2019-12-31T17:40:54Z

r? @Zoxc

Zoxc · 2019-12-31T21:32:51Z

I think this PR will need some discussion, cc @michaelwoerister

michaelwoerister · 2020-01-02T09:08:42Z

Thanks for flagging that, @Zoxc. I think this PR is indeed a bit too invasive to just merge it. Is there any plan agreed upon by the compiler team for splitting things into more crates? I'm all for moving things out of librustc but it would be good to discuss the high-level picture/crate structure before jumping into bigger refactorings.

Centril · 2020-01-02T09:22:22Z

Is there any plan agreed upon by the compiler team for splitting things into more crates? I'm all for moving things out of librustc but it would be good to discuss the high-level picture/crate structure before jumping into bigger refactorings.

I think there's a general desire to split things and we've had a design meeting where we loosely agreed to splitting crates based on data vs. logic crates (see e.g. #65324). The execution of that will vary depending on how deep the implicit dependencies go and how much of a mess there is to untangle. Therefore it will be hard to come up with any grand scheme, and so we should experiment and see what logical units pop out and land those.

From what I can tell of the specifics in this PR, it will allow me to further move parts of the HIR (the Map & friends) out of librustc beyond what #67803 achieves.

michaelwoerister · 2020-01-02T09:25:20Z

Therefore it will be hard to come up with any grand scheme, and so we should experiment and see what logical units pop out and land those.

It might not be easy but I think it is still worthwhile to define a clean goal to work towards. Otherwise there's the risking of ending up with even more accidental complexity.

Centril · 2020-01-02T09:31:03Z

It might not be easy but I think it is still worthwhile to define a clean goal to work towards.

I don't disagree, but that clean goal will necessarily be vague and very high level (e.g. a list of crate names and a vague notion of dependencies). Only by actually trying it out experimentally will we find out if it is possible or not.

Otherwise there's the risking of ending up with even more accidental complexity.

I actually doubt this. The longer librustc exists the more it invites chaos (I'm looking at you rustc::mir & rustc_mir) and adding more implicit dependencies over time. By splitting things we'll architecturally prevent "what's another hack". Merging crates is also easier than splitting.

petrochenkov · 2020-01-02T09:39:39Z

I'm wary of introducing more bureaucracy because things may end up similarly to the start-to-end queries - years pass and very little happens except for some meetings once in few months.

The crate-ification with the general, even if vague, "separate data vs logic" idea in mind actively improves build times and parallelism now, making any other work easier.
I'm happy with how it worked out for the libsyntax split so far, and wouldn't mind if people actively working on other parts of the compiler performed this crate-ification independently in accordance with their expertise and domain knowledge, without a centralized compiler team approval.

michaelwoerister · 2020-01-02T09:57:27Z

End-to-end queries has to solve lots of hard architectural problems. You call it "bureaucracy", I call it a safe guard that has prevented us from making unnecessary mistakes :)

I think there should be a high-level tracking, stating something like @Centril mentioned on discord:

"clean goal" is I think to have at least one data crate for each IR and then a series of in-out crates for e.g. AST -> HIR, HIR -> HAIR, ...

I think that (vague) goal can be considered as approved by the compiler team. I also it should be made a bit more concrete, with at least the list of "header" crates we are shooting for.

I do agree that large parts of this are domain specific and "local" questions should not have to wait for compiler team approval. I would just like to have a little more transparency of what's even going on. This is something the team is discussed in the last design meeting: https://rust-lang.github.io/compiler-team/minutes/design-meeting/2019-12-20-major-change-process/

michaelwoerister · 2020-01-03T12:49:45Z

I should be able to take a closer look at this next week. @Zoxc, feel free to assign this to me.

@Zoxc

Extract `rustc_hir` out of `rustc` The new crate contains: ```rust pub mod def; pub mod def_id; mod hir; pub mod hir_id; pub mod itemlikevisit; pub mod pat_util; pub mod print; mod stable_hash_impls; pub use hir::*; pub use hir_id::*; pub use stable_hash_impls::HashStableContext; ``` Remains to be done in follow-up PRs: - Move `rustc::hir::map` into `rustc_hir_map` -- this has to be a separate crate due to the `dep_graph` (blocked on #67761). - Move references to `rustc::hir` to `rustc_hir` where possible. cc #65031 r? @Zoxc

bors · 2020-01-05T01:20:59Z

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

bors · 2020-01-09T05:18:29Z

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

michaelwoerister · 2020-01-10T09:53:04Z

From my point of view we can go forward with this PR as it is. I have a gut feeling that things could be factored slightly better so that not everything goes through DepKind but that can be looked into at some other time and should not cause major changes.

I'll leave it to @Zoxc to give the final r+.

cjgillot · 2020-01-10T12:26:56Z

From my point of view we can go forward with this PR as it is. I have a gut feeling that things could be factored slightly better so that not everything goes through DepKind but that can be looked into at some other time and should not cause major changes.

I plan to do it in a follow-up PR.

bors · 2020-01-10T15:48:59Z

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

cjgillot · 2020-01-10T16:23:09Z

Rebased.

bors · 2020-01-10T23:36:22Z

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

cjgillot · 2020-01-16T20:36:58Z

I have implemented a cleverer interface in commit 9aa0a9a2bbaad8f7fc27648a126294d4e668c252. I can add it to this PR if you think it good enough.

bors · 2020-03-23T22:02:59Z

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

This is needed since `middle::cstore` defines another type named `DepKind`, and we should not rely on shadowing to get the right one.

cjgillot · 2020-03-23T22:20:29Z

Rebased.

Zoxc · 2020-03-23T23:18:54Z

@bors r+

bors · 2020-03-23T23:18:55Z

📌 Commit 0f918cb has been approved by Zoxc

@ghost

Rollup of 11 pull requests Successful merges: - rust-lang#67761 (Move the dep_graph construction to a dedicated crate.) - rust-lang#69740 (Replace some desc logic in librustc_lint with article_and_desc) - rust-lang#69981 (Evaluate repeat expression lengths as late as possible) - rust-lang#70087 (Remove const eval loop detector) - rust-lang#70242 (Improve E0308 error message wording) - rust-lang#70264 (Fix invalid suggestion on `&mut` iterators yielding `&` references) - rust-lang#70267 (get rid of ConstPropUnsupported; use ZST marker structs instead) - rust-lang#70277 (Remove `ReClosureBound`) - rust-lang#70283 (Add regression test for rust-lang#70155.) - rust-lang#70294 (Account for bad placeholder types in where clauses) - rust-lang#70309 (Clean up E0452 explanation) Failed merges: r? @ghost

bors · 2020-03-24T04:07:48Z

⌛ Testing commit 0f918cb with merge 342c5f3...

bors · 2020-03-24T04:08:41Z

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

Zoxc · 2020-03-24T04:27:05Z

src/librustc_query_system/dep_graph/graph.rs

    ) -> (R, DepNodeIndex)
    where
-        C: DepGraphSafe + StableHashingContextProvider<'a>,
+        C: DepGraphSafe + HashStableContextProvider<H>,


You can replace DepGraphSafe + HashStableContextProvider<H> with DepContext here and in with_task_impl and with_eval_always_task. Then you could remove both DepGraphSafe and HashStableContextProvider.

@Zoxc

Move the query system to a dedicated crate The query system `rustc::ty::query` is split out into the `rustc_query_system` crate. Some commits are unformatted, to ease rebasing. Based on rust-lang#67761 and rust-lang#69910. r? @Zoxc

rust-highfive assigned zackmdavis Dec 31, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2019

Centril reviewed Dec 31, 2019

View reviewed changes

src/librustc_dep_graph/lib.rs Outdated Show resolved Hide resolved

src/librustc_dep_graph/lib.rs Outdated Show resolved Hide resolved

src/librustc_dep_graph/lib.rs Outdated Show resolved Hide resolved

rust-highfive assigned Zoxc and unassigned zackmdavis Dec 31, 2019

Centril mentioned this pull request Jan 2, 2020

Extract rustc_hir out of rustc #67803

Merged

cjgillot force-pushed the split_graph branch from d726099 to 3d2b129 Compare January 8, 2020 18:24

cjgillot force-pushed the split_graph branch from 3d2b129 to 2143dae Compare January 10, 2020 12:25

cjgillot force-pushed the split_graph branch from 2143dae to 31440e3 Compare January 10, 2020 16:22

cjgillot force-pushed the split_graph branch from 31440e3 to 36d5304 Compare January 11, 2020 18:01

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 23, 2020

cjgillot added 9 commits March 23, 2020 23:07

Move dep_graph to new crate librustc_query_system.

a7e2641

Make librustc_query_system compile.

6624dc4

Fallout in other crates.

db7bd5f

Merge ensure_node_can_be_forced into force_from_dep_node.

2326ae3

Remove assert_ignored and with_ignore_deps.

3a8bb20

Put extract_def_id back on DepNode.

d08cc0b

Address review.

e0f7b89

Fully qualify the path to DepKind.

9f4c888

This is needed since `middle::cstore` defines another type named `DepKind`, and we should not rely on shadowing to get the right one.

Move import.

0f918cb

cjgillot force-pushed the split_graph branch from 49ffed9 to 0f918cb Compare March 23, 2020 22:20

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 23, 2020

Centril removed the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Mar 23, 2020

Centril mentioned this pull request Mar 23, 2020

Rollup of 11 pull requests #70343

Merged

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 24, 2020

bors merged commit 9da25d9 into rust-lang:master Mar 24, 2020

Zoxc reviewed Mar 24, 2020

View reviewed changes

cjgillot deleted the split_graph branch March 24, 2020 06:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move the dep_graph construction to a dedicated crate. #67761

Move the dep_graph construction to a dedicated crate. #67761

cjgillot commented Dec 31, 2019

rust-highfive commented Dec 31, 2019

cjgillot commented Dec 31, 2019

Centril commented Dec 31, 2019

Zoxc commented Dec 31, 2019

michaelwoerister commented Jan 2, 2020

Centril commented Jan 2, 2020

michaelwoerister commented Jan 2, 2020

Centril commented Jan 2, 2020 •

edited

Loading

petrochenkov commented Jan 2, 2020 •

edited

Loading

michaelwoerister commented Jan 2, 2020

michaelwoerister commented Jan 3, 2020

bors commented Jan 5, 2020

bors commented Jan 9, 2020

michaelwoerister commented Jan 10, 2020 •

edited

Loading

cjgillot commented Jan 10, 2020

bors commented Jan 10, 2020

cjgillot commented Jan 10, 2020

bors commented Jan 10, 2020

cjgillot commented Jan 16, 2020

bors commented Mar 23, 2020

cjgillot commented Mar 23, 2020

Zoxc commented Mar 23, 2020

bors commented Mar 23, 2020

bors commented Mar 24, 2020

bors commented Mar 24, 2020

Zoxc Mar 24, 2020

Move the dep_graph construction to a dedicated crate. #67761

Move the dep_graph construction to a dedicated crate. #67761

Conversation

cjgillot commented Dec 31, 2019

rust-highfive commented Dec 31, 2019

cjgillot commented Dec 31, 2019

Centril commented Dec 31, 2019

Zoxc commented Dec 31, 2019

michaelwoerister commented Jan 2, 2020

Centril commented Jan 2, 2020

michaelwoerister commented Jan 2, 2020

Centril commented Jan 2, 2020 • edited Loading

petrochenkov commented Jan 2, 2020 • edited Loading

michaelwoerister commented Jan 2, 2020

michaelwoerister commented Jan 3, 2020

bors commented Jan 5, 2020

bors commented Jan 9, 2020

michaelwoerister commented Jan 10, 2020 • edited Loading

cjgillot commented Jan 10, 2020

bors commented Jan 10, 2020

cjgillot commented Jan 10, 2020

bors commented Jan 10, 2020

cjgillot commented Jan 16, 2020

bors commented Mar 23, 2020

cjgillot commented Mar 23, 2020

Zoxc commented Mar 23, 2020

bors commented Mar 23, 2020

bors commented Mar 24, 2020

bors commented Mar 24, 2020

Zoxc Mar 24, 2020

Choose a reason for hiding this comment

Centril commented Jan 2, 2020 •

edited

Loading

petrochenkov commented Jan 2, 2020 •

edited

Loading

michaelwoerister commented Jan 10, 2020 •

edited

Loading