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.

Sign up for GitHub

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

Jump to bottom

Move more of rustc::lint into rustc_lint #68045

Merged
merged 14 commits into from
Jan 12, 2020
Merged

Move more of rustc::lint into rustc_lint #68045

merged 14 commits into from
Jan 12, 2020

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jan 9, 2020 •
edited by Mark-Simulacrum
Loading

Based on #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 Slimmer syntax #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.

@rust-highfive

This comment has been minimized.

Sign in to view
@Centril Centril added the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Jan 9, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@Centril Centril force-pushed the liberate-lints branch 2 times, most recently from d288c66 to b980596 Compare January 10, 2020 06:13
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@Centril Centril force-pushed the liberate-lints branch 3 times, most recently from 9297562 to bc1c936 Compare January 11, 2020 06:41
@Centril Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jan 11, 2020
@Centril
Copy link
Contributor Author

Centril commented Jan 11, 2020

This is ready for review now.

Mark-Simulacrum
Mark-Simulacrum reviewed Jan 11, 2020
View reviewed changes
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

Changes look good.

It'd be helpful to include a description of the new/old crate deps in the PR description I think, but not critical (would help me understand exactly how things have moved at a high level).

src/librustc_lint/late.rs Show resolved Hide resolved
src/librustc/lint/levels.rs Outdated Show resolved Hide resolved
src/librustc/lint.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor Author

Centril commented Jan 11, 2020

Added a PR description. :)

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jan 11, 2020

📌 Commit 51078ce has been approved by Mark-Simulacrum

@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 11, 2020
Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
@Centril
Rollup merge of rust-lang#68045 - Centril:liberate-lints, r=Mark-Simu…
7f81ad9 
…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`.
@Centril Centril mentioned this pull request Jan 11, 2020
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 11, 2020
@JohnTitor
Rollup merge of rust-lang#68045 - Centril:liberate-lints, r=Mark-Simu…
97c5332 
…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`.
@JohnTitor JohnTitor mentioned this pull request Jan 11, 2020
Closed
Centril added a commit to Centril/rust that referenced this pull request Jan 12, 2020
@Centril
Rollup merge of rust-lang#68045 - Centril:liberate-lints, r=Mark-Simu…
cc51d03 
…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 Centril mentioned this pull request Jan 12, 2020
Merged
bors added a commit that referenced this pull request Jan 12, 2020
@bors
Auto merge of #68142 - Centril:rollup-dl232e9, r=Centril
0b6c116 
Rollup of 6 pull requests

Successful merges:

 - #67494 (Constify more of alloc::Layout)
 - #67867 (Correctly check for opaque types in `assoc_ty_def`)
 - #67948 (Galloping search for binary_search_util)
 - #68045 (Move more of `rustc::lint` into `rustc_lint`)
 - #68089 (Unstabilize `Vec::remove_item`)
 - #68108 (Add suggestions when encountering chained comparisons)

Failed merges:

r? @ghost
@bors bors merged commit 51078ce into rust-lang:master Jan 12, 2020
@Centril Centril deleted the liberate-lints branch January 12, 2020 05:42
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Jan 12, 2020
@JohnTitor
Rustup to rust-lang/rust#68045
17e5517
@cjgillot cjgillot mentioned this pull request Jan 12, 2020
Closed
bors added a commit to rust-lang/rust-clippy that referenced this pull request Jan 13, 2020
@bors
Auto merge of #5042 - JohnTitor:rustup-0112, r=phansch
9c24afc 
Rustup to rust-lang/rust#68045

This is blocked because `rustc_lint::context` is not pub module and `CheckLintNameResult` is not marked as `pub use`.

changelog: none
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Jan 13, 2020
@JohnTitor
Rustup to rust-lang/rust#68045
c97f7fc
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Jan 13, 2020
@JohnTitor
Rustup to rust-lang/rust#68045
ce1c6b2
bors added a commit to rust-lang/rust-clippy that referenced this pull request Jan 13, 2020
@bors
Auto merge of #5042 - JohnTitor:rustup-0112, r=phansch
920cdb5 
Rustup to rust-lang/rust#68045

This is blocked because `rustc_lint::context` is not pub module and `CheckLintNameResult` is not marked as `pub use`.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnTitor JohnTitor JohnTitor left review comments

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Centril @rust-highfive @bors @Mark-Simulacrum @JohnTitor