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 thewasmtime::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 thatwasmtime::runtime
is working in, however, this is a much more useful lint. We in general want to be very careful about casting betweenusize
,u32
, andu64
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.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton requested elliottt for a review on PR #9209.
alexcrichton requested wasmtime-core-reviewers for a review on PR #9209.
alexcrichton updated PR #9209.
elliottt submitted PR review:
Looks great!
elliottt created PR review comment:
Does this indirect through
Option
because the error types are different between line67
and69
? Otherwise, wouldn't a combination ofand_then
andunwrap_or_else
work here?
alexcrichton submitted PR review.
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.
alexcrichton merged PR #9209.
Last updated: Dec 23 2024 at 13:07 UTC