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

Separate most of rustc::lint::builtin into a separate crate. #22801

Merged
merged 2 commits into from
Feb 28, 2015

Conversation

huonw
Copy link
Member

@huonw huonw commented Feb 25, 2015

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.

@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@huonw
Copy link
Member Author

huonw commented Feb 25, 2015

r? @kmcallister

@rust-highfive rust-highfive assigned kmcallister and unassigned eddyb Feb 25, 2015
@nagisa
Copy link
Member

nagisa commented Feb 25, 2015

Nice improvement!

Since we have a separate crate for lints now, would be nice to split librustc_lint/builtin.rs into several files, e.g. each for a lint.

@huonw
Copy link
Member Author

huonw commented Feb 25, 2015

@nagisa yeah definitely, that's #22206. I don't have time to do it all immediately, but this patch makes it much easier to do incrementally since the compile times are much shorter. I thought it better to land this (to let everyone get the benefits now) than have it blocked on me doing the reorganisation first.

//! Lints in the Rust compiler.
//!
//! This contains lints which can feasibly be implemented as their own
//! AST visitor. Also `rustc::lint::builtin`, which contains the
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "Also see"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup

@bors
Copy link
Contributor

bors commented Feb 25, 2015

☔ The latest upstream changes (presumably #22796) made this pull request unmergeable. Please resolve the merge conflicts.

@huonw
Copy link
Member Author

huonw commented Feb 26, 2015

Rebased & fixed the missing word.

@kmcallister
Copy link
Contributor

@bors: r+ 26ea732

@bors
Copy link
Contributor

bors commented Feb 27, 2015

⌛ Testing commit 26ea732 with merge 35923cb...

@bors
Copy link
Contributor

bors commented Feb 27, 2015

💔 Test failed - auto-win-32-nopt-t

@alexcrichton
Copy link
Member

@bors: retry clean

@bors
Copy link
Contributor

bors commented Feb 28, 2015

☔ The latest upstream changes (presumably #22860) made this pull request unmergeable. Please resolve the merge conflicts.

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.
@huonw
Copy link
Member Author

huonw commented Feb 28, 2015

@bors r=kmcallister 3909

@bors
Copy link
Contributor

bors commented Feb 28, 2015

⌛ Testing commit 3909253 with merge 48aeaba...

bors added a commit that referenced this pull request Feb 28, 2015
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.
@Manishearth
Copy link
Member

\o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants