From 1f74c4b3597d9062f3e6d8f908758598b3f904db Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 7 Jun 2015 12:03:56 +0200 Subject: [PATCH 1/2] removed false positives from inline_always (issue #84) --- src/attrs.rs | 71 ++++++++++++++++++++++++++++++++----- tests/compile-fail/attrs.rs | 27 ++++++++++++-- 2 files changed, 87 insertions(+), 11 deletions(-) diff --git a/src/attrs.rs b/src/attrs.rs index 5ee8745c33e..d5a56b1547f 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -6,7 +6,7 @@ use syntax::ast::*; use syntax::ptr::P; use syntax::codemap::{Span, ExpnInfo}; use syntax::parse::token::InternedString; -use utils::in_macro; +use utils::{in_macro, match_path}; declare_lint! { pub INLINE_ALWAYS, Warn, "#[inline(always)] is usually a bad idea."} @@ -21,18 +21,73 @@ impl LintPass for AttrPass { } fn check_item(&mut self, cx: &Context, item: &Item) { - cx.sess().codemap().with_expn_info(item.span.expn_id, - |info| check_attrs(cx, info, &item.ident, &item.attrs)) + if is_relevant_item(item) { + cx.sess().codemap().with_expn_info(item.span.expn_id, + |info| check_attrs(cx, info, &item.ident, &item.attrs)) + } } - fn check_impl_item(&mut self, cx: &Context, item: &ImplItem) { - cx.sess().codemap().with_expn_info(item.span.expn_id, - |info| check_attrs(cx, info, &item.ident, &item.attrs)) + fn check_impl_item(&mut self, cx: &Context, item: &ImplItem) { + if is_relevant_impl(item) { + cx.sess().codemap().with_expn_info(item.span.expn_id, + |info| check_attrs(cx, info, &item.ident, &item.attrs)) + } } fn check_trait_item(&mut self, cx: &Context, item: &TraitItem) { - cx.sess().codemap().with_expn_info(item.span.expn_id, - |info| check_attrs(cx, info, &item.ident, &item.attrs)) + if is_relevant_trait(item) { + cx.sess().codemap().with_expn_info(item.span.expn_id, + |info| check_attrs(cx, info, &item.ident, &item.attrs)) + } + } +} + +fn is_relevant_item(item: &Item) -> bool { + if let &ItemFn(_, _, _, _, _, ref block) = &item.node { + is_relevant_block(block) + } else { false } +} + +fn is_relevant_impl(item: &ImplItem) -> bool { + match item.node { + MethodImplItem(_, ref block) => is_relevant_block(block), + _ => false + } +} + +fn is_relevant_trait(item: &TraitItem) -> bool { + match item.node { + MethodTraitItem(_, None) => true, + MethodTraitItem(_, Some(ref block)) => is_relevant_block(block), + _ => false + } +} + +fn is_relevant_block(block: &Block) -> bool { + for stmt in block.stmts.iter() { + match stmt.node { + StmtDecl(_, _) => return true, + StmtExpr(ref expr, _) | StmtSemi(ref expr, _) => { + return is_relevant_expr(expr); + } + _ => () + } + } + block.expr.as_ref().map_or(false, |e| is_relevant_expr(&*e)) +} + +fn is_relevant_expr(expr: &Expr) -> bool { + match expr.node { + ExprBlock(ref block) => is_relevant_block(block), + ExprRet(Some(ref e)) | ExprParen(ref e) => + is_relevant_expr(&*e), + ExprRet(None) | ExprBreak(_) | ExprMac(_) => false, + ExprCall(ref path_expr, _) => { + if let ExprPath(_, ref path) = path_expr.node { + !match_path(path, &["std", "rt", "begin_unwind"]) + } else { true } + } + _ => true } } diff --git a/tests/compile-fail/attrs.rs b/tests/compile-fail/attrs.rs index 30ce191d3db..9b648a517e0 100644 --- a/tests/compile-fail/attrs.rs +++ b/tests/compile-fail/attrs.rs @@ -1,12 +1,33 @@ #![feature(plugin)] #![plugin(clippy)] -#[deny(inline_always)] +#![deny(inline_always)] + #[inline(always)] //~ERROR You have declared #[inline(always)] on test_attr_lint. fn test_attr_lint() { assert!(true) } -fn main() { - test_attr_lint() +#[inline(always)] +fn false_positive_expr() { + unreachable!() +} + +#[inline(always)] +fn false_positive_stmt() { + unreachable!(); +} + +#[inline(always)] +fn empty_and_false_positive_stmt() { + ; + unreachable!(); +} + + +fn main() { + test_attr_lint(); + if false { false_positive_expr() } + if false { false_positive_stmt() } + if false { empty_and_false_positive_stmt() } } From 19e718966d0d3660df374f60022f219f5bf48469 Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 7 Jun 2015 12:05:14 +0200 Subject: [PATCH 2/2] forgot to update utils, there are a few new s --- src/utils.rs | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index c67ded3d93d..b99ed45b3c3 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,12 +1,42 @@ use rustc::lint::Context; -use syntax::codemap::ExpnInfo; +use syntax::ast::{DefId, Name, Path}; +use syntax::codemap::{ExpnInfo, Span}; +use rustc::middle::ty; +/// returns true if the macro that expanded the crate was outside of +/// the current crate or was a compiler plugin pub fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { + // no ExpnInfo = no macro opt_info.map_or(false, |info| { + // no span for the callee = external macro info.callee.span.map_or(true, |span| { + // no snippet = external macro or compiler-builtin expansion cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| + // macro doesn't start with "macro_rules" + // = compiler plugin !code.starts_with("macro_rules") ) }) }) } + +/// invokes in_macro with the expansion info of the given span +pub fn in_external_macro(cx: &Context, span: Span) -> bool { + cx.sess().codemap().with_expn_info(span.expn_id, + |info| in_macro(cx, info)) +} + +/// check if a DefId's path matches the given absolute type path +/// usage e.g. with +/// `match_def_path(cx, id, &["core", "option", "Option"])` +pub fn match_def_path(cx: &Context, def_id: DefId, path: &[&str]) -> bool { + ty::with_path(cx.tcx, def_id, |iter| iter.map(|elem| elem.name()) + .zip(path.iter()).all(|(nm, p)| &nm.as_str() == p)) +} + +/// match a Path against a slice of segment string literals, e.g. +/// `match_path(path, &["std", "rt", "begin_unwind"])` +pub fn match_path(path: &Path, segments: &[&str]) -> bool { + path.segments.iter().rev().zip(segments.iter().rev()).all( + |(a,b)| a.identifier.as_str() == *b) +}