Skip to content

Remove the cstore reference from Session #44341

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

Conversation

michaelwoerister
Copy link
Member

This PR removes the CrateStore reference from Session in preparation to control all CrateStore access through the tcx. This control will later be used to either interpose a CrateStore wrapper for dependency tracking or forcing all CrateStore access to go through queries.

@rust-highfive
Copy link
Contributor

r? @arielb1

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

@michaelwoerister
Copy link
Member Author

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned arielb1 Sep 5, 2017
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 5, 2017

📌 Commit 27b50f6 has been approved by nikomatsakis

@arielb1 arielb1 added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 5, 2017
@arielb1
Copy link
Contributor

arielb1 commented Sep 5, 2017

[01:05:41]    Compiling rustc v0.0.0 (file:///checkout/src/librustc)

[01:05:47] error: unused import: `DummyCrateStore`

[01:05:47]     --> /checkout/src/librustc/session/config.rs:1954:32

[01:05:47]      |

[01:05:47] 1954 |     use middle::cstore::{self, DummyCrateStore};

[01:05:47]      |                                ^^^^^^^^^^^^^^^

[01:05:47]      |

[01:05:47] note: lint level defined here

[01:05:47]     --> /checkout/src/librustc/lib.rs:20:9

[01:05:47]      |

[01:05:47] 20   | #![deny(warnings)]

[01:05:47]      |         ^^^^^^^^

[01:05:47]      = note: #[deny(unused_imports)] implied by #[deny(warnings)]

[01:05:47] 

[01:05:47] error: unused import: `std::rc::Rc`

[01:05:47]     --> /checkout/src/librustc/session/config.rs:1960:9

[01:05:47]      |

[01:05:47] 1960 |     use std::rc::Rc;

[01:05:47]      |         ^^^^^^^^^^^

[01:05:47] 

@bors r-

@arielb1 arielb1 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 Sep 5, 2017
@michaelwoerister michaelwoerister force-pushed the remove-cstore-from-session branch from 27b50f6 to 8a7d919 Compare September 6, 2017 08:20
@michaelwoerister michaelwoerister force-pushed the remove-cstore-from-session branch from 8a7d919 to ef96a41 Compare September 6, 2017 10:47
@michaelwoerister
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Sep 6, 2017

📌 Commit ef96a41 has been approved by nikomatsakis

@Mark-Simulacrum
Copy link
Member

@bors rollup

@Mark-Simulacrum
Copy link
Member

@bors rollup- actually, this feels like it might have perf implications

@michaelwoerister michaelwoerister 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 Sep 7, 2017
@alexcrichton
Copy link
Member

Note that this is also likely to conflict quite a bit with #44142, where I think a lot of the tcx.sess.cstore -> tcx.cstore() may no longer be necessary after that lands

@michaelwoerister
Copy link
Member Author

@alexcrichton I don't mind holding off on this PR until #44142 is merged.

@alexcrichton
Copy link
Member

@bors: r-

Ok gonna r- this until #44142 lands. I feel bad though as that's gonna be a nasty merge, so I went ahead and did it!

I rebased this commit in alexcrichton@a00804b and then tacked alexcrichton@e3e88cd on top which makes CrateStore entirely private, yay!

@michaelwoerister
Copy link
Member Author

Thanks, @alexcrichton! I'll close this then in favor of #44142.

@alexcrichton
Copy link
Member

I've reopened this as #44420

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 11, 2017
…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 added a commit to frewsxcv/rust that referenced this pull request Sep 11, 2017
…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)
bors added a commit that referenced this pull request Sep 13, 2017
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 added a commit that referenced this pull request Sep 13, 2017
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants