From 0c156af20d74b2e23c2b862110a5ed5fa8d65a51 Mon Sep 17 00:00:00 2001 From: sapir Date: Wed, 1 Jan 2020 01:51:37 +0200 Subject: [PATCH 01/10] Add tests for issue #67691 --- ...sue-67691-unused-field-in-or-pattern.fixed | 61 +++++++++++++++++++ .../issue-67691-unused-field-in-or-pattern.rs | 61 +++++++++++++++++++ ...ue-67691-unused-field-in-or-pattern.stderr | 52 ++++++++++++++++ 3 files changed, 174 insertions(+) create mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed create mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs create mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed new file mode 100644 index 0000000000000..7ec0b33f919b7 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed @@ -0,0 +1,61 @@ +// run-rustfix + +#![feature(or_patterns)] +#![deny(unused)] + +pub enum MyEnum { + A { i: i32, j: i32 }, + B { i: i32, j: i32 }, +} + +pub fn no_ref(x: MyEnum) { + use MyEnum::*; + + match x { + A { i, j: _ } | B { i, j: _ } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn with_ref(x: MyEnum) { + use MyEnum::*; + + match x { + A { i, j: _ } | B { i, j: _ } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn inner_no_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +pub fn inner_with_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +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 })); +} diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs new file mode 100644 index 0000000000000..d43c637767ad9 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs @@ -0,0 +1,61 @@ +// run-rustfix + +#![feature(or_patterns)] +#![deny(unused)] + +pub enum MyEnum { + A { i: i32, j: i32 }, + B { i: i32, j: i32 }, +} + +pub fn no_ref(x: MyEnum) { + use MyEnum::*; + + match x { + A { i, j } | B { i, j } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn with_ref(x: MyEnum) { + use MyEnum::*; + + match x { + A { i, ref j } | B { i, ref j } => { //~ ERROR unused variable + println!("{}", i); + } + } +} + +pub fn inner_no_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, j } | B { i, j }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +pub fn inner_with_ref(x: Option) { + use MyEnum::*; + + match x { + Some(A { i, ref j } | B { i, ref j }) => { //~ ERROR unused variable + println!("{}", i); + } + + _ => {} + } +} + +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 })); +} diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr new file mode 100644 index 0000000000000..bda53c6733183 --- /dev/null +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr @@ -0,0 +1,52 @@ +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:15:16 + | +LL | A { i, j } | B { i, j } => { + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:4:9 + | +LL | #![deny(unused)] + | ^^^^^^ + = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]` +help: try ignoring the field + | +LL | A { i, j: _ } | B { i, j: _ } => { + | ^^^^ ^^^^ + +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:25:16 + | +LL | A { i, ref j } | B { i, ref j } => { + | ^^^^^ ^^^^^ + | +help: try ignoring the field + | +LL | A { i, j: _ } | B { i, j: _ } => { + | ^^^^ ^^^^ + +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:35:21 + | +LL | Some(A { i, j } | B { i, j }) => { + | ^ ^ + | +help: try ignoring the field + | +LL | Some(A { i, j: _ } | B { i, j: _ }) => { + | ^^^^ ^^^^ + +error: unused variable: `j` + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:47:21 + | +LL | Some(A { i, ref j } | B { i, ref j }) => { + | ^^^^^ ^^^^^ + | +help: try ignoring the field + | +LL | Some(A { i, j: _ } | B { i, j: _ }) => { + | ^^^^ ^^^^ + +error: aborting due to 5 previous errors + From e22e443208b4e6e1f0bf430877b19ecbf7a3d0ca Mon Sep 17 00:00:00 2001 From: sapir Date: Wed, 1 Jan 2020 01:20:34 +0200 Subject: [PATCH 02/10] Try to fix warning for unused variables in or patterns, issue #67691 --- src/librustc_passes/liveness.rs | 71 ++++++++++++------- ...0-unused-variable-in-struct-pattern.stderr | 12 ++-- .../lint/issue-54180-unused-ref-field.stderr | 18 ++--- ...sue-67691-unused-field-in-or-pattern.fixed | 24 +++++++ .../issue-67691-unused-field-in-or-pattern.rs | 24 +++++++ ...ue-67691-unused-field-in-or-pattern.stderr | 32 +++++++-- src/test/ui/liveness/liveness-dead.stderr | 8 +-- src/test/ui/liveness/liveness-unused.stderr | 8 +-- 8 files changed, 140 insertions(+), 57 deletions(-) diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index ee71d09cb21e9..74a2d99789680 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -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)> = <_>::default(); + let mut vars: FxIndexMap)> = <_>::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, 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::>(), |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::>(), |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::>(); + + let non_shorthands = non_shorthands + .into_iter() + .map(|(_, span)| (span, format!("_{}", name))) + .collect::>(); + + // 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() }, ); diff --git a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr index cc675a709a2c6..413a51d4e5dd7 100644 --- a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr +++ b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr @@ -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 diff --git a/src/test/ui/lint/issue-54180-unused-ref-field.stderr b/src/test/ui/lint/issue-54180-unused-ref-field.stderr index 840ecc0ce09ee..c501aa25f1352 100644 --- a/src/test/ui/lint/issue-54180-unused-ref-field.stderr +++ b/src/test/ui/lint/issue-54180-unused-ref-field.stderr @@ -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 diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed index 7ec0b33f919b7..cfe3760620205 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed @@ -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) { } } +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)); } diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs index d43c637767ad9..4b19f85dfa477 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs @@ -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) { } } +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)); } diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr index bda53c6733183..4e9d02abacd71 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr @@ -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 diff --git a/src/test/ui/liveness/liveness-dead.stderr b/src/test/ui/liveness/liveness-dead.stderr index 12680ab11568f..e9d20cf981fbd 100644 --- a/src/test/ui/liveness/liveness-dead.stderr +++ b/src/test/ui/liveness/liveness-dead.stderr @@ -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? diff --git a/src/test/ui/liveness/liveness-unused.stderr b/src/test/ui/liveness/liveness-unused.stderr index 42187330a3eb1..1ea8461439320 100644 --- a/src/test/ui/liveness/liveness-unused.stderr +++ b/src/test/ui/liveness/liveness-unused.stderr @@ -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 From a8e3d0b71e2b79b0ce880541e5a0f0fe303a1a8d Mon Sep 17 00:00:00 2001 From: sapir Date: Fri, 6 Mar 2020 10:03:34 +0200 Subject: [PATCH 03/10] Replace non-shorthand variables with _, not _var --- src/librustc_passes/liveness.rs | 20 +++++++++++-------- ...sue-67691-unused-field-in-or-pattern.fixed | 4 ++-- ...ue-67691-unused-field-in-or-pattern.stderr | 8 ++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index 74a2d99789680..24f6d1a9c5894 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -1556,15 +1556,16 @@ impl<'tcx> Liveness<'_, 'tcx> { .map(|(_, span)| (span, format!("{}: _", name))) .collect::>(); - let non_shorthands = non_shorthands - .into_iter() - .map(|(_, span)| (span, format!("_{}", name))) - .collect::>(); - // If we have both shorthand and non-shorthand, prefer the "try ignoring - // the field" message. + // the field" message, and suggest `_` for the non-shorthands. If we only + // have non-shorthand, then prefix with an underscore instead. if !shorthands.is_empty() { - shorthands.extend(non_shorthands); + shorthands.extend( + non_shorthands + .into_iter() + .map(|(_, span)| (span, "_".to_string())) + .collect::>(), + ); err.multipart_suggestion( "try ignoring the field", @@ -1574,7 +1575,10 @@ impl<'tcx> Liveness<'_, 'tcx> { } else { err.multipart_suggestion( "if this is intentional, prefix it with an underscore", - non_shorthands, + non_shorthands + .into_iter() + .map(|(_, span)| (span, format!("_{}", name))) + .collect::>(), Applicability::MachineApplicable, ); } diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed index cfe3760620205..f842fcebe1f8f 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed @@ -59,7 +59,7 @@ pub fn inner_with_ref(x: Option) { pub fn mixed_no_ref(x: MixedEnum) { match x { - MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + MixedEnum::A { i: _ } | MixedEnum::B(_) => { println!("match"); } } @@ -67,7 +67,7 @@ pub fn mixed_no_ref(x: MixedEnum) { pub fn mixed_with_ref(x: MixedEnum) { match x { - MixedEnum::A { i: _ } | MixedEnum::B(_i) => { + MixedEnum::A { i: _ } | MixedEnum::B(_) => { println!("match"); } } diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr index 4e9d02abacd71..8aefe243a944b 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr @@ -56,8 +56,8 @@ LL | MixedEnum::A { i } | MixedEnum::B(i) => { | help: try ignoring the field | -LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => { - | ^^^^ ^^ +LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { + | ^^^^ ^ error: unused variable: `i` --> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24 @@ -67,8 +67,8 @@ LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => { | help: try ignoring the field | -LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => { - | ^^^^ ^^ +LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { + | ^^^^ ^ error: aborting due to 6 previous errors From 32216383fa2f16047f1130f1b7bc6dfea0272272 Mon Sep 17 00:00:00 2001 From: sapir Date: Fri, 10 Apr 2020 03:32:49 +0300 Subject: [PATCH 04/10] Replace run-rustfix for issue 67691 test with a FIXME --- ...sue-67691-unused-field-in-or-pattern.fixed | 85 ------------------- .../issue-67691-unused-field-in-or-pattern.rs | 3 +- ...ue-67691-unused-field-in-or-pattern.stderr | 14 +-- 3 files changed, 9 insertions(+), 93 deletions(-) delete mode 100644 src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed deleted file mode 100644 index f842fcebe1f8f..0000000000000 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed +++ /dev/null @@ -1,85 +0,0 @@ -// run-rustfix - -#![feature(or_patterns)] -#![deny(unused)] - -pub enum MyEnum { - A { i: i32, j: i32 }, - B { i: i32, j: i32 }, -} - -pub enum MixedEnum { - A { i: i32 }, - B(i32), -} - -pub fn no_ref(x: MyEnum) { - use MyEnum::*; - - match x { - A { i, j: _ } | B { i, j: _ } => { //~ ERROR unused variable - println!("{}", i); - } - } -} - -pub fn with_ref(x: MyEnum) { - use MyEnum::*; - - match x { - A { i, j: _ } | B { i, j: _ } => { //~ ERROR unused variable - println!("{}", i); - } - } -} - -pub fn inner_no_ref(x: Option) { - use MyEnum::*; - - match x { - Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable - println!("{}", i); - } - - _ => {} - } -} - -pub fn inner_with_ref(x: Option) { - use MyEnum::*; - - match x { - Some(A { i, j: _ } | B { i, j: _ }) => { //~ ERROR unused variable - println!("{}", i); - } - - _ => {} - } -} - -pub fn mixed_no_ref(x: MixedEnum) { - match x { - MixedEnum::A { i: _ } | MixedEnum::B(_) => { - println!("match"); - } - } -} - -pub fn mixed_with_ref(x: MixedEnum) { - match x { - MixedEnum::A { i: _ } | MixedEnum::B(_) => { - 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)); -} diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs index 4b19f85dfa477..b21f1ef6b2603 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs @@ -1,4 +1,5 @@ -// run-rustfix +// FIXME: should be run-rustfix, but rustfix doesn't currently support multipart suggestions, see +// #53934 #![feature(or_patterns)] #![deny(unused)] diff --git a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr index 8aefe243a944b..9cff2900908e6 100644 --- a/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr +++ b/src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr @@ -1,11 +1,11 @@ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:20:16 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:21:16 | LL | A { i, j } | B { i, j } => { | ^ ^ | note: the lint level is defined here - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:4:9 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:5:9 | LL | #![deny(unused)] | ^^^^^^ @@ -16,7 +16,7 @@ LL | A { i, j: _ } | B { i, j: _ } => { | ^^^^ ^^^^ error: unused variable: `j` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:30:16 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:31: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:40:21 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:41: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:52:21 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:53:21 | LL | Some(A { i, ref j } | B { i, ref j }) => { | ^^^^^ ^^^^^ @@ -49,7 +49,7 @@ LL | Some(A { i, j: _ } | B { i, j: _ }) => { | ^^^^ ^^^^ error: unused variable: `i` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:62:24 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:63:24 | LL | MixedEnum::A { i } | MixedEnum::B(i) => { | ^ ^ @@ -60,7 +60,7 @@ LL | MixedEnum::A { i: _ } | MixedEnum::B(_) => { | ^^^^ ^ error: unused variable: `i` - --> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24 + --> $DIR/issue-67691-unused-field-in-or-pattern.rs:71:24 | LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => { | ^^^^^ ^^^^^ From 72ae73ae6119b9726f0f0b66da0445ca039e8059 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 11 Apr 2020 01:39:50 +0200 Subject: [PATCH 05/10] Pass the `PlaceElem::Index` local to `visit_local` --- src/librustc_middle/mir/visit.rs | 59 +++++++++++--------- src/librustc_mir/borrow_check/renumber.rs | 6 +- src/librustc_mir/transform/generator.rs | 7 --- src/librustc_mir/transform/inline.rs | 12 ---- src/librustc_mir/transform/promote_consts.rs | 9 --- src/librustc_mir/transform/simplify.rs | 7 --- src/librustc_mir/util/def_use.rs | 13 +---- 7 files changed, 39 insertions(+), 74 deletions(-) diff --git a/src/librustc_middle/mir/visit.rs b/src/librustc_middle/mir/visit.rs index 400d15cdc144b..5c33db299ae85 100644 --- a/src/librustc_middle/mir/visit.rs +++ b/src/librustc_middle/mir/visit.rs @@ -838,7 +838,7 @@ macro_rules! make_mir_visitor { } macro_rules! visit_place_fns { - (mut) => ( + (mut) => { fn tcx<'a>(&'a self) -> TyCtxt<'tcx>; fn super_place( @@ -849,7 +849,7 @@ macro_rules! visit_place_fns { ) { self.visit_place_base(&mut place.local, context, location); - if let Some(new_projection) = self.process_projection(&place.projection) { + if let Some(new_projection) = self.process_projection(&place.projection, location) { place.projection = self.tcx().intern_place_elems(&new_projection); } } @@ -857,12 +857,13 @@ macro_rules! visit_place_fns { fn process_projection( &mut self, projection: &'a [PlaceElem<'tcx>], + location: Location, ) -> Option>> { let mut projection = Cow::Borrowed(projection); for i in 0..projection.len() { if let Some(elem) = projection.get(i) { - if let Some(elem) = self.process_projection_elem(elem) { + if let Some(elem) = self.process_projection_elem(elem, location) { // This converts the borrowed projection into `Cow::Owned(_)` and returns a // clone of the projection so we can mutate and reintern later. let vec = projection.to_mut(); @@ -879,13 +880,30 @@ macro_rules! visit_place_fns { fn process_projection_elem( &mut self, - _elem: &PlaceElem<'tcx>, + elem: &PlaceElem<'tcx>, + location: Location, ) -> Option> { - None + match elem { + PlaceElem::Index(local) => { + let mut new_local = *local; + self.visit_local( + &mut new_local, + PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), + location, + ); + + if new_local == *local { None } else { Some(PlaceElem::Index(new_local)) } + } + PlaceElem::Deref + | PlaceElem::Field(..) + | PlaceElem::ConstantIndex { .. } + | PlaceElem::Subslice { .. } + | PlaceElem::Downcast(..) => None, + } } - ); + }; - () => ( + () => { fn visit_projection( &mut self, local: Local, @@ -907,12 +925,7 @@ macro_rules! visit_place_fns { self.super_projection_elem(local, proj_base, elem, context, location); } - fn super_place( - &mut self, - place: &Place<'tcx>, - context: PlaceContext, - location: Location, - ) { + fn super_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) { let mut context = context; if !place.projection.is_empty() { @@ -925,10 +938,7 @@ macro_rules! visit_place_fns { self.visit_place_base(&place.local, context, location); - self.visit_projection(place.local, - &place.projection, - context, - location); + self.visit_projection(place.local, &place.projection, context, location); } fn super_projection( @@ -961,19 +971,16 @@ macro_rules! visit_place_fns { self.visit_local( local, PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), - location + location, ); } - ProjectionElem::Deref | - ProjectionElem::Subslice { from: _, to: _, from_end: _ } | - ProjectionElem::ConstantIndex { offset: _, - min_length: _, - from_end: _ } | - ProjectionElem::Downcast(_, _) => { - } + ProjectionElem::Deref + | ProjectionElem::Subslice { from: _, to: _, from_end: _ } + | ProjectionElem::ConstantIndex { offset: _, min_length: _, from_end: _ } + | ProjectionElem::Downcast(_, _) => {} } } - ); + }; } make_mir_visitor!(Visitor,); diff --git a/src/librustc_mir/borrow_check/renumber.rs b/src/librustc_mir/borrow_check/renumber.rs index a1d7bc1462f95..a6b4aa7497722 100644 --- a/src/librustc_mir/borrow_check/renumber.rs +++ b/src/librustc_mir/borrow_check/renumber.rs @@ -64,7 +64,11 @@ impl<'a, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'tcx> { debug!("visit_ty: ty={:?}", ty); } - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { + fn process_projection_elem( + &mut self, + elem: &PlaceElem<'tcx>, + _: Location, + ) -> Option> { if let PlaceElem::Field(field, ty) = elem { let new_ty = self.renumber_regions(ty); diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index d25fd8ebb0c2b..bc3c20d9f409a 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -89,13 +89,6 @@ impl<'tcx> MutVisitor<'tcx> for RenameLocalVisitor<'tcx> { *local = self.to; } } - - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { - match elem { - PlaceElem::Index(local) if *local == self.from => Some(PlaceElem::Index(self.to)), - _ => None, - } - } } struct DerefArgVisitor<'tcx> { diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 157dada831a2e..8121d4ead1394 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -706,18 +706,6 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { self.super_place(place, context, location) } - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { - if let PlaceElem::Index(local) = elem { - let new_local = self.make_integrate_local(*local); - - if new_local != *local { - return Some(PlaceElem::Index(new_local)); - } - } - - None - } - fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) { self.in_cleanup_block = data.is_cleanup; self.super_basic_block_data(block, data); diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index ec0b89ebb4d0a..9579fe1f405ba 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -1036,15 +1036,6 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> { *local = self.promote_temp(*local); } } - - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { - match elem { - PlaceElem::Index(local) if self.is_temp_kind(*local) => { - Some(PlaceElem::Index(self.promote_temp(*local))) - } - _ => None, - } - } } pub fn promote_candidates<'tcx>( diff --git a/src/librustc_mir/transform/simplify.rs b/src/librustc_mir/transform/simplify.rs index c2029a223b941..c0da2c446d65f 100644 --- a/src/librustc_mir/transform/simplify.rs +++ b/src/librustc_mir/transform/simplify.rs @@ -417,11 +417,4 @@ impl<'tcx> MutVisitor<'tcx> for LocalUpdater<'tcx> { fn visit_local(&mut self, l: &mut Local, _: PlaceContext, _: Location) { *l = self.map[*l].unwrap(); } - - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { - match elem { - PlaceElem::Index(local) => Some(PlaceElem::Index(self.map[*local].unwrap())), - _ => None, - } - } } diff --git a/src/librustc_mir/util/def_use.rs b/src/librustc_mir/util/def_use.rs index 6b5f6aa991c1e..0ac743359be96 100644 --- a/src/librustc_mir/util/def_use.rs +++ b/src/librustc_mir/util/def_use.rs @@ -2,9 +2,7 @@ use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; -use rustc_middle::mir::{ - Body, BodyAndCache, Local, Location, PlaceElem, ReadOnlyBodyAndCache, VarDebugInfo, -}; +use rustc_middle::mir::{Body, BodyAndCache, Local, Location, ReadOnlyBodyAndCache, VarDebugInfo}; use rustc_middle::ty::TyCtxt; use std::mem; @@ -157,13 +155,4 @@ impl MutVisitor<'tcx> for MutateUseVisitor<'tcx> { *local = self.new_local; } } - - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { - match elem { - PlaceElem::Index(local) if *local == self.query => { - Some(PlaceElem::Index(self.new_local)) - } - _ => None, - } - } } From 57ed3d378d2430b0db296c3b37a81e69cca76e8c Mon Sep 17 00:00:00 2001 From: David Renshaw Date: Sun, 12 Apr 2020 11:36:37 -0400 Subject: [PATCH 06/10] fix issue 69130 --- src/librustc_errors/lib.rs | 5 ++++- .../miri_unleashed/mutable_const2.stderr | 2 +- src/test/ui/issues/issue-69130.rs | 7 +++++++ src/test/ui/issues/issue-69130.stderr | 21 +++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/issues/issue-69130.rs create mode 100644 src/test/ui/issues/issue-69130.stderr diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 55eb9fd566d87..151241fdb0b5f 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -231,7 +231,10 @@ impl CodeSuggestion { } } if let Some(cur_line) = sf.get_line(cur_lo.line - 1) { - let end = std::cmp::min(cur_line.len(), cur_lo.col.to_usize()); + let end = match cur_line.char_indices().nth(cur_lo.col.to_usize()) { + Some((i, _)) => i, + None => cur_line.len(), + }; buf.push_str(&cur_line[..end]); } } diff --git a/src/test/ui/consts/miri_unleashed/mutable_const2.stderr b/src/test/ui/consts/miri_unleashed/mutable_const2.stderr index 1699223f74f43..39027dd2b4103 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const2.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_const2.stderr @@ -12,7 +12,7 @@ error: internal compiler error: mutable allocation in constant LL | const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:363:17 +thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:366:17 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace error: internal compiler error: unexpected panic diff --git a/src/test/ui/issues/issue-69130.rs b/src/test/ui/issues/issue-69130.rs new file mode 100644 index 0000000000000..9552e8ec2a876 --- /dev/null +++ b/src/test/ui/issues/issue-69130.rs @@ -0,0 +1,7 @@ +// Issue 69130: character indexing bug in rustc_errors::CodeSuggestion::splice_lines(). + +enum F { +M (§& u8)} +//~^ ERROR unknown start of token +//~| missing lifetime specifier +fn main() {} diff --git a/src/test/ui/issues/issue-69130.stderr b/src/test/ui/issues/issue-69130.stderr new file mode 100644 index 0000000000000..a4700a5ed1da0 --- /dev/null +++ b/src/test/ui/issues/issue-69130.stderr @@ -0,0 +1,21 @@ +error: unknown start of token: \u{a7} + --> $DIR/issue-69130.rs:4:4 + | +LL | M (§& u8)} + | ^ + +error[E0106]: missing lifetime specifier + --> $DIR/issue-69130.rs:4:5 + | +LL | M (§& u8)} + | ^ expected named lifetime parameter + | +help: consider introducing a named lifetime parameter + | +LL | enum F<'a> { +LL | M (§&'a u8)} + | + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0106`. From 812854cdd313c0644534ce50b22d2a1c4fbe4957 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 12 Apr 2020 18:42:45 +0100 Subject: [PATCH 07/10] Remove usage of `DUMMY_HIR_ID` in calls to `ObligationClause::misc` Use `ObligationClause::dummy()` when appropriate or replace `hir::DUMMY_HIR_ID` by `hir::CRATE_HIR_ID`, as used in `ObligationClause::dummy()`. --- src/librustc_trait_selection/traits/auto_trait.rs | 12 +++--------- src/librustc_trait_selection/traits/mod.rs | 6 +++--- src/librustc_traits/normalize_projection_ty.rs | 4 +--- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/librustc_trait_selection/traits/auto_trait.rs b/src/librustc_trait_selection/traits/auto_trait.rs index b4c790eebc106..052de4a4e5b55 100644 --- a/src/librustc_trait_selection/traits/auto_trait.rs +++ b/src/librustc_trait_selection/traits/auto_trait.rs @@ -187,13 +187,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { // to store all of the necessary region/lifetime bounds in the InferContext, as well as // an additional sanity check. let mut fulfill = FulfillmentContext::new(); - fulfill.register_bound( - &infcx, - full_env, - ty, - trait_did, - ObligationCause::misc(DUMMY_SP, hir::DUMMY_HIR_ID), - ); + fulfill.register_bound(&infcx, full_env, ty, trait_did, ObligationCause::dummy()); fulfill.select_all_or_error(&infcx).unwrap_or_else(|e| { panic!("Unable to fulfill trait {:?} for '{:?}': {:?}", trait_did, ty, e) }); @@ -292,7 +286,7 @@ impl AutoTraitFinder<'tcx> { user_env.caller_bounds.iter().cloned().collect(); let mut new_env = param_env; - let dummy_cause = ObligationCause::misc(DUMMY_SP, hir::DUMMY_HIR_ID); + let dummy_cause = ObligationCause::dummy(); while let Some(pred) = predicates.pop_front() { infcx.clear_caches(); @@ -615,7 +609,7 @@ impl AutoTraitFinder<'tcx> { select: &mut SelectionContext<'_, 'tcx>, only_projections: bool, ) -> bool { - let dummy_cause = ObligationCause::misc(DUMMY_SP, hir::DUMMY_HIR_ID); + let dummy_cause = ObligationCause::dummy(); for (obligation, mut predicate) in nested.map(|o| (o.clone(), o.predicate)) { let is_new_pred = fresh_preds.insert(self.clean_pred(select.infcx(), predicate)); diff --git a/src/librustc_trait_selection/traits/mod.rs b/src/librustc_trait_selection/traits/mod.rs index a9d0f35fb276a..f5f4a51eb54e2 100644 --- a/src/librustc_trait_selection/traits/mod.rs +++ b/src/librustc_trait_selection/traits/mod.rs @@ -31,7 +31,7 @@ use rustc_middle::middle::region; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::subst::{InternalSubsts, SubstsRef}; use rustc_middle::ty::{self, GenericParamDefKind, ToPredicate, Ty, TyCtxt, WithConstness}; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::Span; use std::fmt::Debug; @@ -136,7 +136,7 @@ pub fn type_known_to_meet_bound_modulo_regions<'a, 'tcx>( let trait_ref = ty::TraitRef { def_id, substs: infcx.tcx.mk_substs_trait(ty, &[]) }; let obligation = Obligation { param_env, - cause: ObligationCause::misc(span, hir::DUMMY_HIR_ID), + cause: ObligationCause::misc(span, hir::CRATE_HIR_ID), recursion_depth: 0, predicate: trait_ref.without_const().to_predicate(), }; @@ -163,7 +163,7 @@ pub fn type_known_to_meet_bound_modulo_regions<'a, 'tcx>( // We can use a dummy node-id here because we won't pay any mind // to region obligations that arise (there shouldn't really be any // anyhow). - let cause = ObligationCause::misc(span, hir::DUMMY_HIR_ID); + let cause = ObligationCause::misc(span, hir::CRATE_HIR_ID); fulfill_cx.register_bound(infcx, param_env, ty, def_id, cause); diff --git a/src/librustc_traits/normalize_projection_ty.rs b/src/librustc_traits/normalize_projection_ty.rs index 11c97b03c44c3..d6d3e86a2c8d3 100644 --- a/src/librustc_traits/normalize_projection_ty.rs +++ b/src/librustc_traits/normalize_projection_ty.rs @@ -1,10 +1,8 @@ -use rustc_hir as hir; use rustc_infer::infer::canonical::{Canonical, QueryResponse}; use rustc_infer::infer::TyCtxtInferExt; use rustc_infer::traits::TraitEngineExt as _; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{ParamEnvAnd, TyCtxt}; -use rustc_span::DUMMY_SP; use rustc_trait_selection::infer::InferCtxtBuilderExt; use rustc_trait_selection::traits::query::{ normalize::NormalizationResult, CanonicalProjectionGoal, NoSolution, @@ -27,7 +25,7 @@ fn normalize_projection_ty<'tcx>( &goal, |infcx, fulfill_cx, ParamEnvAnd { param_env, value: goal }| { let selcx = &mut SelectionContext::new(infcx); - let cause = ObligationCause::misc(DUMMY_SP, hir::DUMMY_HIR_ID); + let cause = ObligationCause::dummy(); let mut obligations = vec![]; let answer = traits::normalize_projection_type( selcx, From b4edda98495a672e472d9510399d6a2f680a0f26 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 12 Apr 2020 19:20:07 +0100 Subject: [PATCH 08/10] Remove usage of `DUMMY_HIR_ID` in `CheckLoopVisitor` --- src/librustc_passes/loops.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_passes/loops.rs b/src/librustc_passes/loops.rs index 0fce08192bb3e..23a605bef0cd3 100644 --- a/src/librustc_passes/loops.rs +++ b/src/librustc_passes/loops.rs @@ -77,31 +77,31 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> { } let loop_id = match label.target_id { - Ok(loop_id) => loop_id, - Err(hir::LoopIdError::OutsideLoopScope) => hir::DUMMY_HIR_ID, + Ok(loop_id) => Some(loop_id), + Err(hir::LoopIdError::OutsideLoopScope) => None, Err(hir::LoopIdError::UnlabeledCfInWhileCondition) => { self.emit_unlabled_cf_in_while_condition(e.span, "break"); - hir::DUMMY_HIR_ID + None } - Err(hir::LoopIdError::UnresolvedLabel) => hir::DUMMY_HIR_ID, + Err(hir::LoopIdError::UnresolvedLabel) => None, }; - if loop_id != hir::DUMMY_HIR_ID { + if let Some(loop_id) = loop_id { if let Node::Block(_) = self.hir_map.find(loop_id).unwrap() { return; } } if opt_expr.is_some() { - let loop_kind = if loop_id == hir::DUMMY_HIR_ID { - None - } else { + let loop_kind = if let Some(loop_id) = loop_id { Some(match self.hir_map.expect_expr(loop_id).kind { hir::ExprKind::Loop(_, _, source) => source, ref r => { span_bug!(e.span, "break label resolved to a non-loop: {:?}", r) } }) + } else { + None }; match loop_kind { None | Some(hir::LoopSource::Loop) => (), From 502ae0e8988c1fa3ffc109c0d924e20c85f9426d Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 12 Apr 2020 20:57:43 +0100 Subject: [PATCH 09/10] Remove usage of `DUMMY_HIR_ID` in `CheckAttrVisitor::check_inline` --- src/librustc_passes/check_attr.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustc_passes/check_attr.rs b/src/librustc_passes/check_attr.rs index 3f2c02f6c461b..6c9d25cfaa54b 100644 --- a/src/librustc_passes/check_attr.rs +++ b/src/librustc_passes/check_attr.rs @@ -14,7 +14,6 @@ use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; -use rustc_hir::DUMMY_HIR_ID; use rustc_hir::{self, HirId, Item, ItemKind, TraitItem}; use rustc_hir::{MethodKind, Target}; use rustc_session::lint::builtin::{CONFLICTING_REPR_HINTS, UNUSED_ATTRIBUTES}; @@ -360,7 +359,7 @@ impl CheckAttrVisitor<'tcx> { if let hir::StmtKind::Local(ref l) = stmt.kind { for attr in l.attrs.iter() { if attr.check_name(sym::inline) { - self.check_inline(DUMMY_HIR_ID, attr, &stmt.span, Target::Statement); + self.check_inline(l.hir_id, attr, &stmt.span, Target::Statement); } if attr.check_name(sym::repr) { self.emit_repr_error( @@ -381,7 +380,7 @@ impl CheckAttrVisitor<'tcx> { }; for attr in expr.attrs.iter() { if attr.check_name(sym::inline) { - self.check_inline(DUMMY_HIR_ID, attr, &expr.span, target); + self.check_inline(expr.hir_id, attr, &expr.span, target); } if attr.check_name(sym::repr) { self.emit_repr_error( From 0634789dee9fadf31bd0fe94898eb5937c130f77 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 12 Apr 2020 20:59:13 +0100 Subject: [PATCH 10/10] Remove usage of `DUMMY_HIR_ID` in `Scope::hir_id` --- src/librustc_infer/infer/error_reporting/mod.rs | 3 ++- src/librustc_middle/middle/region.rs | 17 ++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/librustc_infer/infer/error_reporting/mod.rs b/src/librustc_infer/infer/error_reporting/mod.rs index 942d76e3202b9..db81ceea43f01 100644 --- a/src/librustc_infer/infer/error_reporting/mod.rs +++ b/src/librustc_infer/infer/error_reporting/mod.rs @@ -93,7 +93,8 @@ pub(super) fn note_and_explain_region( let unknown_scope = || format!("{}unknown scope: {:?}{}. Please report a bug.", prefix, scope, suffix); let span = scope.span(tcx, region_scope_tree); - let tag = match tcx.hir().find(scope.hir_id(region_scope_tree)) { + let hir_id = scope.hir_id(region_scope_tree); + let tag = match hir_id.and_then(|hir_id| tcx.hir().find(hir_id)) { Some(Node::Block(_)) => "block", Some(Node::Expr(expr)) => match expr.kind { hir::ExprKind::Call(..) => "call", diff --git a/src/librustc_middle/middle/region.rs b/src/librustc_middle/middle/region.rs index dd9ab102129e6..2ad6fe14ec716 100644 --- a/src/librustc_middle/middle/region.rs +++ b/src/librustc_middle/middle/region.rs @@ -159,21 +159,20 @@ impl Scope { self.id } - pub fn hir_id(&self, scope_tree: &ScopeTree) -> hir::HirId { - match scope_tree.root_body { - Some(hir_id) => hir::HirId { owner: hir_id.owner, local_id: self.item_local_id() }, - None => hir::DUMMY_HIR_ID, - } + pub fn hir_id(&self, scope_tree: &ScopeTree) -> Option { + scope_tree + .root_body + .map(|hir_id| hir::HirId { owner: hir_id.owner, local_id: self.item_local_id() }) } /// Returns the span of this `Scope`. Note that in general the /// returned span may not correspond to the span of any `NodeId` in /// the AST. pub fn span(&self, tcx: TyCtxt<'_>, scope_tree: &ScopeTree) -> Span { - let hir_id = self.hir_id(scope_tree); - if hir_id == hir::DUMMY_HIR_ID { - return DUMMY_SP; - } + let hir_id = match self.hir_id(scope_tree) { + Some(hir_id) => hir_id, + None => return DUMMY_SP, + }; let span = tcx.hir().span(hir_id); if let ScopeData::Remainder(first_statement_index) = self.data { if let Node::Block(ref blk) = tcx.hir().get(hir_id) {