From c1421c6e820c147c1572b98bbcac9c05e51fd066 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 10 Jul 2016 18:53:50 +0530 Subject: [PATCH] Don't warn when boxing large arrays --- clippy_lints/src/escape.rs | 41 +++++++++++++++++++++++---- clippy_lints/src/lib.rs | 3 +- clippy_lints/src/utils/conf.rs | 2 ++ tests/compile-fail/escape_analysis.rs | 9 ++++++ 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 2bdfe91a908..d98420a69f6 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -1,17 +1,21 @@ use rustc::hir::*; use rustc::hir::intravisit as visit; use rustc::hir::map::Node::{NodeExpr, NodeStmt}; +use rustc::infer::InferCtxt; use rustc::lint::*; use rustc::middle::expr_use_visitor::*; use rustc::middle::mem_categorization::{cmt, Categorization}; use rustc::ty::adjustment::AutoAdjustment; use rustc::ty; +use rustc::ty::layout::TargetDataLayout; use rustc::util::nodemap::NodeSet; use syntax::ast::NodeId; use syntax::codemap::Span; use utils::span_lint; -pub struct Pass; +pub struct Pass { + pub too_large_for_stack: u64, +} /// **What it does:** This lint checks for usage of `Box` where an unboxed `T` would work fine. /// @@ -39,9 +43,12 @@ fn is_non_trait_box(ty: ty::Ty) -> bool { } } -struct EscapeDelegate<'a, 'tcx: 'a> { +struct EscapeDelegate<'a, 'tcx: 'a+'gcx, 'gcx: 'a> { tcx: ty::TyCtxt<'a, 'tcx, 'tcx>, set: NodeSet, + infcx: &'a InferCtxt<'a, 'gcx, 'gcx>, + target: TargetDataLayout, + too_large_for_stack: u64, } impl LintPass for Pass { @@ -55,9 +62,15 @@ impl LateLintPass for Pass { let param_env = ty::ParameterEnvironment::for_item(cx.tcx, id); let infcx = cx.tcx.borrowck_fake_infer_ctxt(param_env); + + // we store the infcx because it is expensive to recreate + // the context each time. let mut v = EscapeDelegate { tcx: cx.tcx, set: NodeSet(), + infcx: &infcx, + target: TargetDataLayout::parse(cx.sess()), + too_large_for_stack: self.too_large_for_stack, }; { @@ -74,7 +87,7 @@ impl LateLintPass for Pass { } } -impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { +impl<'a, 'tcx: 'a+'gcx, 'gcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx, 'gcx> { fn consume(&mut self, _: NodeId, _: Span, cmt: cmt<'tcx>, mode: ConsumeMode) { if let Categorization::Local(lid) = cmt.cat { if self.set.contains(&lid) { @@ -93,7 +106,7 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { if let Some(NodeExpr(..)) = map.find(map.get_parent_node(consume_pat.id)) { return; } - if is_non_trait_box(cmt.ty) { + if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { self.set.insert(consume_pat.id); } return; @@ -104,7 +117,7 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { if let DeclLocal(ref loc) = decl.node { if let Some(ref ex) = loc.init { if let ExprBox(..) = ex.node { - if is_non_trait_box(cmt.ty) { + if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { // let x = box (...) self.set.insert(consume_pat.id); } @@ -170,3 +183,21 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { fn decl_without_init(&mut self, _: NodeId, _: Span) {} fn mutate(&mut self, _: NodeId, _: Span, _: cmt<'tcx>, _: MutateMode) {} } + +impl<'a, 'tcx: 'a+'gcx, 'gcx: 'a> EscapeDelegate<'a, 'tcx, 'gcx> { + fn is_large_box(&self, ty: ty::Ty<'gcx>) -> bool { + // Large types need to be boxed to avoid stack + // overflows. + match ty.sty { + ty::TyBox(ref inner) => { + if let Ok(layout) = inner.layout(self.infcx) { + let size = layout.size(&self.target); + size.bytes() > self.too_large_for_stack + } else { + false + } + }, + _ => false, + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c873ef8cc57..5549e19ccb1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -9,6 +9,7 @@ #![feature(slice_patterns)] #![feature(stmt_expr_attributes)] #![feature(type_macros)] +#![feature(iter_arith)] #![allow(indexing_slicing, shadow_reuse, unknown_lints)] @@ -219,7 +220,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box temporary_assignment::Pass); reg.register_late_lint_pass(box transmute::Transmute); reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold)); - reg.register_late_lint_pass(box escape::Pass); + reg.register_late_lint_pass(box escape::Pass{too_large_for_stack: conf.too_large_for_stack}); reg.register_early_lint_pass(box misc_early::MiscEarly); reg.register_late_lint_pass(box misc::UsedUnderscoreBinding); reg.register_late_lint_pass(box array_indexing::ArrayIndexing); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 6f2bf158521..5d335c8c12c 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -158,6 +158,8 @@ define_Conf! { ("type-complexity-threshold", type_complexity_threshold, 250 => u64), /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have ("single-char-binding-names-threshold", max_single_char_names, 5 => u64), + /// Lint: BOXED_LOCAL. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap + ("too-large-for-stack", too_large_for_stack, 200 => u64), } /// Read the `toml` configuration file. The function will ignore “File not found” errors iif diff --git a/tests/compile-fail/escape_analysis.rs b/tests/compile-fail/escape_analysis.rs index c0893bcd767..cb4f2b0a655 100644 --- a/tests/compile-fail/escape_analysis.rs +++ b/tests/compile-fail/escape_analysis.rs @@ -103,3 +103,12 @@ fn warn_match() { ref y => () } } + +fn nowarn_large_array() { + // should not warn, is large array + // and should not be on stack + let x = box [1; 10000]; + match &x { // not moved + ref y => () + } +}