Skip to content

lint unportable clike enum discriminants #710

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

Merged
merged 1 commit into from
Feb 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to usage instructions](#usage)

##Lints
There are 127 lints included in this crate:
There are 128 lints included in this crate:

name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -38,6 +38,7 @@ name
[drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value
[duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore
[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected
[enum_clike_unportable_variant](https://github.com/Manishearth/rust-clippy/wiki#enum_clike_unportable_variant) | warn | finds C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32`
[enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | finds use items that import all variants of an enum
[enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names) | warn | finds enums where all variants share a prefix/postfix
[eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
Expand Down
56 changes: 56 additions & 0 deletions src/enum_clike.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
//! lint on C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32`

use rustc::lint::*;
use syntax::ast::{IntTy, UintTy};
use syntax::attr::*;
use rustc_front::hir::*;
use rustc::middle::const_eval::{ConstVal, EvalHint, eval_const_expr_partial};
use rustc::middle::ty;
use utils::span_lint;

/// **What it does:** Lints on C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32`.
///
/// **Why is this bad?** This will truncate the variant value on 32bit architectures, but works fine on 64 bit.
///
/// **Known problems:** None
///
/// **Example:** `#[repr(usize)] enum NonPortable { X = 0x1_0000_0000, Y = 0 }`
declare_lint! {
pub ENUM_CLIKE_UNPORTABLE_VARIANT, Warn,
"finds C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32`"
}

pub struct EnumClikeUnportableVariant;

impl LintPass for EnumClikeUnportableVariant {
fn get_lints(&self) -> LintArray {
lint_array!(ENUM_CLIKE_UNPORTABLE_VARIANT)
}
}

impl LateLintPass for EnumClikeUnportableVariant {
#[allow(cast_possible_truncation, cast_sign_loss)]
fn check_item(&mut self, cx: &LateContext, item: &Item) {
if let ItemEnum(ref def, _) = item.node {
for var in &def.variants {
let variant = &var.node;
if let Some(ref disr) = variant.disr_expr {
let cv = eval_const_expr_partial(cx.tcx, &**disr, EvalHint::ExprTypeChecked, None);
let bad = match (cv, &cx.tcx.expr_ty(&**disr).sty) {
(Ok(ConstVal::Int(i)), &ty::TyInt(IntTy::Is)) => i as i32 as i64 != i,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I'm not comparing against constants is explained here and in the preceding posts

(Ok(ConstVal::Uint(i)), &ty::TyInt(IntTy::Is)) => i as i32 as u64 != i,
(Ok(ConstVal::Int(i)), &ty::TyUint(UintTy::Us)) => (i < 0) || (i as u32 as i64 != i),
(Ok(ConstVal::Uint(i)), &ty::TyUint(UintTy::Us)) => i as u32 as u64 != i,
_ => false,
};
if bad {
span_lint(cx,
ENUM_CLIKE_UNPORTABLE_VARIANT,
var.span,
"Clike enum variant discriminant is not portable to 32-bit targets");
}
}
}
}
}
}
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub mod cyclomatic_complexity;
pub mod derive;
pub mod drop_ref;
pub mod entry;
pub mod enum_clike;
pub mod enum_glob_use;
pub mod enum_variants;
pub mod eq_op;
Expand Down Expand Up @@ -108,6 +109,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box eq_op::EqOp);
reg.register_early_lint_pass(box enum_variants::EnumVariantNames);
reg.register_late_lint_pass(box enum_glob_use::EnumGlobUse);
reg.register_late_lint_pass(box enum_clike::EnumClikeUnportableVariant);
reg.register_late_lint_pass(box bit_mask::BitMask);
reg.register_late_lint_pass(box ptr_arg::PtrArg);
reg.register_late_lint_pass(box needless_bool::NeedlessBool);
Expand Down Expand Up @@ -211,6 +213,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
derive::EXPL_IMPL_CLONE_ON_COPY,
drop_ref::DROP_REF,
entry::MAP_ENTRY,
enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,
enum_variants::ENUM_VARIANT_NAMES,
eq_op::EQ_OP,
escape::BOXED_LOCAL,
Expand Down
53 changes: 53 additions & 0 deletions tests/compile-fail/enums_clike.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#![feature(plugin, associated_consts)]
#![plugin(clippy)]
#![deny(clippy)]

#![allow(unused)]

#[repr(usize)]
enum NonPortable {
X = 0x1_0000_0000, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
Y = 0,
Z = 0x7FFF_FFFF,
A = 0xFFFF_FFFF,
}

enum NonPortableNoHint {
X = 0x1_0000_0000, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says this lints repr(isize/usize) enums, why is that one linted too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple, this one is inferred to be isize. Don't ask me why, I think it's a bug. https://github.com/rust-lang/rust/blob/master/src/test/run-pass/enum-discrim-autosizing.rs shows that even on 32-bit, the size is 8 bytes but the type is isize xD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related issues rust-lang/rust#24290 and UB because of it rust-lang/rust#31886

Y = 0,
Z = 0x7FFF_FFFF,
A = 0xFFFF_FFFF, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
}

#[repr(isize)]
enum NonPortableSigned {
X = -1,
Y = 0x7FFF_FFFF,
Z = 0xFFFF_FFFF, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
A = 0x1_0000_0000, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
B = std::i32::MIN as isize,
C = (std::i32::MIN as isize) - 1, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
}

enum NonPortableSignedNoHint {
X = -1,
Y = 0x7FFF_FFFF,
Z = 0xFFFF_FFFF, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
A = 0x1_0000_0000, //~ ERROR: Clike enum variant discriminant is not portable to 32-bit targets
}

/*
FIXME: uncomment once https://github.com/rust-lang/rust/issues/31910 is fixed
#[repr(usize)]
enum NonPortable2<T: Trait> {
X = Trait::Number,
Y = 0,
}

trait Trait {
const Number: usize = 0x1_0000_0000;
}
*/

fn main() {
}