Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lossy type conversion via 'as' #114

Closed
llogiq opened this Issue Jul 14, 2015 · 16 comments

Comments

Projects
None yet
5 participants
@llogiq
Copy link
Collaborator

llogiq commented Jul 14, 2015

E.g. x as u8 where x is u32. For isize/usize we could even have a variant that warns of lossy conversions when compiled on 32-bit/16-bit systems. Suggested on internals.

I'd suggest we make this Allow by default. We may also want to split this into 3 "lossy conversion" lints:

  • possible overflow (e.g. u32 as u8)
  • loss of sign (e.g. i32 as u64)
  • loss of precision (e.g. i32 as f32)
@tbu-

This comment has been minimized.

Copy link

tbu- commented Aug 11, 2015

How should one perform these lossy conversions?

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Aug 11, 2015

We don't perform the conversions, we lint for them. So if we find an expression which cases a uX to a uY where X>Y, we lint. Et cetera.

@tbu-

This comment has been minimized.

Copy link

tbu- commented Aug 12, 2015

@Manishearth I was just wondering what one should do to avert this lint? #[allow] it?

@llogiq

This comment has been minimized.

Copy link
Collaborator Author

llogiq commented Aug 12, 2015

@tbu- Since all of them are #[allow] by default, you don't need to do anything. But if you want to ensure that you're not losing anything, you could #[warn] on an item or #![warn] on a module. Or even #[deny], if you want to be extra safe.

This is not about being opinionated, this is about being helpful when explicitly asked for.

@tbu-

This comment has been minimized.

Copy link

tbu- commented Aug 12, 2015

I understand this, but suppose you have it enabled, like #[warn]. Then sometimes you're going to need a lossy conversion. How should you annotate that you know what you're doing? I think there's currently no function in the standard library that explicitely says "I know that I'm performing a lossy conversion" – should you #[allow] where you're sure that you need a lossy conversion?

@llogiq

This comment has been minimized.

Copy link
Collaborator Author

llogiq commented Aug 12, 2015

If you know you need a lossy conversion, why would you #[warn] yourself about it in the first place?

The only point of #[allow] would be as an override for specific items given a module-level #![warn] when it's shorter than adding #[warn] on all other items.

I work in statistical data analysis, and having a lint to show me the places where I must be wary of overflow would be quite useful to me.

If we wanted to get extra fancy, we could add some guards to find assert!(..) or debug_assert!(..) that ensure the value is always in range and not report in those cases.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Aug 12, 2015

It's a lint, so it's supposed to be overridden with #[allow], though this particular one is allow by default anyway 😄

@Robzz

This comment has been minimized.

Copy link
Contributor

Robzz commented Aug 17, 2015

I'm currently (painfully) getting to know the compiler internals and working on this.
@Manishearth i'm the reddit guy who told you he'd work on this a few days ago. Does your offer to give me a hand still hold? :)

@llogiq

This comment has been minimized.

Copy link
Collaborator Author

llogiq commented Aug 17, 2015

Both Manish and I will be glad to help.

Your starting point will be matching on ExprCast.

@Robzz

This comment has been minimized.

Copy link
Contributor

Robzz commented Aug 17, 2015

Thank you very much. :) I did do that (all by myself! (woohoo!)), so at least I'm going somewhere.
I think I have something that works when converting from ints (resp. floats) to floats (resp. ints), and from signed (resp. unsigned) to unsigned (resp. signed)
The part with types of different sizes, I'm still figuring out what rustc provides to help me.

In the meantime, I forked the repo and pushed what I have. I would really appreciate it if you took a look and gave me some feedback.
edit : by the way, the dogfood script reports a bunch of errors, some of them justified, some others, well... Some others just leave me staring at my screen in incomprehension. For instance :

src/consts.rs:14:10: 14:19 error: loss of precision : casting from unsigned type to signed type [-D cast-precision-loss]
src/consts.rs:14 #[derive(PartialEq, Eq, Debug, Copy, Clone)]
                          ^~~~~~~~~

edit2 : Oh. I rustc -Z unstable-options --pretty expandeded the file in question, and it turns out the derived PartialEq implementation performs casts, so that must be it.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Aug 18, 2015

Try using in_external_macro to ignore things that come from a macro (see how we use it elsewhere in the repo)

Otherwise you seem to be going down the right path! I'm not sure what @llogiq intended for the third precision-loss lint, though. (But it looks like you'll need to match on the actual type variants)

@llogiq

This comment has been minimized.

Copy link
Collaborator Author

llogiq commented Aug 18, 2015

It appears I'll have to explain my reasoning a bit more: Let X and Y be one of { 8, 16, 32, 64 }. Then we get:

X _ Y cast from into lint
X>Y ix iy cast_possible_overflow
X>Y ux uy cast_possible_overflow
X>Y fx fy cast_possible_overflow
fx iy cast_possible_overflow
fx uy cast_possible_overflow, cast_sign_loss
ix uy cast_sign_loss
X >= Y ix fy cast_precision_loss
X >= Y ux fy cast_precision_loss

The cast_precision_loss cases are subtle: Since a f64 value uses only 52 bits to represent the actual value (using 11 for the exponent + one sign bit), it is possible to have values where the lowest bits are cut off during the transformation. Same goes for i32 as f32 casts (but not for i32 as f64).

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Aug 18, 2015

ohh, that's what you're trying to catch 😄

@llogiq

This comment has been minimized.

Copy link
Collaborator Author

llogiq commented Aug 18, 2015

As I said, I work in statistical data processing, and while we often use floating point for performance reasons, it's often interesting to see where we lose precision. Rust with its explicit conversions is well suited for the task, but local type inference means it's still possible to lose track. Therefore this lints.

Btw. this would be a good time to start the wiki...

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Aug 18, 2015

Heh. I need time, I've got a million quizzes and assignments piling on top of me now :p

Feel free to start it though, any format will do.

llogiq added a commit that referenced this issue Aug 21, 2015

Merge pull request #208 from Robzz/iss114
Implementation of lossy cast lints (issue #114)

@llogiq llogiq closed this Aug 21, 2015

@vitiral

This comment has been minimized.

Copy link

vitiral commented Aug 31, 2017

@llogiq what is the recommened function to use for explicit lossy conversions. For example:

let x: u32 = 7;
let y: u8 = is_lossy(x)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.