]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/escape.rs
Auto merge of #4938 - flip1995:rustup, r=flip1995
[rust.git] / clippy_lints / src / escape.rs
index 0491cde4fed6f4228655a2bb98dad9c9acb5b8e6..c44f2c696580c311700f3a7bd4a51c0bde8b1187 100644 (file)
@@ -1,48 +1,38 @@
-// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
-// file at the top-level directory of this distribution.
-//
-// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-
-use crate::rustc::hir::*;
-use crate::rustc::hir::intravisit as visit;
-use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use crate::rustc::{declare_tool_lint, lint_array};
-use crate::rustc::middle::expr_use_visitor::*;
-use crate::rustc::middle::mem_categorization::{cmt_, Categorization};
-use crate::rustc::ty::{self, Ty};
-use crate::rustc::ty::layout::LayoutOf;
-use crate::rustc::util::nodemap::NodeSet;
-use crate::syntax::ast::NodeId;
-use crate::syntax::source_map::Span;
+use rustc::hir::intravisit as visit;
+use rustc::hir::{self, *};
+use rustc::impl_lint_pass;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::ty::layout::LayoutOf;
+use rustc::ty::{self, Ty};
+use rustc::util::nodemap::HirIdSet;
+use rustc_session::declare_tool_lint;
+use rustc_typeck::expr_use_visitor::*;
+use syntax::source_map::Span;
+
 use crate::utils::span_lint;
 
-pub struct Pass {
+#[derive(Copy, Clone)]
+pub struct BoxedLocal {
     pub too_large_for_stack: u64,
 }
 
-/// **What it does:** Checks for usage of `Box<T>` where an unboxed `T` would
-/// work fine.
-///
-/// **Why is this bad?** This is an unnecessary allocation, and bad for
-/// performance. It is only necessary to allocate if you wish to move the box
-/// into something.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-/// ```rust
-/// fn main() {
-///     let x = Box::new(1);
-///     foo(*x);
-///     println!("{}", *x);
-/// }
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `Box<T>` where an unboxed `T` would
+    /// work fine.
+    ///
+    /// **Why is this bad?** This is an unnecessary allocation, and bad for
+    /// performance. It is only necessary to allocate if you wish to move the box
+    /// into something.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # fn foo(bar: usize) {}
+    /// let x = Box::new(1);
+    /// foo(*x);
+    /// println!("{}", *x);
+    /// ```
     pub BOXED_LOCAL,
     perf,
     "using `Box<T>` where unnecessary"
@@ -52,131 +42,122 @@ fn is_non_trait_box(ty: Ty<'_>) -> bool {
     ty.is_box() && !ty.boxed_ty().is_trait()
 }
 
-struct EscapeDelegate<'a, 'tcx: 'a> {
+struct EscapeDelegate<'a, 'tcx> {
     cx: &'a LateContext<'a, 'tcx>,
-    set: NodeSet,
+    set: HirIdSet,
     too_large_for_stack: u64,
 }
 
-impl LintPass for Pass {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(BOXED_LOCAL)
-    }
-}
+impl_lint_pass!(BoxedLocal => [BOXED_LOCAL]);
 
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal {
     fn check_fn(
         &mut self,
         cx: &LateContext<'a, 'tcx>,
         _: visit::FnKind<'tcx>,
         _: &'tcx FnDecl,
-        body: &'tcx Body,
+        body: &'tcx Body<'_>,
         _: Span,
-        node_id: NodeId,
+        hir_id: HirId,
     ) {
-        let fn_def_id = cx.tcx.hir.local_def_id(node_id);
+        // If the method is an impl for a trait, don't warn.
+        let parent_id = cx.tcx.hir().get_parent_item(hir_id);
+        let parent_node = cx.tcx.hir().find(parent_id);
+
+        if let Some(Node::Item(item)) = parent_node {
+            if let ItemKind::Impl(_, _, _, _, Some(..), _, _) = item.kind {
+                return;
+            }
+        }
+
         let mut v = EscapeDelegate {
             cx,
-            set: NodeSet(),
+            set: HirIdSet::default(),
             too_large_for_stack: self.too_large_for_stack,
         };
 
-        let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
-        ExprUseVisitor::new(&mut v, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None).consume_body(body);
+        let fn_def_id = cx.tcx.hir().local_def_id(hir_id);
+        cx.tcx.infer_ctxt().enter(|infcx| {
+            ExprUseVisitor::new(&mut v, &infcx, fn_def_id, cx.param_env, cx.tables).consume_body(body);
+        });
 
         for node in v.set {
             span_lint(
                 cx,
                 BOXED_LOCAL,
-                cx.tcx.hir.span(node),
+                cx.tcx.hir().span(node),
                 "local variable doesn't need to be boxed here",
             );
         }
     }
 }
 
-impl<'a, 'tcx> 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 let Move(DirectRefMove) = mode {
-                // moved out or in. clearly can't be localized
-                self.set.remove(&lid);
-            }
-        }
+// TODO: Replace with Map::is_argument(..) when it's fixed
+fn is_argument(map: &hir::map::Map<'_>, id: HirId) -> bool {
+    match map.find(id) {
+        Some(Node::Binding(_)) => (),
+        _ => return false,
     }
-    fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {}
-    fn consume_pat(&mut self, consume_pat: &Pat, cmt: &cmt_<'tcx>, _: ConsumeMode) {
-        let map = &self.cx.tcx.hir;
-        if map.is_argument(consume_pat.id) {
-            // Skip closure arguments
-            if let Some(Node::Expr(..)) = map.find(map.get_parent_node(consume_pat.id)) {
-                return;
-            }
-            if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) {
-                self.set.insert(consume_pat.id);
-            }
-            return;
-        }
-        if let Categorization::Rvalue(..) = cmt.cat {
-            let id = map.hir_to_node_id(cmt.hir_id);
-            if let Some(Node::Stmt(st)) = map.find(map.get_parent_node(id)) {
-                if let StmtKind::Decl(ref decl, _) = st.node {
-                    if let DeclKind::Local(ref loc) = decl.node {
-                        if let Some(ref ex) = loc.init {
-                            if let ExprKind::Box(..) = ex.node {
-                                if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) {
-                                    // let x = box (...)
-                                    self.set.insert(consume_pat.id);
-                                }
-                                // TODO Box::new
-                                // TODO vec![]
-                                // TODO "foo".to_owned() and friends
-                            }
-                        }
+
+    match map.find(map.get_parent_node(id)) {
+        Some(Node::Param(_)) => true,
+        _ => false,
+    }
+}
+
+impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
+    fn consume(&mut self, cmt: &Place<'tcx>, mode: ConsumeMode) {
+        if cmt.projections.is_empty() {
+            if let PlaceBase::Local(lid) = cmt.base {
+                if let ConsumeMode::Move = mode {
+                    // moved out or in. clearly can't be localized
+                    self.set.remove(&lid);
+                }
+                let map = &self.cx.tcx.hir();
+                if let Some(Node::Binding(_)) = map.find(cmt.hir_id) {
+                    if self.set.contains(&lid) {
+                        // let y = x where x is known
+                        // remove x, insert y
+                        self.set.insert(cmt.hir_id);
+                        self.set.remove(&lid);
                     }
                 }
             }
         }
-        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);
+    }
+
+    fn borrow(&mut self, cmt: &Place<'tcx>, _: ty::BorrowKind) {
+        if cmt.projections.is_empty() {
+            if let PlaceBase::Local(lid) = cmt.base {
                 self.set.remove(&lid);
             }
         }
     }
-    fn borrow(&mut self, _: NodeId, _: Span, cmt: &cmt_<'tcx>, _: ty::Region<'_>, _: ty::BorrowKind, loan_cause: LoanCause) {
-        if let Categorization::Local(lid) = cmt.cat {
-            match loan_cause {
-                // x.foo()
-                // Used without autodereffing (i.e. x.clone())
-                LoanCause::AutoRef |
-
-                // &x
-                // foo(&x) where no extra autoreffing is happening
-                LoanCause::AddrOf |
-
-                // `match x` can move
-                LoanCause::MatchDiscriminant => {
-                    self.set.remove(&lid);
+
+    fn mutate(&mut self, cmt: &Place<'tcx>) {
+        if cmt.projections.is_empty() {
+            let map = &self.cx.tcx.hir();
+            if is_argument(map, cmt.hir_id) {
+                // Skip closure arguments
+                let parent_id = map.get_parent_node(cmt.hir_id);
+                if let Some(Node::Expr(..)) = map.find(map.get_parent_node(parent_id)) {
+                    return;
                 }
 
-                // do nothing for matches, etc. These can't escape
-                _ => {}
+                if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) {
+                    self.set.insert(cmt.hir_id);
+                }
+                return;
             }
         }
     }
-    fn decl_without_init(&mut self, _: NodeId, _: Span) {}
-    fn mutate(&mut self, _: NodeId, _: Span, _: &cmt_<'tcx>, _: MutateMode) {}
 }
 
 impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> {
     fn is_large_box(&self, ty: Ty<'tcx>) -> bool {
-        // Large types need to be boxed to avoid stack
-        // overflows.
+        // Large types need to be boxed to avoid stack overflows.
         if ty.is_box() {
-            self.cx.layout_of(ty.boxed_ty()).ok().map_or(0, |l| l.size.bytes()) > self.too_large_for_stack
+            self.cx.layout_of(ty.boxed_ty()).map_or(0, |l| l.size.bytes()) > self.too_large_for_stack
         } else {
             false
         }