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

Avoid running cargo+internal lints when not enabled #5505

Merged
merged 2 commits into from
Apr 25, 2020

Conversation

flip1995
Copy link
Member

r? @matthiaskrgr

changelog: none

@@ -59,8 +59,12 @@ fn is_empty_vec(value: &[String]) -> bool {

declare_lint_pass!(CargoCommonMetadata => [CARGO_COMMON_METADATA]);

impl EarlyLintPass for CargoCommonMetadata {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
impl LateLintPass<'_, '_> for CargoCommonMetadata {
Copy link
Member Author

Choose a reason for hiding this comment

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

Downside: The cargo lints are now LateLintPasses

@tesuji
Copy link
Contributor

tesuji commented Apr 23, 2020

What's the motivation of this change?

@matthiaskrgr
Copy link
Member

@lzutao when running clippy-driver foo.rs in a directory that had a big Cargo.toml somewhere in a parent path, clippy-driver would redundantly search for the nearest cargo manifest run clippy::cargo-lints which invoced cargo-metadata on the entire crate, slowing everything down significantly depending on the project complexity.
https://discordapp.com/channels/442252698964721669/459149871191031810/701877035936645192

https://discordapp.com/channels/442252698964721669/459149871191031810/702159543538417714

When I tested a version of this branch, it managed to speed up my testcase (running clippy-driver on testcases inside the rustc repo) by ~10x :)

@matthiaskrgr
Copy link
Member

Thanks a lot @flip1995 !
Feel free to r=me once github is working again :)

@flip1995
Copy link
Member Author

To add to this: I ran flamegraph on clippy-driver, when run inside the clippy repo dir on a single (hello-world) file. Without this change the cargo metadata collection took ~30-40% of the runtime.

@bors r=matthiaskrgr

@bors
Copy link
Collaborator

bors commented Apr 23, 2020

📌 Commit f31502f has been approved by matthiaskrgr

bors added a commit that referenced this pull request Apr 23, 2020
Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
@bors
Copy link
Collaborator

bors commented Apr 23, 2020

⌛ Testing commit f31502f with merge a7e6134...

@bors
Copy link
Collaborator

bors commented Apr 23, 2020

💥 Test timed out

@flip1995
Copy link
Member Author

@bors retry

bors added a commit that referenced this pull request Apr 23, 2020
Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
@bors
Copy link
Collaborator

bors commented Apr 23, 2020

⌛ Testing commit f31502f with merge dc1adc6...

@bors
Copy link
Collaborator

bors commented Apr 23, 2020

💔 Test failed - checks-action_test

@flip1995
Copy link
Member Author

@bors retry (please?)

bors added a commit that referenced this pull request Apr 23, 2020
Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
@bors
Copy link
Collaborator

bors commented Apr 23, 2020

⌛ Testing commit f31502f with merge 3c3c2cb...

@bors
Copy link
Collaborator

bors commented Apr 23, 2020

💔 Test failed - checks-action_test

@flip1995 flip1995 added the S-waiting-on-bors Status: The marked PR was approved and is only waiting bors label Apr 23, 2020
@matthiaskrgr

This comment has been minimized.

@flip1995
Copy link
Member Author

@bors retry

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Apr 25, 2020
…hiaskrgr

Avoid running cargo+internal lints when not enabled

r? @matthiaskrgr

changelog: none
This was referenced Apr 25, 2020
bors added a commit that referenced this pull request Apr 25, 2020
Rollup of 5 pull requests

Successful merges:

 - #5408 (Downgrade match_bool to pedantic)
 - #5505 (Avoid running cargo+internal lints when not enabled)
 - #5516 (Add a note to the beta sections of release.md)
 - #5517 (Deploy time travel)
 - #5523 (Add lifetime test case for `new_ret_no_self`)

Failed merges:

r? @ghost

changelog: rollup
@bors bors merged commit a33d64a into rust-lang:master Apr 25, 2020
@flip1995 flip1995 deleted the avoid_running_lints branch May 4, 2020 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants