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

Slimmer syntax #68133

Merged
merged 14 commits into from
Feb 1, 2020
Merged

Slimmer syntax #68133

merged 14 commits into from
Feb 1, 2020

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jan 11, 2020

High-level summary of changes:

  • The syntax::node_count pass is moved into rustc_ast_passes. This works towards improving The rustc_query_impl crate is too big, which hurts compile times for the compiler itself #65031 by making compiling syntax go faster.

  • The syntax::{GLOBALS, with_globals, ..} business is consolidated into syntax::attr for cleaner code and future possible improvements.

  • The pretty printer loses its dependency on ParseSess, opting to use SourceMap & friends directly instead.

  • Some drive by cleanup of syntax::attr::HasAttr happens.

  • Builtin attribute logic (syntax::attr::builtin) + syntax::attr::allow_internal_unstable is moved into a new rustc_attr crate. More logic from syntax::attr should be moved into that crate over time. This also means that syntax loses all mentions of ParseSess, which enables the next point.

  • The pretty printer syntax::print is moved into a new crate rustc_ast_pretty.

  • rustc_session::node_id is moved back as syntax::node_id. As a result, syntax gets to drop dependencies on rustc_session (and implicitly rustc_target), rustc_error_codes, and rustc_errors. Moreover rustc_hir gets to drop its dependency on rustc_session as well. At this point, these crates are mostly "pure data crates", which is approaching a desirable end state.

    • We should consider renaming syntax to rustc_ast now.

@rust-highfive

This comment has been minimized.

Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
…lacrum

Move more of `rustc::lint` into `rustc_lint`

Based on rust-lang#67806.

Here we try to consolidate more of the linting infra into `rustc::lint`. Some high-level notes:

- We now store an `Lrc<dyn Any + Send + Sync>` as opposed to `Lrc<LintStore>` in the `GlobalCtxt`. This enables us to avoid referring to the type, breaking a cyclic dependency, and so we can move things from `rustc::lint` to `rustc_lint`.

- `in_derive_expansion` is, and needs to, be moved as a method on `Span`.

- We reduce the number of ways on `tcx` to emit a lint so that the developer UX is more streamlined.

- `LintLevelsBuilder` is moved to `rustc_lint::levels`, leaving behind `LintLevelMap/Set` in a purified form due to current constraints (hopefully fixable in the future after rust-lang#68133).

- `struct_lint_level` is moved to `rustc::lint` due to current dependency constraints.

- `rustc::context` is moved to `rustc_lint::context`.

- The visitors in `rustc::lint` are moved to `rustc_lint::passes`.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 11, 2020
…lacrum

Move more of `rustc::lint` into `rustc_lint`

Based on rust-lang#67806.

Here we try to consolidate more of the linting infra into `rustc::lint`. Some high-level notes:

- We now store an `Lrc<dyn Any + Send + Sync>` as opposed to `Lrc<LintStore>` in the `GlobalCtxt`. This enables us to avoid referring to the type, breaking a cyclic dependency, and so we can move things from `rustc::lint` to `rustc_lint`.

- `in_derive_expansion` is, and needs to, be moved as a method on `Span`.

- We reduce the number of ways on `tcx` to emit a lint so that the developer UX is more streamlined.

- `LintLevelsBuilder` is moved to `rustc_lint::levels`, leaving behind `LintLevelMap/Set` in a purified form due to current constraints (hopefully fixable in the future after rust-lang#68133).

- `struct_lint_level` is moved to `rustc::lint` due to current dependency constraints.

- `rustc::lint::context` is moved to `rustc_lint::context`.

- The visitors in `rustc::lint` are moved to `rustc_lint::passes`.
Centril added a commit to Centril/rust that referenced this pull request Jan 12, 2020
…lacrum

Move more of `rustc::lint` into `rustc_lint`

Based on rust-lang#67806.

Here we try to consolidate more of the linting infra into `rustc::lint`. Some high-level notes:

- We now store an `Lrc<dyn Any + Send + Sync>` as opposed to `Lrc<LintStore>` in the `GlobalCtxt`. This enables us to avoid referring to the type, breaking a cyclic dependency, and so we can move things from `rustc::lint` to `rustc_lint`.

- `in_derive_expansion` is, and needs to, be moved as a method on `Span`.

- We reduce the number of ways on `tcx` to emit a lint so that the developer UX is more streamlined.

- `LintLevelsBuilder` is moved to `rustc_lint::levels`, leaving behind `LintLevelMap/Set` in a purified form due to current constraints (hopefully fixable in the future after rust-lang#68133).

- `struct_lint_level` is moved to `rustc::lint` due to current dependency constraints.

- `rustc::lint::context` is moved to `rustc_lint::context`.

- The visitors in `rustc::lint` are moved to `rustc_lint::passes`.
@bors

This comment has been minimized.

@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 12, 2020
@Centril Centril changed the title [WIP] Slimmer syntax Slimmer syntax Jan 17, 2020
@Centril
Copy link
Contributor Author

Centril commented Jan 17, 2020

r? @petrochenkov

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

src/librustc_ast_pretty/pprust.rs Outdated Show resolved Hide resolved
src/librustc_interface/util.rs Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

We should consider renaming syntax to rustc_ast now.

👍

@Centril
Copy link
Contributor Author

Centril commented Jan 17, 2020

We should consider renaming syntax to rustc_ast now.

👍

Cool; I'll follow up with this. :)

@Centril Centril 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 Jan 18, 2020
@Centril
Copy link
Contributor Author

Centril commented Jan 18, 2020

Addressed the review comments.

@rust-highfive

This comment has been minimized.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2020

📌 Commit 1fbab97f3d177c4f4d58f69a2b73d938740a2abc has been approved by petrochenkov

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2020
@rust-highfive

This comment has been minimized.

@Centril

This comment has been minimized.

@bors

This comment has been minimized.

@Centril
Copy link
Contributor Author

Centril commented Feb 1, 2020

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Feb 1, 2020

📌 Commit 1a3141c has been approved by petrochenkov

@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 Feb 1, 2020
@bors
Copy link
Contributor

bors commented Feb 1, 2020

⌛ Testing commit 1a3141c with merge 13db650...

bors added a commit that referenced this pull request Feb 1, 2020
Slimmer syntax

High-level summary of changes:

- The `syntax::node_count` pass is moved into `rustc_ast_passes`. This works towards improving #65031 by making compiling `syntax` go faster.

- The `syntax::{GLOBALS, with_globals, ..}` business is consolidated into `syntax::attr` for cleaner code and future possible improvements.

- The pretty printer loses its dependency on `ParseSess`, opting to use `SourceMap` & friends directly instead.

- Some drive by cleanup of `syntax::attr::HasAttr` happens.

- Builtin attribute logic (`syntax::attr::builtin`) + `syntax::attr::allow_internal_unstable` is moved into a new `rustc_attr` crate. More logic from `syntax::attr` should be moved into that crate over time. This also means that `syntax` loses all mentions of `ParseSess`, which enables the next point.

- The pretty printer `syntax::print` is moved into a new crate `rustc_ast_pretty`.

- `rustc_session::node_id` is moved back as `syntax::node_id`. As a result, `syntax` gets to drop dependencies on `rustc_session` (and implicitly `rustc_target`), `rustc_error_codes`, and `rustc_errors`. Moreover `rustc_hir` gets to drop its dependency on `rustc_session` as well. At this point, these crates are mostly "pure data crates", which is approaching a desirable end state.

  - We should consider renaming `syntax` to `rustc_ast` now.
@bors
Copy link
Contributor

bors commented Feb 1, 2020

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing 13db650 to master...

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

📣 Toolstate changed by #68133!

Tested on commit 13db650.
Direct link to PR: #68133

💔 clippy-driver on windows: test-fail → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → 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 1, 2020
Tested on commit rust-lang/rust@13db650.
Direct link to PR: <rust-lang/rust#68133>

💔 clippy-driver on windows: test-fail → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → 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 1, 2020
@Centril Centril deleted the slimmer-syntax branch February 1, 2020 21:58
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 1, 2020
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants