Stream: git-wasmtime

Topic: wasmtime / PR #9209 Warn against `clippy::cast_possible_t...

Wasmtime GitHub notifications bot (Sep 06 2024 at 19:09):

alexcrichton opened PR #9209 from alexcrichton:cast_possible_truncation to bytecodealliance:main:

This commit explicitly enables the clippy::cast_possible_truncation lint in Clippy for just the wasmtime::runtime module. This does not enable it for the entire workspace since it's a very noisy lint and in general has a low signal value. For the domain that wasmtime::runtime is working in, however, this is a much more useful lint. We in general want to be very careful about casting between usize, u32, and u64 and the purpose of this module-targeted lint is to help with just that. I was inspired to do this after reading over #9206 where especially when refactoring code and changing types I think it would be useful to have locations flagged as "truncation may happen here" which previously weren't truncating.

The failure mode for this lint is that panics might be introduced where truncation is explicitly intended. Most of the time though this isn't actually desired so the more practical consequence of this lint is to probably slow down wasmtime ever so slightly and bloat it ever so slightly by having a few more checks in a few places. This is likely best addressed in a more comprehensive manner, however, rather than specifically for just this one case. This problem isn't unique to just casts, but to many other forms of .unwrap() for example.

<!--
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.html

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

Wasmtime GitHub notifications bot (Sep 06 2024 at 19:09):

alexcrichton requested elliottt for a review on PR #9209.

Wasmtime GitHub notifications bot (Sep 06 2024 at 19:09):

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

Wasmtime GitHub notifications bot (Sep 06 2024 at 19:50):

alexcrichton updated PR #9209.

Wasmtime GitHub notifications bot (Sep 06 2024 at 21:32):

elliottt submitted PR review:

Looks great!

Wasmtime GitHub notifications bot (Sep 06 2024 at 21:32):

elliottt created PR review comment:

Does this indirect through Option because the error types are different between line 67 and 69? Otherwise, wouldn't a combination of and_then and unwrap_or_else work here?

Wasmtime GitHub notifications bot (Sep 06 2024 at 22:25):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Sep 06 2024 at 22:25):

alexcrichton created PR review comment:

Yeah this had to do with different error types across the two functions and I figured options was the easiest way to go here. This is code is on the older side in Wasmtime and is probably ripe for some improvement.

Wasmtime GitHub notifications bot (Sep 06 2024 at 22:40):

alexcrichton merged PR #9209.

Last updated: Oct 23 2024 at 20:03 UTC