1488003 - webdriver: Cast i32 to i64 using "::from" instead of "as"

Status: RESOLVED INCOMPLETE

(Testing :: geckodriver, enhancement, P2)

Description

[Andreas Tolfsen ❲:ato❳]

To ensure we don’t perform a lossy cast from i32 to i64, we should use 

> i64::from(x)

rather than

> x as i64

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: webdriver: fix lossy i64 cast

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 9005829 [details] [diff] [review]
webdriver: fix lossy i64 cast

Review of attachment 9005829 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/webdriver/src/command.rs
@@ +578,3 @@
>      let value = match opt {
>          Some(n) => {
> +            if n < i64::from(i32::min_value()) || n > i64::from(i32::max_value()) {

Mind to explain me when this can be lossy? I can agree with i64->i32, but what's broken for i32->i64?

Comment 3

[Andreas Tolfsen ❲:ato❳]

The warning from clippy is this:

> warning: casting i32 to i64 may become silently lossy if types change
>    --> testing/webdriver/src/command.rs:576:20
>     |
> 576 |             if n < i32::min_value() as i64 || n > i32::max_value() as i64 {
>     |                    ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i64::from(i32::min_value())`
>     |
>     = note: #[warn(clippy::cast_lossless)] on by default
>     = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#clippy::cast_lossless

I think my commit message is overstating what it’s fixing: it’s in
fact not fixing a lossy cast, but a potential future accident if
the type itself were to change.

Functionally speaking, I think the patch is equivalent.

Comment 4

[Andreas Tolfsen ❲:ato❳]

I think we should probably have one bug to resolve all the outstanding
clippy lint violations for geckodriver and its in-tree dependencies.