From ff2f7a7a834843ea74b1e7d6511eb4ad06f43981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 6 Nov 2024 19:46:54 +0000 Subject: [PATCH] Point at `const` definition when used instead of a binding in a `let` statement After: ``` error[E0005]: refutable pattern in local binding --> $DIR/bad-pattern.rs:19:13 | LL | const PAT: u32 = 0; | -------------- missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable ... LL | let PAT = v1; | ^^^ | | | pattern `1_u32..=u32::MAX` not covered | help: introduce a variable instead: `PAT_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html = note: the matched value is of type `u32` ``` Before: ``` error[E0005]: refutable pattern in local binding --> $DIR/bad-pattern.rs:19:13 | LL | let PAT = v1; | ^^^ | | | pattern `1_u32..=u32::MAX` not covered | missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `PAT_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html = note: the matched value is of type `u32` ``` --- compiler/rustc_middle/src/thir.rs | 11 +++++++++-- compiler/rustc_middle/src/thir/visit.rs | 2 +- .../src/build/custom/parse/instruction.rs | 4 +++- .../src/build/matches/match_pair.rs | 4 +++- .../rustc_mir_build/src/build/matches/mod.rs | 1 + compiler/rustc_mir_build/src/check_unsafety.rs | 1 + compiler/rustc_mir_build/src/errors.rs | 5 +++-- .../src/thir/pattern/check_match.rs | 18 ++++++++++++++++-- .../rustc_mir_build/src/thir/pattern/mod.rs | 12 ++++++++++-- compiler/rustc_mir_build/src/thir/print.rs | 2 +- compiler/rustc_pattern_analysis/src/rustc.rs | 2 +- compiler/rustc_ty_utils/src/consts.rs | 4 +++- .../2229_closure_analysis/bad-pattern.stderr | 4 +++- tests/ui/consts/const-pattern-irrefutable.rs | 6 +++--- .../ui/consts/const-pattern-irrefutable.stderr | 18 ++++++++++++------ tests/ui/mir/issue-112269.stderr | 6 ++++-- .../const-pat-non-exaustive-let-new-var.rs | 2 +- .../const-pat-non-exaustive-let-new-var.stderr | 4 +++- 18 files changed, 78 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 45ceb0a555d..821f8c99704 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -640,6 +640,7 @@ impl<'tcx> Pat<'tcx> { | Range(..) | Binding { subpattern: None, .. } | Constant { .. } + | NamedConstant { .. } | Error(_) => {} AscribeUserType { subpattern, .. } | Binding { subpattern: Some(subpattern), .. } @@ -788,6 +789,12 @@ pub enum PatKind<'tcx> { value: mir::Const<'tcx>, }, + /// Same as `Constant`, but that came from a `const` that we can point at in diagnostics. + NamedConstant { + value: mir::Const<'tcx>, + span: Span, + }, + /// Inline constant found while lowering a pattern. InlineConstant { /// [LocalDefId] of the constant, we need this so that we have a @@ -1084,8 +1091,8 @@ mod size_asserts { static_assert_size!(Block, 48); static_assert_size!(Expr<'_>, 64); static_assert_size!(ExprKind<'_>, 40); - static_assert_size!(Pat<'_>, 64); - static_assert_size!(PatKind<'_>, 48); + static_assert_size!(Pat<'_>, 72); + static_assert_size!(PatKind<'_>, 56); static_assert_size!(Stmt<'_>, 48); static_assert_size!(StmtKind<'_>, 48); // tidy-alphabetical-end diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index 36f0e3d890c..759ed77dbcb 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -246,7 +246,7 @@ pub fn walk_pat<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>( visitor.visit_pat(&subpattern.pattern); } } - Constant { value: _ } => {} + Constant { value: _ } | NamedConstant { value: _, span: _ } => {} InlineConstant { def: _, subpattern } => visitor.visit_pat(subpattern), Range(_) => {} Slice { prefix, slice, suffix } | Array { prefix, slice, suffix } => { diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index 07964e304b9..049586fd6a0 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -144,7 +144,9 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> { let mut targets = Vec::new(); for arm in rest { let arm = &self.thir[*arm]; - let PatKind::Constant { value } = arm.pattern.kind else { + let (PatKind::Constant { value } | PatKind::NamedConstant { value, span: _ }) = + arm.pattern.kind + else { return Err(ParseError { span: arm.pattern.span, item_description: format!("{:?}", arm.pattern.kind), diff --git a/compiler/rustc_mir_build/src/build/matches/match_pair.rs b/compiler/rustc_mir_build/src/build/matches/match_pair.rs index 6df50057ee8..83a1e021484 100644 --- a/compiler/rustc_mir_build/src/build/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/build/matches/match_pair.rs @@ -129,7 +129,9 @@ impl<'pat, 'tcx> MatchPairTree<'pat, 'tcx> { } } - PatKind::Constant { value } => TestCase::Constant { value }, + PatKind::Constant { value } | PatKind::NamedConstant { value, span: _ } => { + TestCase::Constant { value } + } PatKind::AscribeUserType { ascription: thir::Ascription { ref annotation, variance }, diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index a62d4e9d873..ac5be665654 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -882,6 +882,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } PatKind::Constant { .. } + | PatKind::NamedConstant { .. } | PatKind::Range { .. } | PatKind::Wild | PatKind::Never diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 33e194fa246..d073cbdd877 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -316,6 +316,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { PatKind::Binding { .. } // match is conditional on having this value | PatKind::Constant { .. } + | PatKind::NamedConstant { .. } | PatKind::Variant { .. } | PatKind::Leaf { .. } | PatKind::Deref { .. } diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 00f65e0c7d0..04eb7ffe776 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -860,8 +860,10 @@ pub(crate) struct PatternNotCovered<'s, 'tcx> { pub(crate) uncovered: Uncovered, #[subdiagnostic] pub(crate) inform: Option, + #[label(mir_build_confused)] + pub(crate) interpreted_as_const: Option, #[subdiagnostic] - pub(crate) interpreted_as_const: Option, + pub(crate) interpreted_as_const_sugg: Option, #[subdiagnostic] pub(crate) adt_defined_here: Option>, #[note(mir_build_privately_uninhabited)] @@ -913,7 +915,6 @@ impl<'tcx> Subdiagnostic for AdtDefinedHere<'tcx> { code = "{variable}_var", applicability = "maybe-incorrect" )] -#[label(mir_build_confused)] pub(crate) struct InterpretedAsConst { #[primary_span] pub(crate) span: Span, diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index f222a869c03..b54e1c2b552 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -668,8 +668,20 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { let mut let_suggestion = None; let mut misc_suggestion = None; let mut interpreted_as_const = None; + let mut interpreted_as_const_sugg = None; - if let PatKind::Constant { .. } + if let PatKind::NamedConstant { span, .. } + | PatKind::AscribeUserType { + subpattern: box Pat { kind: PatKind::NamedConstant { span, .. }, .. }, + .. + } = pat.kind + && let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(pat.span) + { + // When we encounter a constant as the binding name, point at the `const` definition. + interpreted_as_const = Some(span); + interpreted_as_const_sugg = + Some(InterpretedAsConst { span: pat.span, variable: snippet }); + } else if let PatKind::Constant { .. } | PatKind::AscribeUserType { subpattern: box Pat { kind: PatKind::Constant { .. }, .. }, .. @@ -683,7 +695,8 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { start_span: pat.span.shrink_to_lo(), }); } else if snippet.chars().all(|c| c.is_alphanumeric() || c == '_') { - interpreted_as_const = + interpreted_as_const = Some(pat.span); + interpreted_as_const_sugg = Some(InterpretedAsConst { span: pat.span, variable: snippet }); } } @@ -733,6 +746,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { uncovered: Uncovered::new(pat.span, &cx, witnesses), inform, interpreted_as_const, + interpreted_as_const_sugg, witness_1_is_privately_uninhabited, _p: (), pattern_ty, diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index ec852add94d..260ace55fba 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -157,7 +157,9 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { } kind => (kind, None, None), }; - let value = if let PatKind::Constant { value } = kind { + let value = if let PatKind::Constant { value } + | PatKind::NamedConstant { value, span: _ } = kind + { value } else { let msg = format!( @@ -560,9 +562,15 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { _ => return pat_from_kind(self.lower_variant_or_leaf(res, id, span, ty, vec![])), }; + // HERE let args = self.typeck_results.node_args(id); let c = ty::Const::new_unevaluated(self.tcx, ty::UnevaluatedConst { def: def_id, args }); - let pattern = self.const_to_pat(c, ty, id, span); + let def_span = self.tcx.def_span(def_id); + let mut pattern = self.const_to_pat(c, ty, id, span); + if let PatKind::Constant { value } = pattern.kind { + pattern.kind = PatKind::NamedConstant { value, span: def_span }; + } + tracing::info!("pattern {pattern:#?} {c:?} {ty:?} {id:?}"); if !is_associated_const { return pattern; diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index dae13df4054..3fe75439339 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -702,7 +702,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { self.print_pat(subpattern, depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); } - PatKind::Constant { value } => { + PatKind::Constant { value } | PatKind::NamedConstant { value, span: _ } => { print_indented!(self, "Constant {", depth_lvl + 1); print_indented!(self, format!("value: {:?}", value), depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 9ea5023064c..6496d9fd73d 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -536,7 +536,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { ), } } - PatKind::Constant { value } => { + PatKind::Constant { value } | PatKind::NamedConstant { value, span: _ } => { match ty.kind() { ty::Bool => { ctor = match value.try_eval_bool(cx.tcx, cx.param_env) { diff --git a/compiler/rustc_ty_utils/src/consts.rs b/compiler/rustc_ty_utils/src/consts.rs index 637e239a570..3a0eb8143cd 100644 --- a/compiler/rustc_ty_utils/src/consts.rs +++ b/compiler/rustc_ty_utils/src/consts.rs @@ -370,7 +370,9 @@ impl<'a, 'tcx> IsThirPolymorphic<'a, 'tcx> { } match pat.kind { - thir::PatKind::Constant { value } => value.has_non_region_param(), + thir::PatKind::Constant { value } | thir::PatKind::NamedConstant { value, span: _ } => { + value.has_non_region_param() + } thir::PatKind::Range(box thir::PatRange { lo, hi, .. }) => { lo.has_non_region_param() || hi.has_non_region_param() } diff --git a/tests/ui/closures/2229_closure_analysis/bad-pattern.stderr b/tests/ui/closures/2229_closure_analysis/bad-pattern.stderr index ca8c2a16d32..5692d530b06 100644 --- a/tests/ui/closures/2229_closure_analysis/bad-pattern.stderr +++ b/tests/ui/closures/2229_closure_analysis/bad-pattern.stderr @@ -97,11 +97,13 @@ LL | if let Refutable::A = v3 { todo!() }; error[E0005]: refutable pattern in local binding --> $DIR/bad-pattern.rs:19:13 | +LL | const PAT: u32 = 0; + | -------------- missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable +... LL | let PAT = v1; | ^^^ | | | pattern `1_u32..=u32::MAX` not covered - | missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `PAT_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant diff --git a/tests/ui/consts/const-pattern-irrefutable.rs b/tests/ui/consts/const-pattern-irrefutable.rs index 61bdf57ffdb..54d2ad32386 100644 --- a/tests/ui/consts/const-pattern-irrefutable.rs +++ b/tests/ui/consts/const-pattern-irrefutable.rs @@ -1,28 +1,28 @@ mod foo { pub const b: u8 = 2; + //~^ missing patterns are not covered because `c` is interpreted as a constant pattern, not a new variable pub const d: u8 = 2; + //~^ missing patterns are not covered because `d` is interpreted as a constant pattern, not a new variable } use foo::b as c; use foo::d; const a: u8 = 2; +//~^ missing patterns are not covered because `a` is interpreted as a constant pattern, not a new variable fn main() { let a = 4; //~^ ERROR refutable pattern in local binding //~| patterns `0_u8..=1_u8` and `3_u8..=u8::MAX` not covered - //~| missing patterns are not covered because `a` is interpreted as a constant pattern, not a new variable //~| HELP introduce a variable instead let c = 4; //~^ ERROR refutable pattern in local binding //~| patterns `0_u8..=1_u8` and `3_u8..=u8::MAX` not covered - //~| missing patterns are not covered because `c` is interpreted as a constant pattern, not a new variable //~| HELP introduce a variable instead let d = 4; //~^ ERROR refutable pattern in local binding //~| patterns `0_u8..=1_u8` and `3_u8..=u8::MAX` not covered - //~| missing patterns are not covered because `d` is interpreted as a constant pattern, not a new variable //~| HELP introduce a variable instead fn f() {} // Check that the `NOTE`s still work with an item here (cf. issue #35115). } diff --git a/tests/ui/consts/const-pattern-irrefutable.stderr b/tests/ui/consts/const-pattern-irrefutable.stderr index 2aed68bdd64..d4365fa16e8 100644 --- a/tests/ui/consts/const-pattern-irrefutable.stderr +++ b/tests/ui/consts/const-pattern-irrefutable.stderr @@ -1,11 +1,13 @@ error[E0005]: refutable pattern in local binding - --> $DIR/const-pattern-irrefutable.rs:12:9 + --> $DIR/const-pattern-irrefutable.rs:15:9 | +LL | const a: u8 = 2; + | ----------- missing patterns are not covered because `a` is interpreted as a constant pattern, not a new variable +... LL | let a = 4; | ^ | | | patterns `0_u8..=1_u8` and `3_u8..=u8::MAX` not covered - | missing patterns are not covered because `a` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `a_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant @@ -13,13 +15,15 @@ LL | let a = 4; = note: the matched value is of type `u8` error[E0005]: refutable pattern in local binding - --> $DIR/const-pattern-irrefutable.rs:17:9 + --> $DIR/const-pattern-irrefutable.rs:19:9 | +LL | pub const b: u8 = 2; + | --------------- missing patterns are not covered because `c` is interpreted as a constant pattern, not a new variable +... LL | let c = 4; | ^ | | | patterns `0_u8..=1_u8` and `3_u8..=u8::MAX` not covered - | missing patterns are not covered because `c` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `c_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant @@ -27,13 +31,15 @@ LL | let c = 4; = note: the matched value is of type `u8` error[E0005]: refutable pattern in local binding - --> $DIR/const-pattern-irrefutable.rs:22:9 + --> $DIR/const-pattern-irrefutable.rs:23:9 | +LL | pub const d: u8 = 2; + | --------------- missing patterns are not covered because `d` is interpreted as a constant pattern, not a new variable +... LL | let d = 4; | ^ | | | patterns `0_u8..=1_u8` and `3_u8..=u8::MAX` not covered - | missing patterns are not covered because `d` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `d_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant diff --git a/tests/ui/mir/issue-112269.stderr b/tests/ui/mir/issue-112269.stderr index f5b79602797..945a7e25dd0 100644 --- a/tests/ui/mir/issue-112269.stderr +++ b/tests/ui/mir/issue-112269.stderr @@ -1,11 +1,12 @@ error[E0005]: refutable pattern in local binding --> $DIR/issue-112269.rs:3:9 | +LL | const x: i32 = 4; + | ------------ missing patterns are not covered because `x` is interpreted as a constant pattern, not a new variable LL | let x: i32 = 3; | ^ | | | patterns `i32::MIN..=3_i32` and `5_i32..=i32::MAX` not covered - | missing patterns are not covered because `x` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `x_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant @@ -15,11 +16,12 @@ LL | let x: i32 = 3; error[E0005]: refutable pattern in local binding --> $DIR/issue-112269.rs:7:9 | +LL | const y: i32 = 3; + | ------------ missing patterns are not covered because `y` is interpreted as a constant pattern, not a new variable LL | let y = 4; | ^ | | | patterns `i32::MIN..=2_i32` and `4_i32..=i32::MAX` not covered - | missing patterns are not covered because `y` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `y_var` | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant diff --git a/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.rs b/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.rs index af47ba8baa3..1a440a90cd7 100644 --- a/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.rs +++ b/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.rs @@ -2,9 +2,9 @@ fn main() { let A = 3; //~^ ERROR refutable pattern in local binding //~| patterns `i32::MIN..=1_i32` and `3_i32..=i32::MAX` not covered - //~| missing patterns are not covered because `A` is interpreted as a constant pattern, not a new variable //~| HELP introduce a variable instead //~| SUGGESTION A_var const A: i32 = 2; + //~^ missing patterns are not covered because `A` is interpreted as a constant pattern, not a new variable } diff --git a/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr b/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr index b6c28612802..29c4b2905eb 100644 --- a/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr +++ b/tests/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr @@ -5,8 +5,10 @@ LL | let A = 3; | ^ | | | patterns `i32::MIN..=1_i32` and `3_i32..=i32::MAX` not covered - | missing patterns are not covered because `A` is interpreted as a constant pattern, not a new variable | help: introduce a variable instead: `A_var` +... +LL | const A: i32 = 2; + | ------------ missing patterns are not covered because `A` is interpreted as a constant pattern, not a new variable | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html