Skip to content

Commit ab95b45

Browse files
flip1995flip1995
committed
Fix needless_pass_by_value
This also accidentally improved the spans of the suggestions
1 parent f3cdee4 commit ab95b45

File tree

3 files changed

+14
-108
lines changed

3 files changed

+14
-108
lines changed

‎clippy_lints/src/needless_pass_by_value.rs

Lines changed: 6 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
134134
spans_need_deref,
135135
..
136136
} = {
137-
let mut ctx = MovedVariablesCtxt::new(cx);
137+
let mut ctx = MovedVariablesCtxt::default();
138138
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
139139
euv::ExprUseVisitor::new(
140140
&mut ctx,
@@ -324,101 +324,31 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool {
324324
})
325325
}
326326

327-
struct MovedVariablesCtxt<'a, 'tcx> {
328-
cx: &'a LateContext<'a, 'tcx>,
327+
#[derive(Default)]
328+
struct MovedVariablesCtxt {
329329
moved_vars: FxHashSet<HirId>,
330330
/// Spans which need to be prefixed with `*` for dereferencing the
331331
/// suggested additional reference.
332332
spans_need_deref: FxHashMap<HirId, FxHashSet<Span>>,
333333
}
334334

335-
impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> {
336-
fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
337-
Self {
338-
cx,
339-
moved_vars: FxHashSet::default(),
340-
spans_need_deref: FxHashMap::default(),
341-
}
342-
}
343-
344-
fn move_common(&mut self, _consume_id: HirId, _span: Span, cmt: &mc::cmt_<'tcx>) {
335+
impl MovedVariablesCtxt {
336+
fn move_common(&mut self, _consume_id: HirId, _span: Span, cmt: &mc::cmt_<'_>) {
345337
let cmt = unwrap_downcast_or_interior(cmt);
346338

347339
if let mc::Categorization::Local(vid) = cmt.cat {
348340
self.moved_vars.insert(vid);
349341
}
350342
}
351-
352-
fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>) {
353-
let cmt = unwrap_downcast_or_interior(cmt);
354-
355-
if let mc::Categorization::Local(vid) = cmt.cat {
356-
let mut id = matched_pat.hir_id;
357-
loop {
358-
let parent = self.cx.tcx.hir().get_parent_node(id);
359-
if id == parent {
360-
// no parent
361-
return;
362-
}
363-
id = parent;
364-
365-
if let Some(node) = self.cx.tcx.hir().find(id) {
366-
match node {
367-
Node::Expr(e) => {
368-
// `match` and `if let`
369-
if let ExprKind::Match(ref c, ..) = e.kind {
370-
self.spans_need_deref
371-
.entry(vid)
372-
.or_insert_with(FxHashSet::default)
373-
.insert(c.span);
374-
}
375-
},
376-
377-
Node::Stmt(s) => {
378-
// `let <pat> = x;`
379-
if_chain! {
380-
if let StmtKind::Local(ref local) = s.kind;
381-
then {
382-
self.spans_need_deref
383-
.entry(vid)
384-
.or_insert_with(FxHashSet::default)
385-
.insert(local.init
386-
.as_ref()
387-
.map(|e| e.span)
388-
.expect("`let` stmt without init aren't caught by match_pat"));
389-
}
390-
}
391-
},
392-
393-
_ => {},
394-
}
395-
}
396-
}
397-
}
398-
}
399343
}
400344

401-
impl<'a, 'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt<'a, 'tcx> {
345+
impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt {
402346
fn consume(&mut self, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
403347
if let euv::ConsumeMode::Move = mode {
404348
self.move_common(cmt.hir_id, cmt.span, cmt);
405349
}
406350
}
407351

408-
fn matched_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::MatchMode) {
409-
if let euv::MatchMode::MovingMatch = mode {
410-
self.move_common(matched_pat.hir_id, matched_pat.span, cmt);
411-
} else {
412-
self.non_moving_pat(matched_pat, cmt);
413-
}
414-
}
415-
416-
fn consume_pat(&mut self, consume_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
417-
if let euv::ConsumeMode::Move(_) = mode {
418-
self.move_common(consume_pat.hir_id, consume_pat.span, cmt);
419-
}
420-
}
421-
422352
fn borrow(
423353
&mut self,
424354
_: &mc::cmt_<'tcx>,

‎tests/ui/mut_range_bound.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,27 @@ error: attempt to mutate range bound within loop; note that the range of the loo
22
--> $DIR/mut_range_bound.rs:16:9
33
|
44
LL | m = 5;
5-
| ^^^^^
5+
| ^
66
|
77
= note: `-D clippy::mut-range-bound` implied by `-D warnings`
88

99
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
1010
--> $DIR/mut_range_bound.rs:23:9
1111
|
1212
LL | m *= 2;
13-
| ^^^^^^
13+
| ^
1414

1515
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
1616
--> $DIR/mut_range_bound.rs:31:9
1717
|
1818
LL | m = 5;
19-
| ^^^^^
19+
| ^
2020

2121
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
2222
--> $DIR/mut_range_bound.rs:32:9
2323
|
2424
LL | n = 7;
25-
| ^^^^^
25+
| ^
2626

2727
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
2828
--> $DIR/mut_range_bound.rs:46:22

‎tests/ui/needless_pass_by_value.stderr

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,7 @@ error: this argument is passed by value, but not consumed in the function body
2828
--> $DIR/needless_pass_by_value.rs:48:18
2929
|
3030
LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
31-
| ^^^^^^^^^^^^^^^^^^^^^^
32-
help: consider taking a reference instead
33-
|
34-
LL | fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
35-
LL | match *x {
36-
|
31+
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option<Option<String>>`
3732

3833
error: this argument is passed by value, but not consumed in the function body
3934
--> $DIR/needless_pass_by_value.rs:61:24
@@ -45,14 +40,7 @@ error: this argument is passed by value, but not consumed in the function body
4540
--> $DIR/needless_pass_by_value.rs:61:36
4641
|
4742
LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
48-
| ^^^^^^^
49-
help: consider taking a reference instead
50-
|
51-
LL | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
52-
LL | let Wrapper(s) = z; // moved
53-
LL | let Wrapper(ref t) = *y; // not moved
54-
LL | let Wrapper(_) = *y; // still not moved
55-
|
43+
| ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
5644

5745
error: this argument is passed by value, but not consumed in the function body
5846
--> $DIR/needless_pass_by_value.rs:77:49
@@ -152,37 +140,25 @@ error: this argument is passed by value, but not consumed in the function body
152140
--> $DIR/needless_pass_by_value.rs:130:45
153141
|
154142
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
155-
| ^^^^^^^^^^^
143+
| ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
156144
|
157145
help: consider marking this type as Copy
158146
--> $DIR/needless_pass_by_value.rs:122:1
159147
|
160148
LL | struct CopyWrapper(u32);
161149
| ^^^^^^^^^^^^^^^^^^^^^^^^
162-
help: consider taking a reference instead
163-
|
164-
LL | fn test_destructure_copy(x: CopyWrapper, y: &CopyWrapper, z: CopyWrapper) {
165-
LL | let CopyWrapper(s) = z; // moved
166-
LL | let CopyWrapper(ref t) = *y; // not moved
167-
LL | let CopyWrapper(_) = *y; // still not moved
168-
|
169150

170151
error: this argument is passed by value, but not consumed in the function body
171152
--> $DIR/needless_pass_by_value.rs:130:61
172153
|
173154
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
174-
| ^^^^^^^^^^^
155+
| ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
175156
|
176157
help: consider marking this type as Copy
177158
--> $DIR/needless_pass_by_value.rs:122:1
178159
|
179160
LL | struct CopyWrapper(u32);
180161
| ^^^^^^^^^^^^^^^^^^^^^^^^
181-
help: consider taking a reference instead
182-
|
183-
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: &CopyWrapper) {
184-
LL | let CopyWrapper(s) = *z; // moved
185-
|
186162

187163
error: this argument is passed by value, but not consumed in the function body
188164
--> $DIR/needless_pass_by_value.rs:142:40

0 commit comments

Comments
 (0)