Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warning for unused variables in or pattern (issue #67691) #67766

Merged
merged 4 commits into from
Apr 13, 2020
+229 −52
Merged

Fix warning for unused variables in or pattern (issue #67691) #67766

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c156af
Add tests for issue #67691
sapir Dec 31, 2019
e22e443
Try to fix warning for unused variables in or patterns, issue #67691
sapir Dec 31, 2019
a8e3d0b
Replace non-shorthand variables with _, not _var
sapir Mar 6, 2020
3221638
Replace run-rustfix for issue 67691 test with a FIXME
sapir Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Try to fix warning for unused variables in or patterns, issue #67691
@sapir
sapir committed Apr 10, 2020
commit e22e443208b4e6e1f0bf430877b19ecbf7a3d0ca
71 changes: 45 additions & 26 deletions src/librustc_passes/liveness.rs
View file
Original file line number Diff line number Diff line change
@@ -1492,28 +1492,33 @@ impl<'tcx> Liveness<'_, 'tcx> {
) {
// In an or-pattern, only consider the variable; any later patterns must have the same
// bindings, and we also consider the first pattern to be the "authoritative" set of ids.
// However, we should take the spans of variables with the same name from the later
// However, we should take the ids and spans of variables with the same name from the later
// patterns so the suggestions to prefix with underscores will apply to those too.
let mut vars: FxIndexMap<String, (LiveNode, Variable, HirId, Vec<Span>)> = <_>::default();
let mut vars: FxIndexMap<String, (LiveNode, Variable, Vec<(HirId, Span)>)> = <_>::default();

pat.each_binding(|_, hir_id, pat_sp, ident| {
let ln = entry_ln.unwrap_or_else(|| self.live_node(hir_id, pat_sp));
let var = self.variable(hir_id, ident.span);
let id_and_sp = (hir_id, pat_sp);
vars.entry(self.ir.variable_name(var))
.and_modify(|(.., spans)| spans.push(ident.span))
.or_insert_with(|| (ln, var, hir_id, vec![ident.span]));
.and_modify(|(.., hir_ids_and_spans)| hir_ids_and_spans.push(id_and_sp))
.or_insert_with(|| (ln, var, vec![id_and_sp]));
});

for (_, (ln, var, id, spans)) in vars {
for (_, (ln, var, hir_ids_and_spans)) in vars {
if self.used_on_entry(ln, var) {
let id = hir_ids_and_spans[0].0;
let spans = hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect();
on_used_on_entry(spans, id, ln, var);
} else {
self.report_unused(spans, id, ln, var);
self.report_unused(hir_ids_and_spans, ln, var);
}
}
}

fn report_unused(&self, spans: Vec<Span>, hir_id: HirId, ln: LiveNode, var: Variable) {
fn report_unused(&self, hir_ids_and_spans: Vec<(HirId, Span)>, ln: LiveNode, var: Variable) {
let first_hir_id = hir_ids_and_spans[0].0;

if let Some(name) = self.should_warn(var).filter(|name| name != "self") {
// annoying: for parameters in funcs like `fn(x: i32)
// {ret}`, there is only one node, so asking about
@@ -1524,8 +1529,8 @@ impl<'tcx> Liveness<'_, 'tcx> {
if is_assigned {
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
hir_id,
spans,
first_hir_id,
hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect::<Vec<_>>(),
|lint| {
lint.build(&format!("variable `{}` is assigned to, but never used", name))
.note(&format!("consider using `_{}` instead", name))
@@ -1535,31 +1540,45 @@ impl<'tcx> Liveness<'_, 'tcx> {
} else {
self.ir.tcx.struct_span_lint_hir(
lint::builtin::UNUSED_VARIABLES,
hir_id,
spans.clone(),
first_hir_id,
hir_ids_and_spans.iter().map(|(_, sp)| *sp).collect::<Vec<_>>(),
|lint| {
let mut err = lint.build(&format!("unused variable: `{}`", name));
if self.ir.variable_is_shorthand(var) {
if let Node::Binding(pat) = self.ir.tcx.hir().get(hir_id) {
// Handle `ref` and `ref mut`.
let spans = spans
.iter()
.map(|_span| (pat.span, format!("{}: _", name)))
.collect();

err.multipart_suggestion(
"try ignoring the field",
spans,
Applicability::MachineApplicable,
);
}

let (shorthands, non_shorthands): (Vec<_>, Vec<_>) =
hir_ids_and_spans.into_iter().partition(|(hir_id, span)| {
let var = self.variable(*hir_id, *span);
self.ir.variable_is_shorthand(var)
});

let mut shorthands = shorthands
.into_iter()
.map(|(_, span)| (span, format!("{}: _", name)))
.collect::<Vec<_>>();

let non_shorthands = non_shorthands
.into_iter()
.map(|(_, span)| (span, format!("_{}", name)))
.collect::<Vec<_>>();

// If we have both shorthand and non-shorthand, prefer the "try ignoring
// the field" message.
if !shorthands.is_empty() {
shorthands.extend(non_shorthands);

err.multipart_suggestion(
"try ignoring the field",
shorthands,
Applicability::MachineApplicable,
);
} else {
err.multipart_suggestion(
"if this is intentional, prefix it with an underscore",
spans.iter().map(|span| (*span, format!("_{}", name))).collect(),
non_shorthands,
Applicability::MachineApplicable,
);
}

err.emit()
},
);
12 changes: 6 additions & 6 deletions src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr
View file
Original file line number Diff line number Diff line change
@@ -12,16 +12,16 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`

warning: unused variable: `mut_unused_var`
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:9
|
LL | let mut mut_unused_var = 1;
| ^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var`
| ^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var`

warning: unused variable: `var`
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:14
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:10
|
LL | let (mut var, unused_var) = (1, 2);
| ^^^ help: if this is intentional, prefix it with an underscore: `_var`
| ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_var`

warning: unused variable: `unused_var`
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:19
@@ -36,10 +36,10 @@ LL | if let SoulHistory { corridors_of_light,
| ^^^^^^^^^^^^^^^^^^ help: try ignoring the field: `corridors_of_light: _`

warning: variable `hours_are_suns` is assigned to, but never used
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:30
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:26
|
LL | mut hours_are_suns,
| ^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^
|
= note: consider using `_hours_are_suns` instead

18 changes: 6 additions & 12 deletions src/test/ui/lint/issue-54180-unused-ref-field.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
error: unused variable: `field`
--> $DIR/issue-54180-unused-ref-field.rs:20:26
--> $DIR/issue-54180-unused-ref-field.rs:20:22
|
LL | E::Variant { ref field } => (),
| ----^^^^^
| |
| help: try ignoring the field: `field: _`
| ^^^^^^^^^ help: try ignoring the field: `field: _`
|
note: the lint level is defined here
--> $DIR/issue-54180-unused-ref-field.rs:3:9
@@ -20,20 +18,16 @@ LL | let _: i32 = points.iter().map(|Point { x, y }| y).sum();
| ^ help: try ignoring the field: `x: _`

error: unused variable: `f1`
--> $DIR/issue-54180-unused-ref-field.rs:26:17
--> $DIR/issue-54180-unused-ref-field.rs:26:13
|
LL | let S { ref f1 } = s;
| ----^^
| |
| help: try ignoring the field: `f1: _`
| ^^^^^^ help: try ignoring the field: `f1: _`

error: unused variable: `x`
--> $DIR/issue-54180-unused-ref-field.rs:32:28
--> $DIR/issue-54180-unused-ref-field.rs:32:20
|
LL | Point { y, ref mut x } => y,
| --------^
| |
| help: try ignoring the field: `x: _`
| ^^^^^^^^^ help: try ignoring the field: `x: _`

error: aborting due to 4 previous errors

24 changes: 24 additions & 0 deletions src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed
View file
Original file line number Diff line number Diff line change
@@ -8,6 +8,11 @@ pub enum MyEnum {
B { i: i32, j: i32 },
}

pub enum MixedEnum {
A { i: i32 },
B(i32),
}

pub fn no_ref(x: MyEnum) {
use MyEnum::*;

@@ -52,10 +57,29 @@ pub fn inner_with_ref(x: Option<MyEnum>) {
}
}

pub fn mixed_no_ref(x: MixedEnum) {
match x {
MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
println!("match");
}
}
}

pub fn mixed_with_ref(x: MixedEnum) {
match x {
MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
println!("match");
}
}
}

pub fn main() {
no_ref(MyEnum::A { i: 1, j: 2 });
with_ref(MyEnum::A { i: 1, j: 2 });

inner_no_ref(Some(MyEnum::A { i: 1, j: 2 }));
inner_with_ref(Some(MyEnum::A { i: 1, j: 2 }));

mixed_no_ref(MixedEnum::B(5));
mixed_with_ref(MixedEnum::B(5));
}
24 changes: 24 additions & 0 deletions src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs
View file
Original file line number Diff line number Diff line change
@@ -8,6 +8,11 @@ pub enum MyEnum {
B { i: i32, j: i32 },
}

pub enum MixedEnum {
A { i: i32 },
B(i32),
}

pub fn no_ref(x: MyEnum) {
use MyEnum::*;

@@ -52,10 +57,29 @@ pub fn inner_with_ref(x: Option<MyEnum>) {
}
}

pub fn mixed_no_ref(x: MixedEnum) {
match x {
MixedEnum::A { i } | MixedEnum::B(i) => { //~ ERROR unused variable
println!("match");
}
}
}

pub fn mixed_with_ref(x: MixedEnum) {
match x {
MixedEnum::A { ref i } | MixedEnum::B(ref i) => { //~ ERROR unused variable
println!("match");
}
}
}

pub fn main() {
no_ref(MyEnum::A { i: 1, j: 2 });
with_ref(MyEnum::A { i: 1, j: 2 });

inner_no_ref(Some(MyEnum::A { i: 1, j: 2 }));
inner_with_ref(Some(MyEnum::A { i: 1, j: 2 }));

mixed_no_ref(MixedEnum::B(5));
mixed_with_ref(MixedEnum::B(5));
}
32 changes: 27 additions & 5 deletions src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: unused variable: `j`
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:15:16
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:20:16
|
LL | A { i, j } | B { i, j } => {
| ^ ^
@@ -16,7 +16,7 @@ LL | A { i, j: _ } | B { i, j: _ } => {
| ^^^^ ^^^^

error: unused variable: `j`
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:25:16
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:30:16
|
LL | A { i, ref j } | B { i, ref j } => {
| ^^^^^ ^^^^^
@@ -27,7 +27,7 @@ LL | A { i, j: _ } | B { i, j: _ } => {
| ^^^^ ^^^^

error: unused variable: `j`
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:35:21
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:40:21
|
LL | Some(A { i, j } | B { i, j }) => {
| ^ ^
@@ -38,7 +38,7 @@ LL | Some(A { i, j: _ } | B { i, j: _ }) => {
| ^^^^ ^^^^

error: unused variable: `j`
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:47:21
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:52:21
|
LL | Some(A { i, ref j } | B { i, ref j }) => {
| ^^^^^ ^^^^^
@@ -48,5 +48,27 @@ help: try ignoring the field
LL | Some(A { i, j: _ } | B { i, j: _ }) => {
| ^^^^ ^^^^

error: aborting due to 5 previous errors
error: unused variable: `i`
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:62:24
|
LL | MixedEnum::A { i } | MixedEnum::B(i) => {
| ^ ^
|
help: try ignoring the field
|
LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
| ^^^^ ^^

error: unused variable: `i`
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24
|
LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => {
| ^^^^^ ^^^^^
|
help: try ignoring the field
|
LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
| ^^^^ ^^

error: aborting due to 6 previous errors

8 changes: 4 additions & 4 deletions src/test/ui/liveness/liveness-dead.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: value assigned to `x` is never read
--> $DIR/liveness-dead.rs:9:13
--> $DIR/liveness-dead.rs:9:9
|
LL | let mut x: isize = 3;
| ^
| ^^^^^
|
note: the lint level is defined here
--> $DIR/liveness-dead.rs:2:9
@@ -20,10 +20,10 @@ LL | x = 4;
= help: maybe it is overwritten before being read?

error: value passed to `x` is never read
--> $DIR/liveness-dead.rs:20:11
--> $DIR/liveness-dead.rs:20:7
|
LL | fn f4(mut x: i32) {
| ^
| ^^^^^
|
= help: maybe it is overwritten before being read?

8 changes: 4 additions & 4 deletions src/test/ui/liveness/liveness-unused.stderr
View file
Original file line number Diff line number Diff line change
@@ -44,10 +44,10 @@ LL | let x = 3;
| ^ help: if this is intentional, prefix it with an underscore: `_x`

error: variable `x` is assigned to, but never used
--> $DIR/liveness-unused.rs:30:13
--> $DIR/liveness-unused.rs:30:9
|
LL | let mut x = 3;
| ^
| ^^^^^
|
= note: consider using `_x` instead

@@ -65,10 +65,10 @@ LL | #![deny(unused_assignments)]
= help: maybe it is overwritten before being read?

error: variable `z` is assigned to, but never used
--> $DIR/liveness-unused.rs:37:13
--> $DIR/liveness-unused.rs:37:9
|
LL | let mut z = 3;
| ^
| ^^^^^
|
= note: consider using `_z` instead