factor out the rvalue lifetime rule

remove region_scope_tree from RegionCtxt

Apply suggestions from code review

Co-authored-by: Niko Matsakis <niko@alum.mit.edu>

2 files changed, 76 insertions, 46 deletions

diff --git a/clippy_lints/src/loops/needless_range_loop.rs b/clippy_lints/src/loops/needless_range_loop.rs
index 09f9c05b4fc..e2b82f9fd02 100644
--- a/clippy_lints/src/loops/needless_range_loop.rs
+++ b/clippy_lints/src/loops/needless_range_loop.rs
@@ -3,7 +3,9 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
 use clippy_utils::source::snippet;
 use clippy_utils::ty::has_iter_method;
 use clippy_utils::visitors::is_local_used;
-use clippy_utils::{contains_name, higher, is_integer_const, match_trait_method, paths, sugg, SpanlessEq};
+use clippy_utils::{
+    contains_name, higher, is_integer_const, match_trait_method, paths, sugg, SpanlessEq,
+};
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -27,12 +29,7 @@ pub(super) fn check<'tcx>(
     body: &'tcx Expr<'_>,
     expr: &'tcx Expr<'_>,
 ) {
-    if let Some(higher::Range {
-        start: Some(start),
-        ref end,
-        limits,
-    }) = higher::Range::hir(arg)
-    {
+    if let Some(higher::Range { start: Some(start), ref end, limits }) = higher::Range::hir(arg) {
         // the var must be a single name
         if let PatKind::Binding(_, canonical_id, ident, _) = pat.kind {
             let mut visitor = VarVisitor {
@@ -58,7 +55,11 @@ pub(super) fn check<'tcx>(
                 // ensure that the indexed variable was declared before the loop, see #601
                 if let Some(indexed_extent) = indexed_extent {
                     let parent_def_id = cx.tcx.hir().get_parent_item(expr.hir_id);
-                    let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id);
+                    let parent_body_id = cx
+                        .tcx
+                        .hir()
+                        .body_owned_by(cx.tcx.hir().local_def_id_to_hir_id(parent_def_id));
+                    let region_scope_tree = &cx.tcx.typeck_body(parent_body_id).region_scope_tree;
                     let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).unwrap();
                     if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) {
                         return;
@@ -107,17 +108,22 @@ pub(super) fn check<'tcx>(
                         }
                     }
 
-                    if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
+                    if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty)
+                    {
                         String::new()
-                    } else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr) {
+                    } else if visitor.indexed_mut.contains(&indexed)
+                        && contains_name(indexed, take_expr)
+                    {
                         return;
                     } else {
                         match limits {
                             ast::RangeLimits::Closed => {
                                 let take_expr = sugg::Sugg::hir(cx, take_expr, "<count>");
                                 format!(".take({})", take_expr + sugg::ONE)
-                            },
-                            ast::RangeLimits::HalfOpen => format!(".take({})", snippet(cx, take_expr.span, "..")),
+                            }
+                            ast::RangeLimits::HalfOpen => {
+                                format!(".take({})", snippet(cx, take_expr.span, ".."))
+                            }
                         }
                     }
                 } else {
@@ -143,7 +149,10 @@ pub(super) fn check<'tcx>(
                         cx,
                         NEEDLESS_RANGE_LOOP,
                         arg.span,
-                        &format!("the loop variable `{}` is used to index `{}`", ident.name, indexed),
+                        &format!(
+                            "the loop variable `{}` is used to index `{}`",
+                            ident.name, indexed
+                        ),
                         |diag| {
                             multispan_sugg(
                                 diag,
@@ -152,7 +161,10 @@ pub(super) fn check<'tcx>(
                                     (pat.span, format!("({}, <item>)", ident.name)),
                                     (
                                         arg.span,
-                                        format!("{}.{}().enumerate(){}{}", indexed, method, method_1, method_2),
+                                        format!(
+                                            "{}.{}().enumerate(){}{}",
+                                            indexed, method, method_1, method_2
+                                        ),
                                     ),
                                 ],
                             );
@@ -169,7 +181,10 @@ pub(super) fn check<'tcx>(
                         cx,
                         NEEDLESS_RANGE_LOOP,
                         arg.span,
-                        &format!("the loop variable `{}` is only used to index `{}`", ident.name, indexed),
+                        &format!(
+                            "the loop variable `{}` is only used to index `{}`",
+                            ident.name, indexed
+                        ),
                         |diag| {
                             multispan_sugg(
                                 diag,
@@ -246,7 +261,12 @@ struct VarVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
-    fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
+    fn check(
+        &mut self,
+        idx: &'tcx Expr<'_>,
+        seqexpr: &'tcx Expr<'_>,
+        expr: &'tcx Expr<'_>,
+    ) -> bool {
         if_chain! {
             // the indexed container is referenced by a name
             if let ExprKind::Path(ref seqpath) = seqexpr.kind;
@@ -262,7 +282,16 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
                 match res {
                     Res::Local(hir_id) => {
                         let parent_def_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
-                        let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id).unwrap();
+                        let parent_body_id = self.cx
+                            .tcx
+                            .hir()
+                            .body_owned_by(self.cx.tcx.hir().local_def_id_to_hir_id(parent_def_id));
+                        let extent = self.cx
+                            .tcx
+                            .typeck_body(parent_body_id)
+                            .region_scope_tree
+                            .var_scope(hir_id.local_id)
+                            .unwrap();
                         if index_used_directly {
                             self.indexed_directly.insert(
                                 seqvar.segments[0].ident.name,
@@ -331,13 +360,13 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
                 self.visit_expr(lhs);
                 self.prefer_mutable = false;
                 self.visit_expr(rhs);
-            },
+            }
             ExprKind::AddrOf(BorrowKind::Ref, mutbl, expr) => {
                 if mutbl == Mutability::Mut {
                     self.prefer_mutable = true;
                 }
                 self.visit_expr(expr);
-            },
+            }
             ExprKind::Call(f, args) => {
                 self.visit_expr(f);
                 for expr in args {
@@ -350,10 +379,11 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
                     }
                     self.visit_expr(expr);
                 }
-            },
+            }
             ExprKind::MethodCall(_, args, _) => {
                 let def_id = self.cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap();
-                for (ty, expr) in iter::zip(self.cx.tcx.fn_sig(def_id).inputs().skip_binder(), args) {
+                for (ty, expr) in iter::zip(self.cx.tcx.fn_sig(def_id).inputs().skip_binder(), args)
+                {
                     self.prefer_mutable = false;
                     if let ty::Ref(_, _, mutbl) = *ty.kind() {
                         if mutbl == Mutability::Mut {
@@ -362,11 +392,11 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
                     }
                     self.visit_expr(expr);
                 }
-            },
+            }
             ExprKind::Closure(_, _, body_id, ..) => {
                 let body = self.cx.tcx.hir().body(body_id);
                 self.visit_expr(&body.value);
-            },
+            }
             _ => walk_expr(self, expr),
         }
         self.prefer_mutable = old;
diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs
index 1ab7f52110c..db32b8d740b 100644
--- a/clippy_lints/src/shadow.rs
+++ b/clippy_lints/src/shadow.rs
@@ -5,7 +5,9 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::Res;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::hir_id::ItemLocalId;
-use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp};
+use rustc_hir::{
+    Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp,
+};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{Span, Symbol};
@@ -139,27 +141,31 @@ impl<'tcx> LateLintPass<'tcx> for Shadow {
 
     fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
         let hir = cx.tcx.hir();
-        if !matches!(
-            hir.body_owner_kind(hir.body_owner_def_id(body.id())),
-            BodyOwnerKind::Closure
-        ) {
+        if !matches!(hir.body_owner_kind(hir.body_owner_def_id(body.id())), BodyOwnerKind::Closure)
+        {
             self.bindings.push(FxHashMap::default());
         }
     }
 
     fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
         let hir = cx.tcx.hir();
-        if !matches!(
-            hir.body_owner_kind(hir.body_owner_def_id(body.id())),
-            BodyOwnerKind::Closure
-        ) {
+        if !matches!(hir.body_owner_kind(hir.body_owner_def_id(body.id())), BodyOwnerKind::Closure)
+        {
             self.bindings.pop();
         }
     }
 }
 
-fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool {
-    let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id());
+fn is_shadow(
+    cx: &LateContext<'_>,
+    owner: LocalDefId,
+    first: ItemLocalId,
+    second: ItemLocalId,
+) -> bool {
+    let scope_tree = &cx
+        .tcx
+        .typeck_body(cx.tcx.hir().body_owned_by(cx.tcx.hir().local_def_id_to_hir_id(owner)))
+        .region_scope_tree;
     let first_scope = scope_tree.var_scope(first).unwrap();
     let second_scope = scope_tree.var_scope(second).unwrap();
     scope_tree.is_subscope_of(second_scope, first_scope)
@@ -174,15 +180,16 @@ fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span)
                 snippet(cx, expr.span, "..")
             );
             (SHADOW_SAME, msg)
-        },
+        }
         Some(expr) if is_local_used(cx, expr, shadowed) => {
             let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_"));
             (SHADOW_REUSE, msg)
-        },
+        }
         _ => {
-            let msg = format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_"));
+            let msg =
+                format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_"));
             (SHADOW_UNRELATED, msg)
-        },
+        }
     };
     span_lint_and_note(
         cx,
@@ -211,14 +218,7 @@ fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_
         expr = match expr.kind {
             ExprKind::Box(e)
             | ExprKind::AddrOf(_, _, e)
-            | ExprKind::Block(
-                &Block {
-                    stmts: [],
-                    expr: Some(e),
-                    ..
-                },
-                _,
-            )
+            | ExprKind::Block(&Block { stmts: [], expr: Some(e), .. }, _)
             | ExprKind::Unary(UnOp::Deref, e) => e,
             ExprKind::Path(QPath::Resolved(None, path)) => break path.res == Res::Local(hir_id),
             _ => break false,