mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 23:24:24 +00:00
Fix needless_pass_by_value
This also accidentally improved the spans of the suggestions
This commit is contained in:
parent
b46f5b4a98
commit
c420b07191
3 changed files with 15 additions and 109 deletions
|
@ -134,7 +134,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
|
||||||
spans_need_deref,
|
spans_need_deref,
|
||||||
..
|
..
|
||||||
} = {
|
} = {
|
||||||
let mut ctx = MovedVariablesCtxt::new(cx);
|
let mut ctx = MovedVariablesCtxt::default();
|
||||||
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
|
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
|
||||||
euv::ExprUseVisitor::new(
|
euv::ExprUseVisitor::new(
|
||||||
&mut ctx,
|
&mut ctx,
|
||||||
|
@ -324,98 +324,28 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
struct MovedVariablesCtxt<'a, 'tcx> {
|
#[derive(Default)]
|
||||||
cx: &'a LateContext<'a, 'tcx>,
|
struct MovedVariablesCtxt {
|
||||||
moved_vars: FxHashSet<HirId>,
|
moved_vars: FxHashSet<HirId>,
|
||||||
/// Spans which need to be prefixed with `*` for dereferencing the
|
/// Spans which need to be prefixed with `*` for dereferencing the
|
||||||
/// suggested additional reference.
|
/// suggested additional reference.
|
||||||
spans_need_deref: FxHashMap<HirId, FxHashSet<Span>>,
|
spans_need_deref: FxHashMap<HirId, FxHashSet<Span>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> {
|
impl MovedVariablesCtxt {
|
||||||
fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
|
fn move_common(&mut self, cmt: &mc::cmt_<'_>) {
|
||||||
Self {
|
|
||||||
cx,
|
|
||||||
moved_vars: FxHashSet::default(),
|
|
||||||
spans_need_deref: FxHashMap::default(),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn move_common(&mut self, _consume_id: HirId, _span: Span, cmt: &mc::cmt_<'tcx>) {
|
|
||||||
let cmt = unwrap_downcast_or_interior(cmt);
|
let cmt = unwrap_downcast_or_interior(cmt);
|
||||||
|
|
||||||
if let mc::Categorization::Local(vid) = cmt.cat {
|
if let mc::Categorization::Local(vid) = cmt.cat {
|
||||||
self.moved_vars.insert(vid);
|
self.moved_vars.insert(vid);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>) {
|
|
||||||
let cmt = unwrap_downcast_or_interior(cmt);
|
|
||||||
|
|
||||||
if let mc::Categorization::Local(vid) = cmt.cat {
|
|
||||||
let mut id = matched_pat.hir_id;
|
|
||||||
loop {
|
|
||||||
let parent = self.cx.tcx.hir().get_parent_node(id);
|
|
||||||
if id == parent {
|
|
||||||
// no parent
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
id = parent;
|
|
||||||
|
|
||||||
if let Some(node) = self.cx.tcx.hir().find(id) {
|
|
||||||
match node {
|
|
||||||
Node::Expr(e) => {
|
|
||||||
// `match` and `if let`
|
|
||||||
if let ExprKind::Match(ref c, ..) = e.kind {
|
|
||||||
self.spans_need_deref
|
|
||||||
.entry(vid)
|
|
||||||
.or_insert_with(FxHashSet::default)
|
|
||||||
.insert(c.span);
|
|
||||||
}
|
|
||||||
},
|
|
||||||
|
|
||||||
Node::Stmt(s) => {
|
|
||||||
// `let <pat> = x;`
|
|
||||||
if_chain! {
|
|
||||||
if let StmtKind::Local(ref local) = s.kind;
|
|
||||||
then {
|
|
||||||
self.spans_need_deref
|
|
||||||
.entry(vid)
|
|
||||||
.or_insert_with(FxHashSet::default)
|
|
||||||
.insert(local.init
|
|
||||||
.as_ref()
|
|
||||||
.map(|e| e.span)
|
|
||||||
.expect("`let` stmt without init aren't caught by match_pat"));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
|
|
||||||
_ => {},
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a, 'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt<'a, 'tcx> {
|
impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt {
|
||||||
fn consume(&mut self, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
|
fn consume(&mut self, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
|
||||||
if let euv::ConsumeMode::Move = mode {
|
if let euv::ConsumeMode::Move = mode {
|
||||||
self.move_common(cmt.hir_id, cmt.span, cmt);
|
self.move_common(cmt);
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn matched_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::MatchMode) {
|
|
||||||
if let euv::MatchMode::MovingMatch = mode {
|
|
||||||
self.move_common(matched_pat.hir_id, matched_pat.span, cmt);
|
|
||||||
} else {
|
|
||||||
self.non_moving_pat(matched_pat, cmt);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn consume_pat(&mut self, consume_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
|
|
||||||
if let euv::ConsumeMode::Move(_) = mode {
|
|
||||||
self.move_common(consume_pat.hir_id, consume_pat.span, cmt);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -2,7 +2,7 @@ error: attempt to mutate range bound within loop; note that the range of the loo
|
||||||
--> $DIR/mut_range_bound.rs:16:9
|
--> $DIR/mut_range_bound.rs:16:9
|
||||||
|
|
|
|
||||||
LL | m = 5;
|
LL | m = 5;
|
||||||
| ^^^^^
|
| ^
|
||||||
|
|
|
|
||||||
= note: `-D clippy::mut-range-bound` implied by `-D warnings`
|
= note: `-D clippy::mut-range-bound` implied by `-D warnings`
|
||||||
|
|
||||||
|
@ -10,19 +10,19 @@ error: attempt to mutate range bound within loop; note that the range of the loo
|
||||||
--> $DIR/mut_range_bound.rs:23:9
|
--> $DIR/mut_range_bound.rs:23:9
|
||||||
|
|
|
|
||||||
LL | m *= 2;
|
LL | m *= 2;
|
||||||
| ^^^^^^
|
| ^
|
||||||
|
|
||||||
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
|
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
|
||||||
--> $DIR/mut_range_bound.rs:31:9
|
--> $DIR/mut_range_bound.rs:31:9
|
||||||
|
|
|
|
||||||
LL | m = 5;
|
LL | m = 5;
|
||||||
| ^^^^^
|
| ^
|
||||||
|
|
||||||
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
|
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
|
||||||
--> $DIR/mut_range_bound.rs:32:9
|
--> $DIR/mut_range_bound.rs:32:9
|
||||||
|
|
|
|
||||||
LL | n = 7;
|
LL | n = 7;
|
||||||
| ^^^^^
|
| ^
|
||||||
|
|
||||||
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
|
error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
|
||||||
--> $DIR/mut_range_bound.rs:46:22
|
--> $DIR/mut_range_bound.rs:46:22
|
||||||
|
|
|
@ -28,12 +28,7 @@ error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:49:18
|
--> $DIR/needless_pass_by_value.rs:49:18
|
||||||
|
|
|
|
||||||
LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
|
LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option<Option<String>>`
|
||||||
help: consider taking a reference instead
|
|
||||||
|
|
|
||||||
LL | fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
|
|
||||||
LL | match *x {
|
|
||||||
|
|
|
||||||
|
|
||||||
error: this argument is passed by value, but not consumed in the function body
|
error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:62:24
|
--> $DIR/needless_pass_by_value.rs:62:24
|
||||||
|
@ -45,14 +40,7 @@ error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:62:36
|
--> $DIR/needless_pass_by_value.rs:62:36
|
||||||
|
|
|
|
||||||
LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
|
LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
|
||||||
| ^^^^^^^
|
| ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
|
||||||
help: consider taking a reference instead
|
|
||||||
|
|
|
||||||
LL | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
|
|
||||||
LL | let Wrapper(s) = z; // moved
|
|
||||||
LL | let Wrapper(ref t) = *y; // not moved
|
|
||||||
LL | let Wrapper(_) = *y; // still not moved
|
|
||||||
|
|
|
||||||
|
|
||||||
error: this argument is passed by value, but not consumed in the function body
|
error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:78:49
|
--> $DIR/needless_pass_by_value.rs:78:49
|
||||||
|
@ -152,37 +140,25 @@ error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:131:45
|
--> $DIR/needless_pass_by_value.rs:131:45
|
||||||
|
|
|
|
||||||
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
|
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
|
||||||
| ^^^^^^^^^^^
|
| ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
|
||||||
|
|
|
|
||||||
help: consider marking this type as Copy
|
help: consider marking this type as Copy
|
||||||
--> $DIR/needless_pass_by_value.rs:123:1
|
--> $DIR/needless_pass_by_value.rs:123:1
|
||||||
|
|
|
|
||||||
LL | struct CopyWrapper(u32);
|
LL | struct CopyWrapper(u32);
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
help: consider taking a reference instead
|
|
||||||
|
|
|
||||||
LL | fn test_destructure_copy(x: CopyWrapper, y: &CopyWrapper, z: CopyWrapper) {
|
|
||||||
LL | let CopyWrapper(s) = z; // moved
|
|
||||||
LL | let CopyWrapper(ref t) = *y; // not moved
|
|
||||||
LL | let CopyWrapper(_) = *y; // still not moved
|
|
||||||
|
|
|
||||||
|
|
||||||
error: this argument is passed by value, but not consumed in the function body
|
error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:131:61
|
--> $DIR/needless_pass_by_value.rs:131:61
|
||||||
|
|
|
|
||||||
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
|
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
|
||||||
| ^^^^^^^^^^^
|
| ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
|
||||||
|
|
|
|
||||||
help: consider marking this type as Copy
|
help: consider marking this type as Copy
|
||||||
--> $DIR/needless_pass_by_value.rs:123:1
|
--> $DIR/needless_pass_by_value.rs:123:1
|
||||||
|
|
|
|
||||||
LL | struct CopyWrapper(u32);
|
LL | struct CopyWrapper(u32);
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
help: consider taking a reference instead
|
|
||||||
|
|
|
||||||
LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: &CopyWrapper) {
|
|
||||||
LL | let CopyWrapper(s) = *z; // moved
|
|
||||||
|
|
|
||||||
|
|
||||||
error: this argument is passed by value, but not consumed in the function body
|
error: this argument is passed by value, but not consumed in the function body
|
||||||
--> $DIR/needless_pass_by_value.rs:143:40
|
--> $DIR/needless_pass_by_value.rs:143:40
|
||||||
|
|
Loading…
Reference in a new issue