From e885157f030cdfac377febefb6acd4d33c26aaba Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 1 Apr 2022 21:12:18 +0800 Subject: [PATCH] factor out the rvalue lifetime rule remove region_scope_tree from RegionCtxt Apply suggestions from code review Co-authored-by: Niko Matsakis --- clippy_lints/src/loops/needless_range_loop.rs | 76 +++++++++++++------ clippy_lints/src/shadow.rs | 46 +++++------ 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 09f9c05b4fce..e2b82f9fd02a 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, ""); 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!("({}, )", 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 1ab7f52110ce..db32b8d740b3 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,