Skip to content

miri clippy failing on Windows #3324

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

Closed
RossSmyth opened this issue Feb 25, 2024 · 3 comments · Fixed by #3542
Closed

miri clippy failing on Windows #3324

RossSmyth opened this issue Feb 25, 2024 · 3 comments · Fixed by #3542
Labels
A-dev Area: working on Miri as a developer C-bug Category: This is a bug.

Comments

@RossSmyth
Copy link
Contributor

Currently Clippy on Windows fails because of Windows specific code in
https://github.com/rust-lang/miri/blob/bffd41b00bf593bcf655e16748e223f8302f3de0/src/shims/os_str.rs

Clippy Warnings PS C:\Users\Ross\Documents\miri> .\miri clippy $ cargo +miri clippy --manifest-path C:\Users\Ross\Documents\miri\Cargo.toml --all-targets warning: casting `u8` to `u16` may become silently lossy if you later change the type --> src\shims\os_str.rs:282:58 | 282 | if converted.get(1).copied() == Some(b':' as u16) | ^^^^^^^^^^^ help: try: `u16::from(b':')` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless note: the lint level is defined here --> src\lib.rs:46:5 | 46 | clippy::cast_lossless, | ^^^^^^^^^^^^^^^^^^^^^

warning: casting u8 to u16 may become silently lossy if you later change the type
--> src\shims\os_str.rs:283:62
|
283 | && converted.get(2).copied() == Some(b'/' as u16)
| ^^^^^^^^^^^ help: try: u16::from(b'/')
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless

warning: casting u8 to u16 may become silently lossy if you later change the type
--> src\shims\os_str.rs:287:45
|
287 | converted.insert(0, b'/' as u16);
| ^^^^^^^^^^^ help: try: u16::from(b'/')
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless

warning: casting u8 to u16 may become silently lossy if you later change the type
--> src\shims\os_str.rs:293:58
|
293 | if converted.get(0).copied() == Some(b'\' as u16)
| ^^^^^^^^^^^^ help: try: u16::from(b'\\')
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless

warning: casting u8 to u16 may become silently lossy if you later change the type
--> src\shims\os_str.rs:294:62
|
294 | && converted.get(2).copied() == Some(b':' as u16)
| ^^^^^^^^^^^ help: try: u16::from(b':')
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless

warning: casting u8 to u16 may become silently lossy if you later change the type
--> src\shims\os_str.rs:295:62
|
295 | && converted.get(3).copied() == Some(b'\' as u16)
| ^^^^^^^^^^^^ help: try: u16::from(b'\\')
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless

Clippy should be run in CI for all cfgs. In an ideal world the only change that would be needed would be changing this

run: ./miri clippy -- -D warnings

to this

 run: ./miri clippy --target x86_64-pc-windows-msvc --target x86_64-unknown-linux-gnu --target x86_64-apple-darwin -- -D warnings 

and whatever other targets are needed.

But there is an issue with this. You need to install the stdlib for each target. The typical way is with rustup and running, for example, rustup target add aarch64-apple-darwin. But this is not supported for custom toolchains like +miri.

I guess we can build a sysroot for each target then run clippy?

@RalfJung
Copy link
Member

The toolchain for this job is installed here. It should be possible to install additional targets there.

@RossSmyth
Copy link
Contributor Author

Well I can't run it on my computer because jemalloc fails to build for some reason when cross checking. I'll just see if CI can do it.

@RossSmyth
Copy link
Contributor Author

Oh, yeah.
tikv/jemallocator#22

@RalfJung RalfJung added A-meta Not about any part of Miri per se, but about shaping the environment to make something in/with Miri C-bug Category: This is a bug. A-dev Area: working on Miri as a developer and removed A-meta Not about any part of Miri per se, but about shaping the environment to make something in/with Miri labels May 3, 2024
@bors bors closed this as completed in 4b2a45a May 3, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dev Area: working on Miri as a developer C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants