Skip to content

Split up lint/builtin.rs #22206

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
Closed
kmcallister opened this issue Feb 12, 2015 · 8 comments
Closed

Split up lint/builtin.rs #22206

kmcallister opened this issue Feb 12, 2015 · 8 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@kmcallister
Copy link
Contributor

It's getting pretty big, and the code is very modular.
@kmcallister kmcallister added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Feb 12, 2015
@huonw
Copy link
Member

huonw commented Feb 12, 2015

It would be neat to move as many of these into an external crate as possible, to enable faster iteration.

@huonw huonw self-assigned this Feb 25, 2015
@huonw huonw mentioned this issue Feb 25, 2015
Merged
@huonw huonw removed their assignment Feb 25, 2015
huonw added a commit to huonw/rust that referenced this issue Feb 28, 2015
@huonw
Separate most of rustc::lint::builtin into a separate crate.
532cd5f 
This pulls out the implementations of most built-in lints into a
separate crate, to reduce edit-compile-test iteration times with
librustc_lint and increase parallelism. This should enable lints to be
refactored, added and deleted much more easily as it slashes the
edit-compile cycle to get a minimal working compiler to test with (`make
rustc-stage1`) from

    librustc -> librustc_typeck -> ... -> librustc_driver ->
        libcore -> ... -> libstd

to

    librustc_lint -> librustc_driver -> libcore -> ... libstd

which is significantly faster, mainly due to avoiding the librustc build
itself.

The intention would be to move as much as possible of the infrastructure
into the crate too, but the plumbing is deeply intertwined with librustc
itself at the moment. Also, there are lints for which diagnostics are
registered directly in the compiler code, not in their own crate
traversal, and their definitions have to remain in librustc.

This is a [breaking-change] for direct users of the compiler APIs:
callers of `rustc::session::build_session` or
`rustc::session::build_session_` need to manually call
`rustc_lint::register_builtins` on their return value.

This should make rust-lang#22206 easier.
bors added a commit that referenced this issue Feb 28, 2015
@bors
Auto merge of #22801 - huonw:crateify-lint, r=kmcallister
48aeaba 
This pulls out the implementations of most built-in lints into a
separate crate, to reduce edit-compile-test iteration times with
librustc_lint and increase parallelism. This should enable lints to be
refactored, added and deleted much more easily as it slashes the
edit-compile cycle to get a minimal working compiler to test with (`make
rustc-stage1`) from

    librustc -> librustc_typeck -> ... -> librustc_driver ->
        libcore -> ... -> libstd

to

    librustc_lint -> librustc_driver -> libcore -> ... libstd

which is significantly faster, mainly due to avoiding the librustc build
itself.

The intention would be to move as much as possible of the infrastructure
into the crate too, but the plumbing is deeply intertwined with librustc
itself at the moment. Also, there are lints for which diagnostics are
registered directly in the compiler code, not in their own crate
traversal, and their definitions have to remain in librustc.

This is a [breaking-change] for direct users of the compiler APIs:
callers of `rustc::session::build_session` or
`rustc::session::build_session_` need to manually call
`rustc_lint::register_builtins` on their return value.

This should make #22206 easier.
@richo
Copy link
Contributor

richo commented Mar 29, 2015

It seems like this can be closed, unless I'm misinterpreting the issue to actually split each lint out into it's own crate.

@caipre
Copy link
Contributor

caipre commented May 10, 2015

@kmcallister / @huonw close?

@huonw
Copy link
Member

huonw commented May 11, 2015

Most lints are now defined in the librustc_lint crate, but they're all in one huge file. It would make sense to break that file up (e.g. an "obvious" division would be breaking out separate modules for the lint groups).

@richo
Copy link
Contributor

richo commented May 11, 2015

I'm nominally interested in working on this (basically cc me)

On Sunday, May 10, 2015, Huon Wilson [email protected] wrote:

Most lints are now defined in the librustc_lint crate, but they're all in one
huge file
https://github.com/rust-lang/rust/blob/9ecc9896dedb426e3f4eb3d23dfc60192fe5275f/src/librustc_lint/builtin.rs.
It would make sense to break that file up (e.g. an "obvious" division would
be breaking out separate modules for the lint groups

rust/src/librustc_lint/lib.rs

Lines 122 to 128 in 9ecc989

add_lint_group!(sess, "bad_style",
NON_CAMEL_CASE_TYPES, NON_SNAKE_CASE, NON_UPPER_CASE_GLOBALS);
add_lint_group!(sess, "unused",
UNUSED_IMPORTS, UNUSED_VARIABLES, UNUSED_ASSIGNMENTS, DEAD_CODE,
UNUSED_MUT, UNREACHABLE_CODE, UNUSED_MUST_USE,
UNUSED_UNSAFE, PATH_STATEMENTS);

).

—
Reply to this email directly or view it on GitHub
#22206 (comment).

@caipre
Copy link
Contributor

caipre commented May 13, 2015

@richo: Were you claiming this? Otherwise I'll start on it.

@richo
Copy link
Contributor

richo commented May 13, 2015

Nope, I wanted to make sure that emails wind up in the part of my inbox that I read, but I'm probably not planning to work on it soon. Feel free!

@wesleywiser wesleywiser mentioned this issue Sep 16, 2015
Merged
wesleywiser added a commit to wesleywiser/rust that referenced this issue Sep 19, 2015
@wesleywiser
Split out the bad_style lints into a new module
ed77fcc 
Part of rust-lang#22206
wesleywiser added a commit to wesleywiser/rust that referenced this issue Sep 19, 2015
@wesleywiser
Split out the UNUSED_* lints into a new module
974dfe8 
Part of rust-lang#22206
bors added a commit that referenced this issue Sep 20, 2015
@bors
Auto merge of #28429 - wesleywiser:split_up_lints, r=alexcrichton
2c06bb9 
This breaks out some of the lints defined in `librustc_lint/builtin.rs` into two new modules: `unused` for the `UNUSED_*` lints and `bad_style` for the various style related lints as suggested in #22206. `builtin.rs` could probably get broken up more but this is a start.
@wesleywiser wesleywiser mentioned this issue Sep 22, 2015
Merged
bors pushed a commit that referenced this issue Sep 22, 2015
@wesleywiser
Split out the type lints into a new module
b0bcde4 
Part of #22206
bors added a commit that referenced this issue Sep 23, 2015
@bors
Auto merge of #28580 - wesleywiser:split_out_type_lints, r=alexcrichton
cefe5f2 
Move out the `TypeLimits` and `ImproperCTypes` lints into a separate module. 

Part of #22206
@wesleywiser
Copy link
Member

I've moved most of the lints out of builtin.rs but there's still a number of them in there. If anyone has ideas about how to deal with those, I'd be happy to work on it. Otherwise, I think this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@steveklabnik @caipre @kmcallister @richo @wesleywiser @huonw