Deny warnings on CI - Githubissues

verus-lang / verus

Verified Rust for low-level systems code

MIT License

1.15k stars 66 forks source link

Deny warnings on CI #1127

Closed ouuan closed 4 months ago

ouuan commented 4 months ago

This mainly captures unused variables/imports related to ghost codes / no_std. Examples are #1121 and #1126.

Now the CI will fail, waiting for those two PRs.

ouuan commented 4 months ago

There are some concerns at https://github.com/rust-lang/cargo/issues/8424 regarding setting RUSTFLAGS. The main concern is that it may override rustflags in .cargo/config.toml. Now only the line_count tool uses a local .cargo/config.toml, and vargo is overriding the RUSTFLAGS env.

I'm not sure what's the best way to deny warnings on CI for Verus:

Setting RUSTFLAGS may break if we add a .cargo/config.toml in the future, and we might waste time finding out why it breaks.
Should we add -Dwarnings in vargo on CI?
Can we apply cargo clippy? It has a -D warnings command-line option.

utaal commented 4 months ago

Hi @ouuan, thanks for the PR, but I don't have the bandwidth at the moment to look into this. One of the other maintainers may be able to have a look later.

At the current stage of development, I think warnings in CI are acceptable.

ouuan commented 4 months ago

At the current stage of development, I think warnings in CI are acceptable.

Now we only have warnings with special compilation flags. It might be too strict if clippy is used, but the default warnings are easy to avoid.

utaal commented 4 months ago

We may be making some significant changes to the CI process soon, so I don't think this is the time to make this change (and I don't have time to look into the RUSTFLAGS issue) so I'll close this for now. If one of the other contributors @verus-lang/verus-devs wants to pick this up, please do.