Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46): 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::runtimehas a few off-by-default lints turned on for it do the same for the compilation phase ofwasmtime-cranelift. This is intended to help weed out lossyascasts and instead steer users tofromortry_fromconversions.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):alexcrichton requested fitzgen for a review on PR #9536.
Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9536.
Wasmtime GitHub notifications bot (Oct 31 2024 at 20:46):alexcrichton requested wasmtime-core-reviewers for a review on PR #9536.
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:01):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:36):alexcrichton merged PR #9536.
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:44):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:44):fitzgen created PR review comment:
We can't do this in
Cargo.tomlright? You can't use workspace lints and then also add some more, correct?
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:45):alexcrichton submitted PR review.
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
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:49):fitzgen created PR review comment:
We might want to make these
denysince 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.
Wasmtime GitHub notifications bot (Nov 01 2024 at 17:49):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Nov 02 2024 at 04:16):alexcrichton submitted PR review.
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.
Wasmtime GitHub notifications bot (Nov 04 2024 at 18:34):fitzgen submitted PR review.
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 warningsin CI and how, if we didn't pinrustcversions, then updating to newrustcs 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: Nov 22 2024 at 16:03 UTC