Re-enable cast_lossless lint by default

[Shnatsel]

The cast_lossless lint has been made pedantic following #4528, with the reasoning being "It will only become a problem if the code changes in the future".

I am a happy user of this lint, and I find that converting always-lossless casts to from() calls as the lint suggests clarifies the code a great deal: it makes it explicit where truncation may happen, which makes the code much easier to understand.

As such, I believe this lint is a good candidate for clippy::style category. Perhaps its description could be revised to make the readability aspect more explicit.

[Shnatsel]

Example from real code to back this up:
Before: https://github.com/fschutt/fastblur/blob/c80b518e9633605546352895fb963dde39ce40f6/src/blur.rs#L203-L213
After: https://github.com/fschutt/fastblur/blob/df2032bf50a30186aa569959e960292b3215fa8d/src/blur.rs#L203-L213

[jhpratt]

it makes it explicit where truncation may happen

That's what the "lossless" part is. Truncation can't happen with the current types. It would lint if the types changes in the future such that truncation is possible. As such, I am personally opposed to this.

[llogiq]

I don't think that enabling this lint will be very popular. At best, it improves style (but not much, and it makes the code longer, too), and at worst it is seen as a false positive.

[flip1995]

I agree with @llogiq. IMO pedantic is the most fitting group for this lint.

[sunfishcode]

A common misconception about cast_lossless seems to be that it's just about what might happen if you change your code, and naturally this makes it seem pedantic.

cast_lossless is also about what may happen if someone else changes other code. With as, your code may change from correct to buggy without you yourself changing it, and without anyone seeing a warning.

[llogiq]

if someone else changes other code

How would that play out? Changing the signature or type of a let? That would still be function-local or even scope-local, right?

[Shnatsel]

I suppose violation of correctness could happen like this:

1. Third-party library exposes a struct in version 0.5, say { data: &[u8], number: u16 }
2. Client code that uses the library contains code library_struct.number as usize - this is sound so far
3. Third-party library changes the struct in version 0.6 to { data: &[u8], number: u64 }
4. Client code upgrades the library dependency from 0.5 to 0.6

The code still compiles, so client code developers may assume the semver-incompatible changes did not affect them, but actually the code now unexpectedly truncates the value of library_struct.number. If the above code used from() instead of as, the above semver change would become explicit and the code would stop compiling.

[Shnatsel]

I'm actually not that concerned about future changes to the code breaking it - as discussed in #4528, that may not be a strong enough signal for a correctness lint. In my view from() vs as is about readability, which is why I think it should be a style lint.

Consider the following real-world code:

        for j in 0..min(blur_radius, width) {
            let bb = backbuf[ti + j]; // VERTICAL: ti + j * width
            val_r += bb[0] as isize;
            val_g += bb[1] as isize;
            val_b += bb[2] as isize;
        }
        if blur_radius > width {
            val_r += (blur_radius - height) as isize * lv[0] as isize;
            val_g += (blur_radius - height) as isize * lv[1] as isize;
            val_b += (blur_radius - height) as isize * lv[2] as isize;
        }
        // Process the left side where we need pixels from beyond the left edge
        for _ in 0..min(width, blur_radius + 1) {
            let bb = get_right(ri); ri += 1;
            val_r += bb[0] as isize - fv[0] as isize;
            val_g += bb[1] as isize - fv[1] as isize;
            val_b += bb[2] as isize - fv[2] as isize;

            frontbuf[ti] = [round(val_r as f32 * iarr) as u8,
                            round(val_g as f32 * iarr) as u8,
                            round(val_b as f32 * iarr) as u8];
            ti += 1; // VERTICAL : ti += width, same with the other areas
        }

It was producing incorrect numerical results for some inputs, and I was trying to figure out why. With as used everywhere it was very hard to tell whether truncation or approximation happens. With so much as it could be happening anywhere, and I wound need to painstakingly figure out the types involved in every as and check in the docs if the cast is in fact lossless.

Here is the same code with all as replaced with from() where possible:

        for j in 0..min(blur_radius, width) {
            let bb = backbuf[ti + j]; // VERTICAL: ti + j * width
            val_r += isize::from(bb[0]);
            val_g += isize::from(bb[1]);
            val_b += isize::from(bb[2]);
        }
        if blur_radius > width {
            val_r += (blur_radius - height) as isize * isize::from(lv[0]);
            val_g += (blur_radius - height) as isize * isize::from(lv[1]);
            val_b += (blur_radius - height) as isize * isize::from(lv[2]);
        }


        // Process the left side where we need pixels from beyond the left edge
        for _ in 0..min(width, blur_radius + 1) {
            let bb = get_right(ri); ri += 1;
            val_r += isize::from(bb[0]) - isize::from(fv[0]);
            val_g += isize::from(bb[1]) - isize::from(fv[1]);
            val_b += isize::from(bb[2]) - isize::from(fv[2]);

            frontbuf[ti] = [round(val_r as f32 * iarr) as u8,
                            round(val_g as f32 * iarr) as u8,
                            round(val_b as f32 * iarr) as u8];
            ti += 1; // VERTICAL : ti += width, same with the other areas
        }

Now I don't have to worry about losing data in any of the from() calls, and it makes it explicit where data loss could happen, so I know which parts to look at in case of issues.

[memoryruins]

Although I am more concerned about the fragility of external changes, I agree that it helps readability and that style would be more accurate than pedantic.

[smmalis37]

+1 for promoting to style

[camsteffen]

I'd be in favor of style. When I see from I skim over it quickly because I know it's always safe. With as, I have to slow down and make sure it works as intended. If the type of the casted expression isn't plaininly obvious, I'll have to figure that out too.

[llogiq]

Note that casting bools to integers is also a form of cast_lossless.

[blyxyas]

Closing as this has become stale (1066 days since last comment) and the PR was closed.