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

Ability to skip files or blocks entirely #10220

Closed
danielparks opened this issue Jan 22, 2023 · 7 comments · Fixed by #10229
Closed

Ability to skip files or blocks entirely #10220

danielparks opened this issue Jan 22, 2023 · 7 comments · Fixed by #10229
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages

Comments

@danielparks
Copy link
Contributor

Description

Some experimental code of mine generates a 48,000+ line function1 that takes 20+ minutes to process with Clippy with all lints allowed.

It would be really helpful to be able to skip or ignore files or blocks of code entirely.

This seems like a fairly niche use-case, so perhaps this isn’t worth pursuing.

Discarded idea: using a feature

I considered adding a clippy feature, but this is a bit inconvenient. I usually run cargo clippy --all-features so hiding the code if feature = "clippy" works well, but I also run cargo test --all-features so I need to change one or the other. This is additionally complicated by needing to configure CI correctly.

These are not insurmountable problems at all, but they do make it harder, especially for new people who want to contribute to my project.

Proposal: setting --cfg clippy when Clippy runs

Code that you don’t want Clippy to look at could be guarded with #[cfg(not(clippy))].

I haven’t dug into the Clippy code yet, but this seems like it might be relatively easy to implement (I’m happy to volunteer).

I’m not sure if Clippy would need an option to disable the configuration option so that you can force it to look at all code. For my usage, I would be happy to just remove the #[cfg(not(clippy))] line and run clippy again, but I could imagine that some people would want to run Clippy on some code only some times.

I suppose another solution would be for cargo clippy to accept --cfg options.

Version

rustc 1.66.1 (90743e729 2023-01-10)
binary: rustc
commit-hash: 90743e7298aca107ddaa0c202a4d3604e29bfeb6
commit-date: 2023-01-10
host: x86_64-apple-darwin
release: 1.66.1
LLVM version: 15.0.2

Additional Labels

@rustbot label +C-enhancement

Footnotes

  1. The code is actually used in a private branch on another crate, but there is an example that generates the function. This is all a bit horrific, and I’m still considering just shelving the idea, but it’s much faster than anything else I’ve tried.

@rustbot rustbot added the C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages label Jan 22, 2023
@danielparks
Copy link
Contributor Author

FWIW, just building the code takes less than 30 seconds additional, so it’s not just a matter of Rust taking a long time with a lot of code.

@smoelius
Copy link
Contributor

What commands should one use to reproduce the 20+ minute run? (When I just git clone ..., cd iter-matcher, and cargo clippy, it finishes almost instantaneously.)

@danielparks
Copy link
Contributor Author

iter-matcher just generates code, it doesn’t build it. I published a branch on another crate to demonstrate. I'll try to keep it clean, but this is all under active development.

git clone --branch slow-clippy https://github.com/danielparks/htmlize.git
cd htmlize
cargo clippy --all-features

@smoelius
Copy link
Contributor

Based on a cursory analysis, this appears to be where the time is spent:

    at clippy_utils/src/mir/transitive_relation.rs:21
    at clippy_utils/src/mir/possible_borrower.rs:48
    at clippy_utils/src/mir/possible_borrower.rs:190
    at clippy_lints/src/redundant_clone.rs:85

These line numbers are with respect to the latest nightly (e0ee58b).

This is an area of code actively being worked on for other reasons (see #10134). Hopefully, we'll have a fix soon. (cc: @Jarcho)

@Alexendoo
Copy link
Member

In the meanwhile, clippy does pass --cfg cargo-clippy so you could use #[cfg(not(feature = "cargo-clippy"))]. That does need documenting somewhere

@danielparks
Copy link
Contributor Author

@Alexendoo that works, thanks! (FWIW, it would have to pass --cfg 'feature="cargo-clippy"').

I guess all that remains is documenting it somewhere. I’ll try to take a crack at that in the next few days (if nobody beats me to it).

danielparks added a commit to danielparks/rust-clippy that referenced this issue Jan 26, 2023
It is possible to use conditional compilation to prevent Clippy from
evaluating certain code at all. Unfortunately, it was no longer
documented anywhere. This adds a brief explanation of how to use the
feature with conditional compilation, and mentions a few downsides.

Fixes #rust-lang#10220 — Ability to skip files or blocks entirely
danielparks added a commit to danielparks/rust-clippy that referenced this issue Feb 11, 2023
It is possible to use conditional compilation to prevent Clippy from
evaluating certain code. This adds a brief explanation of how to use the
feature with conditional compilation, and mentions that generally it’s
preferable to use something like `#[allow(clippy::all)]`.

Fixes rust-lang#10220 — Ability to skip files or blocks entirely
@danielparks
Copy link
Contributor Author

I’ve renamed the iter-matcher crate to matchgen, so here are the new instructions for reproducing the slow Clippy behavior:

git clone --branch slow-clippy https://github.com/danielparks/matchgen.git
cd matchgen
cargo clippy

It took 22 minutes on my machine. This generates code with #[allow(clippy::all)]. The code that generates the big function is in matchgen_tests/build.rs; it generates $OUT_DIR/test-matchers.rs.

I figure that a function that’s tens of thousands of lines long isn’t a very high priority, but I’ll reinvestigate and possibly produce a minimal test case once #10134 is finished. (This can’t be the same issue because it’s in stable Clippy, but it sounds like it may be in code that’s going to be significantly changed anyway.)

bors added a commit that referenced this issue Apr 3, 2023
Document `cargo-clippy` feature

It is possible to use conditional compilation to prevent Clippy from evaluating certain code at all. Unfortunately, it was no longer documented anywhere. This adds a brief explanation of how to use the feature with conditional compilation, and mentions a few downsides.

Fixes #10220 — Ability to skip files or blocks entirely
changelog: none
<!-- changelog_checked -->
@bors bors closed this as completed in 471de0c Apr 3, 2023
J-ZhengLi pushed a commit to TogetherGame/rust-clippy that referenced this issue Aug 26, 2023
It is possible to use conditional compilation to prevent Clippy from
evaluating certain code. This adds a brief explanation of how to use the
feature with conditional compilation, and mentions that generally it’s
preferable to use something like `#[allow(clippy::all)]`.

Fixes rust-lang#10220 — Ability to skip files or blocks entirely
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants