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

Allow clippy-driver maybe? #1544

Closed
yaahc opened this issue Oct 10, 2019 · 3 comments · Fixed by #3893
Closed

Allow clippy-driver maybe? #1544

yaahc opened this issue Oct 10, 2019 · 3 comments · Fixed by #3893
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Milestone

Comments

@yaahc
Copy link
Member

yaahc commented Oct 10, 2019

The Dream

So I'm playing around and testing how hard it would be to integrate clippy-driver into cargo build and cargo check by exporting RUSTC=clippy-driver. For most crates this works fine and it builds and outputs just as if it were rustc but also you get clippy warnings.

The Problem

Turns out libc's build.rs file doesn't like clippy very much, and when you try to build you get the following error.

Screenshot from 2019-10-10 15-10-29

Which seems to be caused by this section of the build.rs file

https://github.com/rust-lang/libc/blob/master/build.rs#L87-L96

The Proposal

I'm not sure if this is crazy talk but I'd like it if libc could play nicely with clippy-driver. I know old versions will still miscompile so iunno how important helpful fixing this in libc is, but I figured I'd report it anyways

I'm not an expert but I think using rustc -vV instead of rustc --version might be better and more consistent with how the rest of the ecosystem gets version info from the compiler (thinking about cargo and sccache here). Here is the comparable outputs of those two invocations on rustc and clippy-driver

Screenshot from 2019-10-10 15-09-50

@yaahc
Copy link
Member Author

yaahc commented Oct 10, 2019

One suggestion for fixing this in a backwards compatible(??) way would be to adjust the output of clippy-driver --version to output something like

rustc 1.??.0 + clippy 0.0.??

Lets just hope to god that nobody is depending directly on the output of clippy-driver --version

cc @Manishearth

@KodrAus
Copy link

KodrAus commented Oct 10, 2019

Hmm, it looks like serde uses pretty much the same approach, libraries depending on if_rust_version or version_check will possibly report an incorrect version (using clippy's instead of rustc's), and those using rustc-version should work.

Making clippy-driver's output support existing version parsers would be pragmatic, but it seems like trying to standardize around -vV would be more robust.

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 11, 2019

I'm not an expert but I think using rustc -vV

Sure, we could try doing that. PRs welcome :)

@tgross35 tgross35 added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Aug 29, 2024
@tgross35 tgross35 added this to the 1.x milestone Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants