Skip to content

Commit 6bc7893

Browse files
oli-obkoli-obk
authored
Merge pull request #1956 from sunfishcode/master
Add a lint for lossless casts.
2 parents 5f1f12f + 7714203 commit 6bc7893

File tree

8 files changed

+360
-100
lines changed

8 files changed

+360
-100
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ All notable changes to this project will be documented in this file.
426426
[`box_vec`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#box_vec
427427
[`boxed_local`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#boxed_local
428428
[`builtin_type_shadow`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#builtin_type_shadow
429+
[`cast_lossless`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_lossless
429430
[`cast_possible_truncation`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_possible_truncation
430431
[`cast_possible_wrap`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_possible_wrap
431432
[`cast_precision_loss`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_precision_loss

‎README.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ transparently:
180180

181181
## Lints
182182

183-
There are 208 lints included in this crate:
183+
There are 209 lints included in this crate:
184184

185185
name | default | triggers on
186186
-----------------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -198,6 +198,7 @@ name
198198
[box_vec](https://github.com/rust-lang-nursery/rust-clippy/wiki#box_vec) | warn | usage of `Box<Vec<T>>`, vector elements are already on the heap
199199
[boxed_local](https://github.com/rust-lang-nursery/rust-clippy/wiki#boxed_local) | warn | using `Box<T>` where unnecessary
200200
[builtin_type_shadow](https://github.com/rust-lang-nursery/rust-clippy/wiki#builtin_type_shadow) | warn | shadowing a builtin type
201+
[cast_lossless](https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_lossless) | allow | casts using `as` that are known to be lossless, e.g. `x as u64` where `x: u8`
201202
[cast_possible_truncation](https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_possible_truncation) | allow | casts that may cause truncation of the value, e.g. `x as u8` where `x: u32`, or `x as i32` where `x: f32`
202203
[cast_possible_wrap](https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g. `x as i32` where `x: u32` and `x > i32::MAX`
203204
[cast_precision_loss](https://github.com/rust-lang-nursery/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g. `x as f32` where `x: u64`

‎clippy_lints/src/enum_clike.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnportableVariant {
5454
let bad = match cx.tcx.at(expr.span).const_eval(
5555
param_env.and((did, substs)),
5656
) {
57-
Ok(ConstVal::Integral(Usize(Us64(i)))) => i as u32 as u64 != i,
58-
Ok(ConstVal::Integral(Isize(Is64(i)))) => i as i32 as i64 != i,
57+
Ok(ConstVal::Integral(Usize(Us64(i)))) => u64::from(i as u32) != i,
58+
Ok(ConstVal::Integral(Isize(Is64(i)))) => i64::from(i as i32) != i,
5959
_ => false,
6060
};
6161
if bad {

‎clippy_lints/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
358358
shadow::SHADOW_UNRELATED,
359359
strings::STRING_ADD,
360360
strings::STRING_ADD_ASSIGN,
361+
types::CAST_LOSSLESS,
361362
types::CAST_POSSIBLE_TRUNCATION,
362363
types::CAST_POSSIBLE_WRAP,
363364
types::CAST_PRECISION_LOSS,

‎clippy_lints/src/misc.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -430,17 +430,17 @@ fn is_allowed(cx: &LateContext, expr: &Expr) -> bool {
430430
FloatTy::F32 => {
431431
let zero = ConstFloat {
432432
ty: FloatTy::F32,
433-
bits: 0.0_f32.to_bits() as u128,
433+
bits: u128::from(0.0_f32.to_bits()),
434434
};
435435

436436
let infinity = ConstFloat {
437437
ty: FloatTy::F32,
438-
bits: ::std::f32::INFINITY.to_bits() as u128,
438+
bits: u128::from(::std::f32::INFINITY.to_bits()),
439439
};
440440

441441
let neg_infinity = ConstFloat {
442442
ty: FloatTy::F32,
443-
bits: ::std::f32::NEG_INFINITY.to_bits() as u128,
443+
bits: u128::from(::std::f32::NEG_INFINITY.to_bits()),
444444
};
445445

446446
val.try_cmp(zero) == Ok(Ordering::Equal) || val.try_cmp(infinity) == Ok(Ordering::Equal) ||
@@ -449,17 +449,17 @@ fn is_allowed(cx: &LateContext, expr: &Expr) -> bool {
449449
FloatTy::F64 => {
450450
let zero = ConstFloat {
451451
ty: FloatTy::F64,
452-
bits: 0.0_f64.to_bits() as u128,
452+
bits: u128::from(0.0_f64.to_bits()),
453453
};
454454

455455
let infinity = ConstFloat {
456456
ty: FloatTy::F64,
457-
bits: ::std::f64::INFINITY.to_bits() as u128,
457+
bits: u128::from(::std::f64::INFINITY.to_bits()),
458458
};
459459

460460
let neg_infinity = ConstFloat {
461461
ty: FloatTy::F64,
462-
bits: ::std::f64::NEG_INFINITY.to_bits() as u128,
462+
bits: u128::from(::std::f64::NEG_INFINITY.to_bits()),
463463
};
464464

465465
val.try_cmp(zero) == Ok(Ordering::Equal) || val.try_cmp(infinity) == Ok(Ordering::Equal) ||

‎clippy_lints/src/types.rs

+60-8
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,28 @@ declare_lint! {
481481
and `x > i32::MAX`"
482482
}
483483

484+
/// **What it does:** Checks for on casts between numerical types that may
485+
/// be replaced by safe conversion functions.
486+
///
487+
/// **Why is this bad?** Rust's `as` keyword will perform many kinds of
488+
/// conversions, including silently lossy conversions. Conversion functions such
489+
/// as `i32::from` will only perform lossless conversions. Using the conversion
490+
/// functions prevents conversions from turning into silent lossy conversions if
491+
/// the types of the input expressions ever change, and make it easier for
492+
/// people reading the code to know that the conversion is lossless.
493+
///
494+
/// **Known problems:** None.
495+
///
496+
/// **Example:**
497+
/// ```rust
498+
/// fn as_u64(x: u8) -> u64 { x as u64 }
499+
/// ```
500+
declare_lint! {
501+
pub CAST_LOSSLESS,
502+
Allow,
503+
"casts using `as` that are known to be lossless, e.g. `x as u64` where `x: u8`"
504+
}
505+
484506
/// **What it does:** Checks for casts to the same type.
485507
///
486508
/// **Why is this bad?** It's just unnecessary.
@@ -560,6 +582,17 @@ fn span_precision_loss_lint(cx: &LateContext, expr: &Expr, cast_from: Ty, cast_t
560582
);
561583
}
562584

585+
fn span_lossless_lint(cx: &LateContext, expr: &Expr, op: &Expr, cast_from: Ty, cast_to: Ty) {
586+
span_lint_and_sugg(cx,
587+
CAST_LOSSLESS,
588+
expr.span,
589+
&format!("casting {} to {} may become silently lossy if types change",
590+
cast_from,
591+
cast_to),
592+
"try",
593+
format!("{}::from({})", cast_to, &snippet(cx, op.span, "..")));
594+
}
595+
563596
enum ArchSuffix {
564597
_32,
565598
_64,
@@ -643,13 +676,24 @@ fn check_truncation_and_wrapping(cx: &LateContext, expr: &Expr, cast_from: Ty, c
643676
}
644677
}
645678

679+
fn check_lossless(cx: &LateContext, expr: &Expr, op: &Expr, cast_from: Ty, cast_to: Ty) {
680+
let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed();
681+
let from_nbits = int_ty_to_nbits(cast_from, cx.tcx);
682+
let to_nbits = int_ty_to_nbits(cast_to, cx.tcx);
683+
if !is_isize_or_usize(cast_from) && !is_isize_or_usize(cast_to) &&
684+
from_nbits < to_nbits && !cast_signed_to_unsigned {
685+
span_lossless_lint(cx, expr, op, cast_from, cast_to);
686+
}
687+
}
688+
646689
impl LintPass for CastPass {
647690
fn get_lints(&self) -> LintArray {
648691
lint_array!(
649692
CAST_PRECISION_LOSS,
650693
CAST_SIGN_LOSS,
651694
CAST_POSSIBLE_TRUNCATION,
652695
CAST_POSSIBLE_WRAP,
696+
CAST_LOSSLESS,
653697
UNNECESSARY_CAST
654698
)
655699
}
@@ -688,6 +732,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass {
688732
if is_isize_or_usize(cast_from) || from_nbits >= to_nbits {
689733
span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64);
690734
}
735+
if from_nbits < to_nbits {
736+
span_lossless_lint(cx, expr, ex, cast_from, cast_to);
737+
}
691738
},
692739
(false, true) => {
693740
span_lint(
@@ -715,6 +762,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass {
715762
);
716763
}
717764
check_truncation_and_wrapping(cx, expr, cast_from, cast_to);
765+
check_lossless(cx, expr, ex, cast_from, cast_to);
718766
},
719767
(false, false) => {
720768
if let (&ty::TyFloat(FloatTy::F64), &ty::TyFloat(FloatTy::F32)) =
@@ -727,6 +775,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass {
727775
"casting f64 to f32 may truncate the value",
728776
);
729777
}
778+
if let (&ty::TyFloat(FloatTy::F32), &ty::TyFloat(FloatTy::F64)) =
779+
(&cast_from.sty, &cast_to.sty) {
780+
span_lossless_lint(cx, expr, ex, cast_from, cast_to);
781+
}
730782
},
731783
}
732784
}
@@ -1233,20 +1285,20 @@ fn numeric_cast_precast_bounds<'a>(cx: &LateContext, expr: &'a Expr) -> Option<(
12331285
match pre_cast_ty.sty {
12341286
ty::TyInt(int_ty) => {
12351287
Some(match int_ty {
1236-
IntTy::I8 => (FullInt::S(i8::min_value() as i128), FullInt::S(i8::max_value() as i128)),
1237-
IntTy::I16 => (FullInt::S(i16::min_value() as i128), FullInt::S(i16::max_value() as i128)),
1238-
IntTy::I32 => (FullInt::S(i32::min_value() as i128), FullInt::S(i32::max_value() as i128)),
1239-
IntTy::I64 => (FullInt::S(i64::min_value() as i128), FullInt::S(i64::max_value() as i128)),
1288+
IntTy::I8 => (FullInt::S(i128::from(i8::min_value())), FullInt::S(i128::from(i8::max_value()))),
1289+
IntTy::I16 => (FullInt::S(i128::from(i16::min_value())), FullInt::S(i128::from(i16::max_value()))),
1290+
IntTy::I32 => (FullInt::S(i128::from(i32::min_value())), FullInt::S(i128::from(i32::max_value()))),
1291+
IntTy::I64 => (FullInt::S(i128::from(i64::min_value())), FullInt::S(i128::from(i64::max_value()))),
12401292
IntTy::I128 => (FullInt::S(i128::min_value() as i128), FullInt::S(i128::max_value() as i128)),
12411293
IntTy::Is => (FullInt::S(isize::min_value() as i128), FullInt::S(isize::max_value() as i128)),
12421294
})
12431295
},
12441296
ty::TyUint(uint_ty) => {
12451297
Some(match uint_ty {
1246-
UintTy::U8 => (FullInt::U(u8::min_value() as u128), FullInt::U(u8::max_value() as u128)),
1247-
UintTy::U16 => (FullInt::U(u16::min_value() as u128), FullInt::U(u16::max_value() as u128)),
1248-
UintTy::U32 => (FullInt::U(u32::min_value() as u128), FullInt::U(u32::max_value() as u128)),
1249-
UintTy::U64 => (FullInt::U(u64::min_value() as u128), FullInt::U(u64::max_value() as u128)),
1298+
UintTy::U8 => (FullInt::U(u128::from(u8::min_value())), FullInt::U(u128::from(u8::max_value()))),
1299+
UintTy::U16 => (FullInt::U(u128::from(u16::min_value())), FullInt::U(u128::from(u16::max_value()))),
1300+
UintTy::U32 => (FullInt::U(u128::from(u32::min_value())), FullInt::U(u128::from(u32::max_value()))),
1301+
UintTy::U64 => (FullInt::U(u128::from(u64::min_value())), FullInt::U(u128::from(u64::max_value()))),
12501302
UintTy::U128 => (FullInt::U(u128::min_value() as u128), FullInt::U(u128::max_value() as u128)),
12511303
UintTy::Us => (FullInt::U(usize::min_value() as u128), FullInt::U(usize::max_value() as u128)),
12521304
})

‎tests/ui/cast.rs

+33-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![feature(plugin)]
22
#![plugin(clippy)]
33

4-
#[warn(cast_precision_loss, cast_possible_truncation, cast_sign_loss, cast_possible_wrap)]
4+
#[warn(cast_precision_loss, cast_possible_truncation, cast_sign_loss, cast_possible_wrap, cast_lossless)]
55
#[allow(no_effect, unnecessary_operation)]
66
fn main() {
77
// Test cast_precision_loss
@@ -11,8 +11,6 @@ fn main() {
1111
1u32 as f32;
1212
1u64 as f32;
1313
1u64 as f64;
14-
1i32 as f64; // Should not trigger the lint
15-
1u32 as f64; // Should not trigger the lint
1614
// Test cast_possible_truncation
1715
1f32 as i32;
1816
1f32 as u32;
@@ -27,6 +25,38 @@ fn main() {
2725
1u32 as i32;
2826
1u64 as i64;
2927
1usize as isize;
28+
// Test cast_lossless with casts to integer types
29+
1i8 as i16;
30+
1i8 as i32;
31+
1i8 as i64;
32+
1u8 as i16;
33+
1u8 as i32;
34+
1u8 as i64;
35+
1u8 as u16;
36+
1u8 as u32;
37+
1u8 as u64;
38+
1i16 as i32;
39+
1i16 as i64;
40+
1u16 as i32;
41+
1u16 as i64;
42+
1u16 as u32;
43+
1u16 as u64;
44+
1i32 as i64;
45+
1u32 as i64;
46+
1u32 as u64;
47+
// Test cast_lossless with casts to floating-point types
48+
1i8 as f32;
49+
1i8 as f64;
50+
1u8 as f32;
51+
1u8 as f64;
52+
1i16 as f32;
53+
1i16 as f64;
54+
1u16 as f32;
55+
1u16 as f64;
56+
1i32 as f64;
57+
1u32 as f64;
58+
// Test cast_lossless with casts from floating-point types
59+
1.0f32 as f64;
3060
// Test cast_sign_loss
3161
1i32 as u32;
3262
1isize as usize;
@@ -56,7 +86,6 @@ fn main() {
5686
false as bool;
5787
&1i32 as &i32;
5888
// Should not trigger
59-
1i32 as i64;
6089
let v = vec!(1);
6190
&v as &[i32];
6291
1.0 as f64;

0 commit comments

Comments
 (0)