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

Move Sessions into (new) librustc_session #66878

Merged
merged 18 commits into from
Dec 3, 2019

Conversation

Mark-Simulacrum
Copy link
Member

This PR moves ParseSess and Session from their current locations into a new crate, librustc_session.

There are several intents behind this change. librustc is a very large crate, and we want to split it up over time -- this movement removes the sizeable session module from it. It also helps allow for future movement of things not coupled to TyCtxt but coupled to Session out of the crate.

This movement allows allows for a future follow-up PR which unifies Session and ParseSess, allowing for a single source of truth for APIs interested in global options throughout the compiler; the ParseSess is already created directly as a member of Session in the current compiler (i.e., we do not first construct a ParseSess and then move it into Session later in the compilation).

This PR intentionally avoids changing numerous imports throughout the tree to new locations of the moved types; this is needless noise and can be done as needed.

In the process of moving the sessions back, the lint system received an update as well -- notably, early buffered lints are no longer ad-hoc declared as enum pairs and later associated with proper lint declarations. They are still separately handled (buffered), it is a little unclear whether this is truly necessary, but regardless is left for future PRs.

Many of the types moved back are sort of ad-hoc placed into the same crate (librustc_session) instead of creating other crates; it's unclear whether this is actually a good thing, but it seemed better than creating numerous tiny crates which served no purpose on their own.

@rust-highfive

This comment has been minimized.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2019
@Mark-Simulacrum
Copy link
Member Author

Tentatively r? @Centril for at least initial review

@rust-highfive rust-highfive assigned Centril and unassigned zackmdavis Nov 29, 2019
@rust-highfive

This comment has been minimized.

@petrochenkov petrochenkov self-assigned this Nov 29, 2019
Cargo.lock Show resolved Hide resolved
@@ -12,6 +12,8 @@ use syntax::ast;
use syntax::edition::Edition;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what, if anything, is holding us back from moving more lints from this file to rustc_lints.
It might be good to structure things as one lint per file like is done in Clippy.
(Also not actionable but making a note of it... cc @oli-obk)

src/librustc/lint/internal.rs Outdated Show resolved Hide resolved
src/libsyntax/early_buffered_lints.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,94 @@
use syntax_pos::edition::Edition;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reaction to this is that I would prefer the following layout for feature gating:

  • A crate dedicated to declaring feature gates and the structs ("data crate"); this would consist of active.rs, accepted.rs, removed.rs, most of builtin_attrs.rs (at least the parts not dependent on libsyntax), the structs from mod.rs). Let's call this crate librustc_feature and we'd have librustc_session depend on it.

  • A module for checking (check.rs -> gate_features.rs), possibly as part of librustc_passes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented the first part of this in #66895, which should be sufficient for this PR to build upon.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will rebase atop that.

Cargo.lock Show resolved Hide resolved
src/librustc_session/config.rs Show resolved Hide resolved
src/librustc_session/lib.rs Show resolved Hide resolved
src/librustc_session/search_paths.rs Show resolved Hide resolved
@rust-highfive

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member Author

Rebased on #66895 (but will need to do so again once it lands).

Centril added a commit to Centril/rust that referenced this pull request Nov 30, 2019
Feature gating *declarations* => new crate `rustc_feature`

This PR moves the data-oriented parts of feature gating into its own crate, `rustc_feature`.
The parts consist of some data types as well as `accepted`, `active`, `removed`, and `builtin_attrs`.

Feature gate checking itself remains in `syntax::feature_gate::check`. The parts which define how to emit feature gate errors could probably be moved to `rustc_errors` or to the new `rustc_session` crate introduced in rust-lang#66878. The visitor itself could probably be moved as a pass in `rustc_passes` depending on how the dependency edges work out.

The PR also contains some drive-by cleanup of feature gate checking. As such, the PR probably best read commit-by-commit.

r? @oli-obk
cc @petrochenkov
cc @Mark-Simulacrum
@Centril Centril 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2019
@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 1, 2019
@Mark-Simulacrum
Copy link
Member Author

r? @Centril

Rebased atop your PR and force pushed.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member Author

@bors r=Centril

@bors
Copy link
Contributor

bors commented Dec 3, 2019

📌 Commit 68fb218 has been approved by Centril

@bors 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 Dec 3, 2019
bors added a commit that referenced this pull request Dec 3, 2019
de-macro-ize feature gate checking

This PR removes the usage of macros in feature gating preferring to take in symbols instead. The aim is to make feature gating more understandable overall while also reducing some duplication.

To check whether a feature gate is active, `tcx.features().on(sym::my_feature)` can be used.
Inside `PostExpansionVisitor` it's however better to use `self.gate(...)` which provides a nicer API atop of that. Outside of the visitor, if `gate_feature` can be used, it should be preferred over `feature_err`.

As a follow-up, and once #66878 is merged, it might be a good idea to add a method to `Session` for more convenient access to `gate_feature` and `feature_err` respectively.

Originally I intended to go for a `HashSet` representation, but this felt like a smaller change to just expand to a `match` on the symbol => field pairs.

r? @oli-obk
Centril added a commit to Centril/rust that referenced this pull request Dec 3, 2019
…ntril

Move Sessions into (new) librustc_session

This PR moves `ParseSess` and `Session` from their current locations into a new crate, `librustc_session`.

There are several intents behind this change. librustc is a very large crate, and we want to split it up over time -- this movement removes the sizeable session module from it. It also helps allow for future movement of things not coupled to TyCtxt but coupled to Session out of the crate.

This movement allows allows for a future follow-up PR which unifies Session and ParseSess, allowing for a single source of truth for APIs interested in global options throughout the compiler; the ParseSess is already created directly as a member of Session in the current compiler (i.e., we do not first construct a ParseSess and then move it into Session later in the compilation).

This PR intentionally avoids changing numerous imports throughout the tree to new locations of the moved types; this is needless noise and can be done as needed.

In the process of moving the sessions back, the lint system received an update as well -- notably, early buffered lints are no longer ad-hoc declared as enum pairs and later associated with proper lint declarations. They are still separately handled (buffered), it is a little unclear whether this is truly necessary, but regardless is left for future PRs.

Many of the types moved back are sort of ad-hoc placed into the same crate (librustc_session) instead of creating other crates; it's unclear whether this is actually a good thing, but it seemed better than creating numerous tiny crates which served no purpose on their own.
bors added a commit that referenced this pull request Dec 3, 2019
Rollup of 7 pull requests

Successful merges:

 - #66750 (Update the `wasi` crate for `wasm32-wasi`)
 - #66878 (Move Sessions into (new) librustc_session)
 - #66903 (parse_enum_item -> parse_enum_variant)
 - #66951 (miri: add throw_machine_stop macro)
 - #66957 (Change Linker for x86_64-fortanix-unknown-sgx target to rust-lld)
 - #66960 ([const-prop] Fix ICE calculating enum discriminant)
 - #66973 (Update the minimum external LLVM to 7)

Failed merges:

r? @ghost
@bors bors merged commit 68fb218 into rust-lang:master Dec 3, 2019
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 3, 2019
flip1995 pushed a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 4, 2019
flip1995 pushed a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 4, 2019
bors added a commit to rust-lang/rust-clippy that referenced this pull request Dec 4, 2019
Rustup to rust-lang/rust#66878

I need to sleep now, feel free to pick it up.
The output of the `lint_without_lint_pass` test seems to disappear, I'm not sure why.. :/

changelog: none
flip1995 pushed a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 4, 2019
bors added a commit to rust-lang/rust-clippy that referenced this pull request Dec 4, 2019
Rustup to rust-lang/rust#66878

I need to sleep now, feel free to pick it up.
The output of the `lint_without_lint_pass` test seems to disappear, I'm not sure why.. :/

changelog: none
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