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

Can clippy-driver be always used as replacement of rustc? #8035

Closed
ojeda opened this issue Nov 26, 2021 · 9 comments · Fixed by #8037
Closed

Can clippy-driver be always used as replacement of rustc? #8035

ojeda opened this issue Nov 26, 2021 · 9 comments · Fixed by #8037
Labels
C-question Category: Questions

Comments

@ojeda
Copy link
Contributor

ojeda commented Nov 26, 2021

When invoking Clippy (via Cargo or clippy-driver), we get the usual rustc outputs + the extra lints. However, is it guaranteed that clippy-driver will work as if rustc was called?

That is, can we rely on the generated objects to be the same as the ones that rustc would have generated (modulo unrelated non-deterministic build issues) or they should always be built without clippy-driver if they are intended for "production"?

A potential use case would be to always use clippy-driver instead of rustc in a build system, thus always having Clippy lints enabled (as long as one is OK paying the performance penalty).

(Followup to #6782.)

@rustbot label +C-question

@xFrednet
Copy link
Member

Hey, I have not worked on clippy-driver directly, so take this with a grain of salt.

Rustc uses a query system for compilation, which enables incremental builds. Clippy reuses this infrastructure and only registers new lint passes for Clippy lints. The lexer, parser, type checker etc. are the same. Meaning that until then everything should be equal. cargo clippy basically works like cargo check with more lints. So, you can use it as a slightly slower but more detailed cargo check replacement. Now, like cargo check Clippy doesn't create a binary file, it simply stops once all validation queries are done. Therefore, you can not really build your project with cargo clippy.

This structure also means that errors during the code generation would not be caught by running cargo clippy. These are luckily rare, at least I haven't heard of instances where cargo check passed but cargo build failed.

Does this answer your question? 🙃

@bjorn3
Copy link
Member

bjorn3 commented Nov 27, 2021

cargo clippy runs cargo check with clippy-driver instead of rustc as compiler. You can also run clippy-driver instead of rustc for cargo build by setting the RUSTC env var. And you if you normally run rustc directly you can run clippy-driver directly to compile your code. The question is if this is something that will remain the case forever.

@ojeda
Copy link
Contributor Author

ojeda commented Nov 27, 2021

Yeah, exactly what @bjorn3 says.

For context, I am asking this to see if, in the kernel, we can rely on CLIPPY=1 builds or we should e.g. warn that they should not be used for "actual" builds of the kernel (e.g. intended to be distributed to users). Furthermore, if the answer is positive, then it would mean we could contemplate removing the option and always build with Clippy enabled.

For instance, does the following change codegen that rustc will perform, or does it mean it is requesting an unoptimized MIR on top of what rustc may independently need/use?

rust-clippy/src/driver.rs

Lines 115 to 119 in 3720735

// FIXME: #4825; This is required, because Clippy lints that are based on MIR have to be
// run on the unoptimized MIR. On the other hand this results in some false negatives. If
// MIR passes can be enabled / disabled separately, we should figure out, what passes to
// use for Clippy.
config.opts.debugging_opts.mir_opt_level = Some(0);

A more general question is whether Clippy could be integrated into rustc (i.e. no driver needed).

@bjorn3
Copy link
Member

bjorn3 commented Nov 27, 2021

For instance, does the following change codegen that rustc will perform

Yes. It will disable all optimizations done on the MIR. This means that LLVM has more work to do, but it also disables a couple of optimizations for patterns that LLVM is currently unable to optimize well.

@ojeda
Copy link
Contributor Author

ojeda commented Nov 27, 2021

That is what I was afraid of... :(

Opened #8037 to document this -- please take a look. If later on the guarantee can be made, then it can be removed.

@flip1995
Copy link
Member

I was under the impression, that Clippy doesn't even do CG. It shouldn't be necessary, because Clippy has no lints (and probably won't have any in the future) that would trigger on a CG level. So I'm now wondering if it is possible to disable CG completely for Clippy, like when running cargo check?

@camsteffen
Copy link
Contributor

The following is just my own opinion...

When invoking Clippy (via Cargo or clippy-driver), we get the usual rustc outputs + the extra lints. However, is it guaranteed that clippy-driver will work as if rustc was called?

I think the short answer is no; clippy-driver is designed purely for linting with Clippy, not for actually compiling. Perhaps clippy-driver could handle this kind of usage with more care, but that's another discussion. The current guidance is "just use rustc to compile".

So I think the readme section on clippy-driver should communicate the above more clearly. Instead of saying "rustc replacement" and "replace your rustc compilation commands...", it should say to use clippy-driver to lint with Clippy and continue to use rustc to compile. We can still mention something about output/codegen guarantees, but that's more of a footnote.

@ojeda
Copy link
Contributor Author

ojeda commented Nov 28, 2021

I was under the impression, that Clippy doesn't even do CG. It shouldn't be necessary, because Clippy has no lints (and probably won't have any in the future) that would trigger on a CG level. So I'm now wondering if it is possible to disable CG completely for Clippy, like when running cargo check?

Sounds good to me. Another alternative instead of silently disabling codegen is to error out if a contradicting option is requested, e.g. --emit=link or obj.

@ojeda
Copy link
Contributor Author

ojeda commented Nov 28, 2021

...although someone out there may be using it for quick development cycles with lints enabled.

ojeda added a commit to ojeda/rust-clippy that referenced this issue Dec 14, 2021
Currently, `clippy-driver` may run codegen, but this is an
implementation detail.

See rust-lang#8035.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-clippy that referenced this issue Jan 11, 2022
Currently, `clippy-driver` may run codegen, but this is an
implementation detail.

See rust-lang#8035.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
@bors bors closed this as completed in b9cae79 Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-question Category: Questions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants