Don't lint nan_cmp and zero_ptr in constants

This commit is contained in:
Oliver Schneider 2017-03-07 12:58:07 +01:00
parent e5fd3c7f94
commit 40d50fe8b2
5 changed files with 110 additions and 80 deletions

View file

@ -445,6 +445,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
misc::REDUNDANT_PATTERN,
misc::SHORT_CIRCUIT_STATEMENT,
misc::TOPLEVEL_REF_ARG,
misc::ZERO_PTR,
misc_early::BUILTIN_TYPE_SHADOW,
misc_early::DOUBLE_NEG,
misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
@ -452,7 +453,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
misc_early::REDUNDANT_CLOSURE_CALL,
misc_early::UNNEEDED_FIELD_PATTERN,
misc_early::ZERO_PREFIXED_LITERAL,
misc_early::ZERO_PTR,
mut_reference::UNNECESSARY_MUT_PASSED,
mutex_atomic::MUTEX_ATOMIC,
needless_bool::BOOL_COMPARISON,

View file

@ -8,8 +8,9 @@ use rustc_const_eval::ConstContext;
use rustc_const_math::ConstFloat;
use syntax::codemap::{Span, Spanned, ExpnFormat};
use utils::{get_item_name, get_parent_expr, implements_trait, in_macro, is_integer_literal, match_path, snippet,
span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment, iter_input_pats};
span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment, iter_input_pats, in_constant};
use utils::sugg::Sugg;
use syntax::ast::LitKind;
/// **What it does:** Checks for function arguments and let bindings denoted as `ref`.
///
@ -172,6 +173,24 @@ declare_lint! {
"using a short circuit boolean condition as a statement"
}
/// **What it does:** Catch casts from `0` to some pointer type
///
/// **Why is this bad?** This generally means `null` and is better expressed as
/// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// 0 as *const u32
/// ```
declare_lint! {
pub ZERO_PTR,
Warn,
"using 0 as *{const, mut} T"
}
#[derive(Copy, Clone)]
pub struct Pass;
@ -184,7 +203,8 @@ impl LintPass for Pass {
MODULO_ONE,
REDUNDANT_PATTERN,
USED_UNDERSCORE_BINDING,
SHORT_CIRCUIT_STATEMENT)
SHORT_CIRCUIT_STATEMENT,
ZERO_PTR)
}
}
@ -263,41 +283,48 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprBinary(ref cmp, ref left, ref right) = expr.node {
let op = cmp.node;
if op.is_comparison() {
if let ExprPath(QPath::Resolved(_, ref path)) = left.node {
check_nan(cx, path, expr.span);
match expr.node {
ExprCast(ref e, ref ty) => {
check_cast(cx, expr.span, e, ty);
return;
},
ExprBinary(ref cmp, ref left, ref right) => {
let op = cmp.node;
if op.is_comparison() {
if let ExprPath(QPath::Resolved(_, ref path)) = left.node {
check_nan(cx, path, expr);
}
if let ExprPath(QPath::Resolved(_, ref path)) = right.node {
check_nan(cx, path, expr);
}
check_to_owned(cx, left, right, true, cmp.span);
check_to_owned(cx, right, left, false, cmp.span)
}
if let ExprPath(QPath::Resolved(_, ref path)) = right.node {
check_nan(cx, path, expr.span);
}
check_to_owned(cx, left, right, true, cmp.span);
check_to_owned(cx, right, left, false, cmp.span)
}
if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
if is_allowed(cx, left) || is_allowed(cx, right) {
return;
}
if let Some(name) = get_item_name(cx, expr) {
let name = &*name.as_str();
if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") ||
name.ends_with("_eq") {
if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
if is_allowed(cx, left) || is_allowed(cx, right) {
return;
}
}
span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| {
let lhs = Sugg::hir(cx, left, "..");
let rhs = Sugg::hir(cx, right, "..");
if let Some(name) = get_item_name(cx, expr) {
let name = &*name.as_str();
if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") ||
name.ends_with("_eq") {
return;
}
}
span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| {
let lhs = Sugg::hir(cx, left, "..");
let rhs = Sugg::hir(cx, right, "..");
db.span_suggestion(expr.span,
"consider comparing them within some error",
format!("({}).abs() < error", lhs - rhs));
db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
});
} else if op == BiRem && is_integer_literal(right, 1) {
span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
}
db.span_suggestion(expr.span,
"consider comparing them within some error",
format!("({}).abs() < error", lhs - rhs));
db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
});
} else if op == BiRem && is_integer_literal(right, 1) {
span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
}
},
_ => {}
}
if in_attributes_expansion(cx, expr) {
// Don't lint things expanded by #[derive(...)], etc
@ -349,13 +376,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}
}
fn check_nan(cx: &LateContext, path: &Path, span: Span) {
path.segments.last().map(|seg| if &*seg.name.as_str() == "NAN" {
span_lint(cx,
CMP_NAN,
span,
"doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
});
fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) {
if !in_constant(cx, expr.id) {
path.segments.last().map(|seg| if &*seg.name.as_str() == "NAN" {
span_lint(cx,
CMP_NAN,
expr.span,
"doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
});
}
}
fn is_allowed(cx: &LateContext, expr: &Expr) -> bool {
@ -489,3 +518,19 @@ fn non_macro_local(cx: &LateContext, def: &def::Def) -> bool {
_ => false,
}
}
fn check_cast(cx: &LateContext, span: Span, e: &Expr, ty: &Ty) {
if_let_chain! {[
let TyPtr(MutTy { mutbl, .. }) = ty.node,
let ExprLit(ref lit) = e.node,
let LitKind::Int(value, ..) = lit.node,
value == 0,
!in_constant(cx, e.id)
], {
let msg = match mutbl {
Mutability::MutMutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`",
Mutability::MutImmutable => "`0 as *const _` detected. Consider using `ptr::null()`",
};
span_lint(cx, ZERO_PTR, span, msg);
}}
}

View file

@ -162,24 +162,6 @@ declare_lint! {
"shadowing a builtin type"
}
/// **What it does:** Catch casts from `0` to some pointer type
///
/// **Why is this bad?** This generally means `null` and is better expressed as
/// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// 0 as *const u32
/// ```
declare_lint! {
pub ZERO_PTR,
Warn,
"using 0 as *{const, mut} T"
}
#[derive(Copy, Clone)]
pub struct MiscEarly;
@ -192,8 +174,7 @@ impl LintPass for MiscEarly {
MIXED_CASE_HEX_LITERALS,
UNSEPARATED_LITERAL_SUFFIX,
ZERO_PREFIXED_LITERAL,
BUILTIN_TYPE_SHADOW,
ZERO_PTR)
BUILTIN_TYPE_SHADOW)
}
}
@ -381,9 +362,6 @@ impl EarlyLintPass for MiscEarly {
}
}}
},
ExprKind::Cast(ref e, ref ty) => {
check_cast(cx, expr.span, e, ty);
},
_ => (),
}
}
@ -412,18 +390,3 @@ impl EarlyLintPass for MiscEarly {
}
}
}
fn check_cast(cx: &EarlyContext, span: Span, e: &Expr, ty: &Ty) {
if_let_chain! {[
let TyKind::Ptr(MutTy { mutbl, .. }) = ty.node,
let ExprKind::Lit(ref lit) = e.node,
let LitKind::Int(value, ..) = lit.node,
value == 0
], {
let msg = match mutbl {
Mutability::Mutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`",
Mutability::Immutable => "`0 as *const _` detected. Consider using `ptr::null()`",
};
span_lint(cx, ZERO_PTR, span, msg);
}}
}

View file

@ -10,6 +10,7 @@ use rustc::traits;
use rustc::ty::subst::Subst;
use rustc::ty;
use rustc::ty::layout::TargetDataLayout;
use rustc::mir::transform::MirSource;
use rustc_errors;
use std::borrow::Cow;
use std::env;
@ -98,6 +99,17 @@ pub mod higher;
pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool {
rhs.expn_id != lhs.expn_id
}
pub fn in_constant(cx: &LateContext, id: NodeId) -> bool {
let parent_id = cx.tcx.hir.get_parent(id);
match MirSource::from_node(cx.tcx, parent_id) {
MirSource::Fn(_) => false,
MirSource::Const(_) |
MirSource::Static(..) |
MirSource::Promoted(..) => true,
}
}
/// Returns true if this `expn_info` was expanded by any macro.
pub fn in_macro<'a, T: LintContext<'a>>(cx: &T, span: Span) -> bool {
cx.sess().codemap().with_expn_info(span.expn_id, |info| {

View file

@ -1,12 +1,20 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(mut_mut, zero_ptr, cmp_nan)]
#![allow(dead_code)]
#[macro_use]
extern crate lazy_static;
use std::collections::HashMap;
#[deny(mut_mut)]
// ensure that we don't suggest `is_nan` and `is_null` inside constants
// FIXME: once const fn is stable, suggest these functions again in constants
const BAA: *const i32 = 0 as *const i32;
static mut BAR: *const i32 = BAA;
static mut FOO: *const i32 = 0 as *const i32;
static mut BUH: bool = 42.0 < std::f32::NAN;
#[allow(unused_variables, unused_mut)]
fn main() {
lazy_static! {
@ -19,4 +27,6 @@ fn main() {
static ref MUT_COUNT : usize = MUT_MAP.len();
}
assert!(*MUT_COUNT == 1);
// FIXME: don't lint in array length, requires `check_body`
//let _ = [""; (42.0 < std::f32::NAN) as usize];
}