Merge pull request #1610 from Manishearth/no_const_warnings
Don't lint `nan_cmp` and `zero_ptr` in constants
This commit is contained in:
commit
b48243c08c
@ -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,
|
||||
|
@ -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);
|
||||
}}
|
||||
}
|
||||
|
@ -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);
|
||||
}}
|
||||
}
|
||||
|
@ -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| {
|
||||
|
@ -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];
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user