From 18e81c1b59cd908bf3a1c14463533de64df74f5c Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 4 Dec 2015 15:42:53 +0530 Subject: [PATCH] Rudimentary escape analysis for Box --- README.md | 3 +- src/escape.rs | 156 ++++++++++++++++++++++++++ src/lib.rs | 3 + tests/compile-fail/escape_analysis.rs | 81 +++++++++++++ 4 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 src/escape.rs create mode 100644 tests/compile-fail/escape_analysis.rs diff --git a/README.md b/README.md index 206d86ec1ee..e19ab474c5e 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 80 lints included in this crate: +There are 81 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -15,6 +15,7 @@ name [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces can be eliminated in conditions that are expressions, e.g `if { true } ...` [block_in_if_condition_stmt](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt) | warn | avoid complex blocks in conditions, instead move the block higher and bind it with 'let'; e.g: `if { let x = true; x } ...` [box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box>`, vector elements are already on the heap +[boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using Box where unnecessary [cast_possible_truncation](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_truncation) | allow | casts that may cause truncation of the value, e.g `x as u8` where `x: u32`, or `x as i32` where `x: f32` [cast_possible_wrap](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g `x as i32` where `x: u32` and `x > i32::MAX` [cast_precision_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g `x as f32` where `x: u64` diff --git a/src/escape.rs b/src/escape.rs new file mode 100644 index 00000000000..fbd545acc96 --- /dev/null +++ b/src/escape.rs @@ -0,0 +1,156 @@ +use rustc::lint::*; +use rustc_front::hir::*; +use rustc_front::intravisit as visit; +use rustc::front::map::Node; +use rustc::middle::ty; +use rustc::middle::ty::adjustment::AutoAdjustment; +use rustc::middle::expr_use_visitor::*; +use rustc::middle::infer; +use rustc::middle::mem_categorization::{cmt, Categorization}; +use rustc::util::nodemap::NodeSet; +use syntax::ast::NodeId; +use syntax::codemap::Span; +use utils::span_lint; + +pub struct EscapePass; + +declare_lint!(pub BOXED_LOCAL, Warn, "using Box where unnecessary"); + +struct EscapeDelegate<'a, 'tcx: 'a> { + cx: &'a LateContext<'a, 'tcx>, + set: NodeSet, +} + +impl LintPass for EscapePass { + fn get_lints(&self) -> LintArray { + lint_array!(BOXED_LOCAL) + } +} + +impl LateLintPass for EscapePass { + fn check_fn(&mut self, + cx: &LateContext, + _: visit::FnKind, + decl: &FnDecl, + body: &Block, + _: Span, + id: NodeId) { + let param_env = ty::ParameterEnvironment::for_item(cx.tcx, id); + let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, Some(param_env), false); + let mut v = EscapeDelegate { + cx: cx, + set: NodeSet(), + }; + { + let mut vis = ExprUseVisitor::new(&mut v, &infcx); + vis.walk_fn(decl, body); + } + for node in v.set { + span_lint(cx, + BOXED_LOCAL, + cx.tcx.map.span(node), + "local variable doesn't need to be boxed here"); + } + } +} + +impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { + fn consume(&mut self, + _: NodeId, + _: Span, + cmt: cmt<'tcx>, + mode: ConsumeMode) { + + if let Categorization::Local(lid) = cmt.cat { + if self.set.contains(&lid) { + if let Move(DirectRefMove) = mode { + // moved out or in. clearly can't be localized + self.set.remove(&lid); + } + } + } + } + fn matched_pat(&mut self, _: &Pat, _: cmt<'tcx>, _: MatchMode) {} + fn consume_pat(&mut self, consume_pat: &Pat, cmt: cmt<'tcx>, _: ConsumeMode) { + if let Categorization::Rvalue(..) = cmt.cat { + if let Some(Node::NodeStmt(st)) = self.cx + .tcx + .map + .find(self.cx.tcx.map.get_parent_node(cmt.id)) { + if let StmtDecl(ref decl, _) = st.node { + if let DeclLocal(ref loc) = decl.node { + if let Some(ref ex) = loc.init { + if let ExprBox(..) = ex.node { + if let ty::TyBox(..) = cmt.ty.sty { + // let x = box (...) + self.set.insert(consume_pat.id); + } + // TODO Box::new + // TODO vec![] + // TODO "foo".to_owned() and friends + } + } + } + } + } + } + if let Categorization::Local(lid) = cmt.cat { + if self.set.contains(&lid) { + // let y = x where x is known + // remove x, insert y + self.set.insert(consume_pat.id); + self.set.remove(&lid); + } + } + + } + fn borrow(&mut self, + borrow_id: NodeId, + _: Span, + cmt: cmt<'tcx>, + _: ty::Region, + _: ty::BorrowKind, + loan_cause: LoanCause) { + + if let Categorization::Local(lid) = cmt.cat { + if self.set.contains(&lid) { + if let Some(&AutoAdjustment::AdjustDerefRef(adj)) = self.cx + .tcx + .tables + .borrow() + .adjustments + .get(&borrow_id) { + if LoanCause::AutoRef == loan_cause { + // x.foo() + if adj.autoderefs <= 0 { + self.set.remove(&lid); // Used without autodereffing (i.e. x.clone()) + } + } else { + self.cx.sess().span_bug(cmt.span, "Unknown adjusted AutoRef"); + } + } else if LoanCause::AddrOf == loan_cause { + // &x + if let Some(&AutoAdjustment::AdjustDerefRef(adj)) = + self.cx.tcx.tables.borrow().adjustments + .get(&self.cx.tcx.map.get_parent_node(borrow_id)) { + if adj.autoderefs <= 1 { + // foo(&x) where no extra autoreffing is happening + self.set.remove(&lid); + } + } + + } else if LoanCause::MatchDiscriminant == loan_cause { + self.set.remove(&lid); // `match x` can move + } + // do nothing for matches, etc. These can't escape + } + } + } + fn decl_without_init(&mut self, _: NodeId, _: Span) {} + fn mutate(&mut self, + _: NodeId, + _: Span, + _: cmt<'tcx>, + _: MutateMode) { + } +} diff --git a/src/lib.rs b/src/lib.rs index 57a92f99900..9ead05b93b2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -64,6 +64,7 @@ pub mod no_effect; pub mod temporary_assignment; pub mod transmute; pub mod cyclomatic_complexity; +pub mod escape; mod reexport { pub use syntax::ast::{Name, Ident, NodeId}; @@ -116,6 +117,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass); reg.register_late_lint_pass(box transmute::UselessTransmute); reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(25)); + reg.register_late_lint_pass(box escape::EscapePass); reg.register_lint_group("clippy_pedantic", vec![ methods::OPTION_UNWRAP_USED, @@ -146,6 +148,7 @@ pub fn plugin_registrar(reg: &mut Registry) { collapsible_if::COLLAPSIBLE_IF, cyclomatic_complexity::CYCLOMATIC_COMPLEXITY, eq_op::EQ_OP, + escape::BOXED_LOCAL, eta_reduction::REDUNDANT_CLOSURE, identity_op::IDENTITY_OP, len_zero::LEN_WITHOUT_IS_EMPTY, diff --git a/tests/compile-fail/escape_analysis.rs b/tests/compile-fail/escape_analysis.rs new file mode 100644 index 00000000000..3782cb96da5 --- /dev/null +++ b/tests/compile-fail/escape_analysis.rs @@ -0,0 +1,81 @@ +#![feature(plugin, box_syntax)] +#![plugin(clippy)] +#![allow(warnings, clippy)] + +#![deny(boxed_local)] + +#[derive(Clone)] +struct A; + +impl A { + fn foo(&self){} +} + +fn main() { +} + +fn warn_call() { + let x = box A; //~ ERROR local variable + x.foo(); +} + +fn warn_rename_call() { + let x = box A; + + let y = x; //~ ERROR local variable + y.foo(); // via autoderef +} + +fn warn_notuse() { + let bz = box A; //~ ERROR local variable +} + +fn warn_pass() { + let bz = box A; //~ ERROR local variable + take_ref(&bz); // via deref coercion +} + +fn nowarn_return() -> Box { + let fx = box A; + fx // moved out, "escapes" +} + +fn nowarn_move() { + let bx = box A; + drop(bx) // moved in, "escapes" +} +fn nowarn_call() { + let bx = box A; + bx.clone(); // method only available to Box, not via autoderef +} + +fn nowarn_pass() { + let bx = box A; + take_box(&bx); // fn needs &Box +} + + +fn take_box(x: &Box) {} +fn take_ref(x: &A) {} + + +fn nowarn_ref_take() { + // false positive, should actually warn + let x = box A; //~ ERROR local variable + let y = &x; + take_box(y); +} + +fn nowarn_match() { + let x = box A; // moved into a match + match x { + y => drop(y) + } +} + +fn warn_match() { + let x = box A; //~ ERROR local variable + match &x { // not moved + ref y => () + } +} \ No newline at end of file