Merge pull request #92 from Manishearth/inline_always

Remove false positives on inline_always. this closes issue #84 and adds a few utils

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
 	}
 }
 

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)
+}

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() }
 }