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>
This commit is contained in:
parent
fc965c7300
commit
e885157f03
@ -3,7 +3,9 @@
|
||||
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 @@ fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Ex
|
||||
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 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
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 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
}
|
||||
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 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
}
|
||||
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;
|
||||
|
@ -5,7 +5,9 @@
|
||||
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 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
|
||||
|
||||
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,
|
||||
|
Loading…
Reference in New Issue
Block a user