rustc: Make `CrateStore` private to `TyCtxt` #44420

alexcrichton · 2017-09-08T14:46:43Z

This commit makes the CrateStore object private to the ty/context.rs module and also absent on the Session itself.

cc #44390
cc #44341 (initial commit pulled and rebased from here)

rust-highfive · 2017-09-08T14:46:47Z

r? @arielb1

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

alexcrichton · 2017-09-08T14:47:09Z

r? @michaelwoerister

eddyb · 2017-09-09T08:32:12Z

src/librustc/ty/context.rs

+        assert_eq!(cnum, LOCAL_CRATE);
+        Rc::new(tcx.cstore.crates_untracked())
+    };
+    providers.postorder_cnums = |tcx, cnum| {


Oh, right, we can do this now! @nikomatsakis do you think we should move to this style more uniformly?

eddyb · 2017-09-09T08:33:39Z

src/librustc_metadata/cstore_impl.rs

@@ -60,7 +60,7 @@ macro_rules! provide {

                $tcx.dep_graph.read(dep_node);

-                let $cdata = $tcx.sess.cstore.crate_data_as_rc_any($def_id.krate);
+                let $cdata = $tcx.crate_data_as_rc_any($def_id.krate);


I was worried about this but I think it's fine - this will eventually be just a byte buffer that the query system interprets.

eddyb · 2017-09-09T08:34:39Z

My only concern here is that the CrateStore is threaded through more places - that's related to the TyCtxt not existing yet, isn't it?

michaelwoerister

Thanks, @alexcrichton!

r=me with that comment added and the remaining RustDoc build errors fixed.

michaelwoerister · 2017-09-09T15:53:47Z

src/librustc/ty/context.rs

+        self.cstore.metadata_encoding_version().to_vec()
+    }
+
+    pub fn crate_data_as_rc_any(self, cnum: CrateNum) -> Rc<Any> {


Ideally this would only be visible to the query system since it allows untracked access to crate store contents. Maybe you could add a comment "Do not use this unless you also provide proper dependency tracking" or something.

michaelwoerister · 2017-09-09T15:54:57Z

src/librustc/ty/context.rs

+    pub fn encode_metadata(self, link_meta: &LinkMeta, reachable: &NodeSet)
+        -> (EncodedMetadata, EncodedMetadataHashes)
+    {
+        self.cstore.encode_metadata(self, link_meta, reachable)


I'm not sure that metadata encoding even needs to be a method of CrateStore. It seems rather independent. But that's for another PR, I'd say.

michaelwoerister · 2017-09-09T16:04:54Z

My only concern here is that the CrateStore is threaded through more places - that's related to the TyCtxt not existing yet, isn't it?

At least for some of the cases, yes.

alexcrichton · 2017-09-09T18:06:23Z

@bors: r=michaelwoerister

bors · 2017-09-09T18:06:24Z

📌 Commit 8b9f84c has been approved by michaelwoerister

bors · 2017-09-10T10:15:40Z

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

alexcrichton · 2017-09-10T15:38:00Z

@bors: r=michaelwoerister

bors · 2017-09-10T15:38:02Z

📌 Commit 224d47d has been approved by michaelwoerister

frewsxcv · 2017-09-11T00:54:52Z

@bors rollup

…aelwoerister rustc: Make `CrateStore` private to `TyCtxt` This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself. cc rust-lang#44390 cc rust-lang#44341 (initial commit pulled and rebased from here)

frewsxcv · 2017-09-11T02:39:19Z

@bors r-

this breaks RLS https://ci.appveyor.com/project/rust-lang/rust/build/1.0.4628/job/3bud5pwi1gs21ry5

Update for rust-lang/rust#44420

alexcrichton · 2017-09-11T22:02:50Z

@bors: r=michaelwoerister

bors · 2017-09-11T22:02:51Z

📌 Commit e283efa has been approved by michaelwoerister

alexcrichton · 2017-09-12T01:53:08Z

@bors: rollup-

bors · 2017-09-12T06:53:47Z

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

…ulating CrateStore access in tcx.

alexcrichton · 2017-09-12T14:19:57Z

@bors: r=michaelwoerister

bors · 2017-09-12T14:19:58Z

📌 Commit a4b6a97 has been approved by michaelwoerister

bors · 2017-09-13T02:02:29Z

⌛ Testing commit a4b6a97 with merge dc93eb4...

rustc: Make `CrateStore` private to `TyCtxt` This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself. cc #44390 cc #44341 (initial commit pulled and rebased from here)

bors · 2017-09-13T03:04:45Z

💔 Test failed - status-appveyor

This commit removes the `cstore_untracked` method, making the `CrateStore` trait object entirely private to the `ty/context.rs` module.

alexcrichton · 2017-09-13T04:14:26Z

@bors: r=michaelwoerister

bors · 2017-09-13T04:14:28Z

📌 Commit 921750b has been approved by michaelwoerister

bors · 2017-09-13T05:32:06Z

⌛ Testing commit 921750b with merge c3eccc6...

rustc: Make `CrateStore` private to `TyCtxt` This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself. cc #44390 cc #44341 (initial commit pulled and rebased from here)

bors · 2017-09-13T08:23:41Z

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing c3eccc6 to master...

michaelwoerister · 2017-09-13T08:39:48Z

🎉

rust-highfive assigned arielb1 Sep 8, 2017

alexcrichton mentioned this pull request Sep 8, 2017

Remove the cstore reference from Session #44341

Closed

rust-highfive assigned michaelwoerister and unassigned arielb1 Sep 8, 2017

michaelwoerister mentioned this pull request Sep 8, 2017

incr.comp.: Do not hard-wire DepGraph into Session and CrateStore. #44390

Closed

7 tasks

eddyb reviewed Sep 9, 2017

View reviewed changes

michaelwoerister approved these changes Sep 9, 2017

View reviewed changes

alexcrichton force-pushed the private-cstore branch 2 times, most recently from 77b51c5 to 8b9f84c Compare September 9, 2017 18:06

alexcrichton force-pushed the private-cstore branch from 8b9f84c to 224d47d Compare September 10, 2017 15:37

frewsxcv mentioned this pull request Sep 11, 2017

Rollup of 8 pull requests #44482

Closed

frewsxcv mentioned this pull request Sep 11, 2017

Rollup of 8 pull requests #44483

Closed

alexcrichton force-pushed the private-cstore branch from 224d47d to 6e834ac Compare September 11, 2017 19:15

nrc added a commit to rust-dev-tools/rls-rustc that referenced this pull request Sep 11, 2017

Merge pull request #4 from alexcrichton/tmp

b4b88c2

Update for rust-lang/rust#44420

alexcrichton force-pushed the private-cstore branch from 6e834ac to e283efa Compare September 11, 2017 22:02

Remove the cstore reference from Session in order to prepare encaps…

54fa047

…ulating CrateStore access in tcx.

alexcrichton force-pushed the private-cstore branch from e283efa to a4b6a97 Compare September 12, 2017 14:19

rustc: Make CrateStore private to TyCtxt

921750b

This commit removes the `cstore_untracked` method, making the `CrateStore` trait object entirely private to the `ty/context.rs` module.

alexcrichton force-pushed the private-cstore branch from a4b6a97 to 921750b Compare September 13, 2017 04:14

bors mentioned this pull request Sep 13, 2017

Implement RFC 1861: Extern types #44295

Merged

bors merged commit 921750b into rust-lang:master Sep 13, 2017

gyscos mentioned this pull request Sep 14, 2017

Clippy 0.0.160 fails to build on 1.22.0-nightly (539f2083d 2017-09-13) rust-lang/rust-clippy#2050

Closed

alexcrichton deleted the private-cstore branch September 15, 2017 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: Make `CrateStore` private to `TyCtxt` #44420

rustc: Make `CrateStore` private to `TyCtxt` #44420

alexcrichton commented Sep 8, 2017

rust-highfive commented Sep 8, 2017

alexcrichton commented Sep 8, 2017

eddyb Sep 9, 2017 •

edited

Loading

eddyb Sep 9, 2017

eddyb commented Sep 9, 2017

michaelwoerister left a comment

michaelwoerister Sep 9, 2017

michaelwoerister Sep 9, 2017

michaelwoerister commented Sep 9, 2017

alexcrichton commented Sep 9, 2017

bors commented Sep 9, 2017

bors commented Sep 10, 2017

alexcrichton commented Sep 10, 2017

bors commented Sep 10, 2017

frewsxcv commented Sep 11, 2017

frewsxcv commented Sep 11, 2017

alexcrichton commented Sep 11, 2017

bors commented Sep 11, 2017

alexcrichton commented Sep 12, 2017

bors commented Sep 12, 2017

alexcrichton commented Sep 12, 2017

bors commented Sep 12, 2017

bors commented Sep 13, 2017

bors commented Sep 13, 2017

alexcrichton commented Sep 13, 2017

bors commented Sep 13, 2017

bors commented Sep 13, 2017

bors commented Sep 13, 2017

michaelwoerister commented Sep 13, 2017

rustc: Make CrateStore private to TyCtxt #44420

rustc: Make CrateStore private to TyCtxt #44420

Conversation

alexcrichton commented Sep 8, 2017

rust-highfive commented Sep 8, 2017

alexcrichton commented Sep 8, 2017

eddyb Sep 9, 2017 • edited Loading

Choose a reason for hiding this comment

eddyb Sep 9, 2017

Choose a reason for hiding this comment

eddyb commented Sep 9, 2017

michaelwoerister left a comment

Choose a reason for hiding this comment

michaelwoerister Sep 9, 2017

Choose a reason for hiding this comment

michaelwoerister Sep 9, 2017

Choose a reason for hiding this comment

michaelwoerister commented Sep 9, 2017

alexcrichton commented Sep 9, 2017

bors commented Sep 9, 2017

bors commented Sep 10, 2017

alexcrichton commented Sep 10, 2017

bors commented Sep 10, 2017

frewsxcv commented Sep 11, 2017

frewsxcv commented Sep 11, 2017

alexcrichton commented Sep 11, 2017

bors commented Sep 11, 2017

alexcrichton commented Sep 12, 2017

bors commented Sep 12, 2017

alexcrichton commented Sep 12, 2017

bors commented Sep 12, 2017

bors commented Sep 13, 2017

bors commented Sep 13, 2017

alexcrichton commented Sep 13, 2017

bors commented Sep 13, 2017

bors commented Sep 13, 2017

bors commented Sep 13, 2017

michaelwoerister commented Sep 13, 2017

rustc: Make `CrateStore` private to `TyCtxt` #44420

rustc: Make `CrateStore` private to `TyCtxt` #44420

eddyb Sep 9, 2017 •

edited

Loading