Stream: git-wasmtime

Topic: wasmtime / PR #9536 Enable some Clippy conversion lints f...


view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):

alexcrichton opened PR #9536 from alexcrichton:more-lints-for-compiler to bytecodealliance:main:

Similar to how wasmtime::runtime has a few off-by-default lints turned on for it do the same for the compilation phase of wasmtime-cranelift. This is intended to help weed out lossy as casts and instead steer users to from or try_from conversions.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):

alexcrichton requested fitzgen for a review on PR #9536.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9536.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):

alexcrichton requested wasmtime-core-reviewers for a review on PR #9536.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:01):

pchickey submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:36):

alexcrichton merged PR #9536.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:44):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:44):

fitzgen created PR review comment:

We can't do this in Cargo.toml right? You can't use workspace lints and then also add some more, correct?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:45):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:45):

alexcrichton created PR review comment:

That's right yeah, I believe that this is the intended method of configuring lints if workspace lints are inherited and then per-crate configuration is desired

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:49):

fitzgen created PR review comment:

We might want to make these deny since CI is going to gate on them anyways, and since we are explicitly naming and opting into them, we don't have to worry about warnings on this rustc version vs that rustc version.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2024 at 17:49):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2024 at 04:16):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2024 at 04:16):

alexcrichton created PR review comment:

Personally I prefer to leave lints like this as warn instead of deny because in the middle of development it can be useful to not have to handle everything and defer it to later. Could you elaborate though on the rustc version part? I don't think I fully understand that.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2024 at 18:34):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2024 at 18:34):

fitzgen created PR review comment:

Personally I prefer to leave lints like this as warn instead of deny because in the middle of development it can be useful to not have to handle everything and defer it to later.

Fair, I don't have strong opinions here.

Could you elaborate though on the rustc version part? I don't think I fully understand that.

Oh I just mean how we do -D warnings in CI and how, if we didn't pin rustc versions, then updating to new rustcs could introduce new warnings and break CI despite no code changes to the repo itself. But that kind of update-the-version breakage isn't a concern for specific clippy lints that we enable, because we aren't opting into all default clippy lints, just specific ones that we know we are abiding by.


Last updated: Dec 23 2024 at 13:07 UTC