Skip to content

Commit e885157

Browse files
factor out the rvalue lifetime rule
remove region_scope_tree from RegionCtxt Apply suggestions from code review Co-authored-by: Niko Matsakis <[email protected]>
1 parent fc965c7 commit e885157

File tree

2 files changed

+76
-46
lines changed

2 files changed

+76
-46
lines changed

clippy_lints/src/loops/needless_range_loop.rs

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
33
use clippy_utils::source::snippet;
44
use clippy_utils::ty::has_iter_method;
55
use clippy_utils::visitors::is_local_used;
6-
use clippy_utils::{contains_name, higher, is_integer_const, match_trait_method, paths, sugg, SpanlessEq};
6+
use clippy_utils::{
7+
contains_name, higher, is_integer_const, match_trait_method, paths, sugg, SpanlessEq,
8+
};
79
use if_chain::if_chain;
810
use rustc_ast::ast;
911
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -27,12 +29,7 @@ pub(super) fn check<'tcx>(
2729
body: &'tcx Expr<'_>,
2830
expr: &'tcx Expr<'_>,
2931
) {
30-
if let Some(higher::Range {
31-
start: Some(start),
32-
ref end,
33-
limits,
34-
}) = higher::Range::hir(arg)
35-
{
32+
if let Some(higher::Range { start: Some(start), ref end, limits }) = higher::Range::hir(arg) {
3633
// the var must be a single name
3734
if let PatKind::Binding(_, canonical_id, ident, _) = pat.kind {
3835
let mut visitor = VarVisitor {
@@ -58,7 +55,11 @@ pub(super) fn check<'tcx>(
5855
// ensure that the indexed variable was declared before the loop, see #601
5956
if let Some(indexed_extent) = indexed_extent {
6057
let parent_def_id = cx.tcx.hir().get_parent_item(expr.hir_id);
61-
let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id);
58+
let parent_body_id = cx
59+
.tcx
60+
.hir()
61+
.body_owned_by(cx.tcx.hir().local_def_id_to_hir_id(parent_def_id));
62+
let region_scope_tree = &cx.tcx.typeck_body(parent_body_id).region_scope_tree;
6263
let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).unwrap();
6364
if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) {
6465
return;
@@ -107,17 +108,22 @@ pub(super) fn check<'tcx>(
107108
}
108109
}
109110

110-
if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
111+
if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty)
112+
{
111113
String::new()
112-
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr) {
114+
} else if visitor.indexed_mut.contains(&indexed)
115+
&& contains_name(indexed, take_expr)
116+
{
113117
return;
114118
} else {
115119
match limits {
116120
ast::RangeLimits::Closed => {
117121
let take_expr = sugg::Sugg::hir(cx, take_expr, "<count>");
118122
format!(".take({})", take_expr + sugg::ONE)
119-
},
120-
ast::RangeLimits::HalfOpen => format!(".take({})", snippet(cx, take_expr.span, "..")),
123+
}
124+
ast::RangeLimits::HalfOpen => {
125+
format!(".take({})", snippet(cx, take_expr.span, ".."))
126+
}
121127
}
122128
}
123129
} else {
@@ -143,7 +149,10 @@ pub(super) fn check<'tcx>(
143149
cx,
144150
NEEDLESS_RANGE_LOOP,
145151
arg.span,
146-
&format!("the loop variable `{}` is used to index `{}`", ident.name, indexed),
152+
&format!(
153+
"the loop variable `{}` is used to index `{}`",
154+
ident.name, indexed
155+
),
147156
|diag| {
148157
multispan_sugg(
149158
diag,
@@ -152,7 +161,10 @@ pub(super) fn check<'tcx>(
152161
(pat.span, format!("({}, <item>)", ident.name)),
153162
(
154163
arg.span,
155-
format!("{}.{}().enumerate(){}{}", indexed, method, method_1, method_2),
164+
format!(
165+
"{}.{}().enumerate(){}{}",
166+
indexed, method, method_1, method_2
167+
),
156168
),
157169
],
158170
);
@@ -169,7 +181,10 @@ pub(super) fn check<'tcx>(
169181
cx,
170182
NEEDLESS_RANGE_LOOP,
171183
arg.span,
172-
&format!("the loop variable `{}` is only used to index `{}`", ident.name, indexed),
184+
&format!(
185+
"the loop variable `{}` is only used to index `{}`",
186+
ident.name, indexed
187+
),
173188
|diag| {
174189
multispan_sugg(
175190
diag,
@@ -246,7 +261,12 @@ struct VarVisitor<'a, 'tcx> {
246261
}
247262

248263
impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
249-
fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
264+
fn check(
265+
&mut self,
266+
idx: &'tcx Expr<'_>,
267+
seqexpr: &'tcx Expr<'_>,
268+
expr: &'tcx Expr<'_>,
269+
) -> bool {
250270
if_chain! {
251271
// the indexed container is referenced by a name
252272
if let ExprKind::Path(ref seqpath) = seqexpr.kind;
@@ -262,7 +282,16 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
262282
match res {
263283
Res::Local(hir_id) => {
264284
let parent_def_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
265-
let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id).unwrap();
285+
let parent_body_id = self.cx
286+
.tcx
287+
.hir()
288+
.body_owned_by(self.cx.tcx.hir().local_def_id_to_hir_id(parent_def_id));
289+
let extent = self.cx
290+
.tcx
291+
.typeck_body(parent_body_id)
292+
.region_scope_tree
293+
.var_scope(hir_id.local_id)
294+
.unwrap();
266295
if index_used_directly {
267296
self.indexed_directly.insert(
268297
seqvar.segments[0].ident.name,
@@ -331,13 +360,13 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
331360
self.visit_expr(lhs);
332361
self.prefer_mutable = false;
333362
self.visit_expr(rhs);
334-
},
363+
}
335364
ExprKind::AddrOf(BorrowKind::Ref, mutbl, expr) => {
336365
if mutbl == Mutability::Mut {
337366
self.prefer_mutable = true;
338367
}
339368
self.visit_expr(expr);
340-
},
369+
}
341370
ExprKind::Call(f, args) => {
342371
self.visit_expr(f);
343372
for expr in args {
@@ -350,10 +379,11 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
350379
}
351380
self.visit_expr(expr);
352381
}
353-
},
382+
}
354383
ExprKind::MethodCall(_, args, _) => {
355384
let def_id = self.cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap();
356-
for (ty, expr) in iter::zip(self.cx.tcx.fn_sig(def_id).inputs().skip_binder(), args) {
385+
for (ty, expr) in iter::zip(self.cx.tcx.fn_sig(def_id).inputs().skip_binder(), args)
386+
{
357387
self.prefer_mutable = false;
358388
if let ty::Ref(_, _, mutbl) = *ty.kind() {
359389
if mutbl == Mutability::Mut {
@@ -362,11 +392,11 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
362392
}
363393
self.visit_expr(expr);
364394
}
365-
},
395+
}
366396
ExprKind::Closure(_, _, body_id, ..) => {
367397
let body = self.cx.tcx.hir().body(body_id);
368398
self.visit_expr(&body.value);
369-
},
399+
}
370400
_ => walk_expr(self, expr),
371401
}
372402
self.prefer_mutable = old;

clippy_lints/src/shadow.rs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ use rustc_data_structures::fx::FxHashMap;
55
use rustc_hir::def::Res;
66
use rustc_hir::def_id::LocalDefId;
77
use rustc_hir::hir_id::ItemLocalId;
8-
use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp};
8+
use rustc_hir::{
9+
Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp,
10+
};
911
use rustc_lint::{LateContext, LateLintPass};
1012
use rustc_session::{declare_tool_lint, impl_lint_pass};
1113
use rustc_span::{Span, Symbol};
@@ -139,27 +141,31 @@ impl<'tcx> LateLintPass<'tcx> for Shadow {
139141

140142
fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
141143
let hir = cx.tcx.hir();
142-
if !matches!(
143-
hir.body_owner_kind(hir.body_owner_def_id(body.id())),
144-
BodyOwnerKind::Closure
145-
) {
144+
if !matches!(hir.body_owner_kind(hir.body_owner_def_id(body.id())), BodyOwnerKind::Closure)
145+
{
146146
self.bindings.push(FxHashMap::default());
147147
}
148148
}
149149

150150
fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
151151
let hir = cx.tcx.hir();
152-
if !matches!(
153-
hir.body_owner_kind(hir.body_owner_def_id(body.id())),
154-
BodyOwnerKind::Closure
155-
) {
152+
if !matches!(hir.body_owner_kind(hir.body_owner_def_id(body.id())), BodyOwnerKind::Closure)
153+
{
156154
self.bindings.pop();
157155
}
158156
}
159157
}
160158

161-
fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool {
162-
let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id());
159+
fn is_shadow(
160+
cx: &LateContext<'_>,
161+
owner: LocalDefId,
162+
first: ItemLocalId,
163+
second: ItemLocalId,
164+
) -> bool {
165+
let scope_tree = &cx
166+
.tcx
167+
.typeck_body(cx.tcx.hir().body_owned_by(cx.tcx.hir().local_def_id_to_hir_id(owner)))
168+
.region_scope_tree;
163169
let first_scope = scope_tree.var_scope(first).unwrap();
164170
let second_scope = scope_tree.var_scope(second).unwrap();
165171
scope_tree.is_subscope_of(second_scope, first_scope)
@@ -174,15 +180,16 @@ fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span)
174180
snippet(cx, expr.span, "..")
175181
);
176182
(SHADOW_SAME, msg)
177-
},
183+
}
178184
Some(expr) if is_local_used(cx, expr, shadowed) => {
179185
let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_"));
180186
(SHADOW_REUSE, msg)
181-
},
187+
}
182188
_ => {
183-
let msg = format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_"));
189+
let msg =
190+
format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_"));
184191
(SHADOW_UNRELATED, msg)
185-
},
192+
}
186193
};
187194
span_lint_and_note(
188195
cx,
@@ -211,14 +218,7 @@ fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_
211218
expr = match expr.kind {
212219
ExprKind::Box(e)
213220
| ExprKind::AddrOf(_, _, e)
214-
| ExprKind::Block(
215-
&Block {
216-
stmts: [],
217-
expr: Some(e),
218-
..
219-
},
220-
_,
221-
)
221+
| ExprKind::Block(&Block { stmts: [], expr: Some(e), .. }, _)
222222
| ExprKind::Unary(UnOp::Deref, e) => e,
223223
ExprKind::Path(QPath::Resolved(None, path)) => break path.res == Res::Local(hir_id),
224224
_ => break false,

0 commit comments

Comments
 (0)