Auto merge of #108504 - cjgillot:thir-pattern, r=compiler-errors,Nilstrieb

Check pattern refutability on THIR

The current `check_match` query is based on HIR, but partially re-lowers HIR into THIR.
This PR proposed to use the results of the `thir_body` query to check matches, instead of re-building THIR.

Most of the diagnostic changes are spans getting shorter, or commas/semicolons not getting removed.

This PR degrades the diagnostic for confusing constants in patterns (`let A = foo()` where `A` resolves to a `const A` somewhere): it does not point ot the definition of `const A` any more.

Author: bors

Diff for: compiler/rustc_hir/src/pat_util.rs

@@ -2,7 +2,6 @@ use crate::def::{CtorOf, DefKind, Res};
 use crate::def_id::DefId;
 use crate::hir::{self, BindingAnnotation, ByRef, HirId, PatKind};
 use rustc_data_structures::fx::FxHashSet;
-use rustc_span::hygiene::DesugaringKind;
 use rustc_span::symbol::Ident;
 use rustc_span::Span;
 
@@ -136,14 +135,4 @@ impl hir::Pat<'_> {
         });
         result
     }
-
-    /// If the pattern is `Some(<pat>)` from a desugared for loop, returns the inner pattern
-    pub fn for_loop_some(&self) -> Option<&Self> {
-        if self.span.desugaring_kind() == Some(DesugaringKind::ForLoop) {
-            if let hir::PatKind::Struct(_, [pat_field], _) = self.kind {
-                return Some(pat_field.pat);
-            }
-        }
-        None
-    }
 }

Diff for: compiler/rustc_interface/src/passes.rs

@@ -765,7 +765,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
         parallel!(
             {
                 sess.time("match_checking", || {
-                    tcx.hir().par_body_owners(|def_id| tcx.ensure().check_match(def_id.to_def_id()))
+                    tcx.hir().par_body_owners(|def_id| tcx.ensure().check_match(def_id))
                 });
             },
             {

Diff for: compiler/rustc_middle/src/dep_graph/mod.rs

@@ -94,7 +94,7 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {
     }
 
     #[inline]
-    fn dep_kind_info(&self, dep_kind: DepKind) -> &DepKindStruct<'tcx> {
-        &self.query_kinds[dep_kind as usize]
+    fn dep_kind_info(&self, dk: DepKind) -> &DepKindStruct<'tcx> {
+        &self.query_kinds[dk as usize]
     }
 }

Diff for: compiler/rustc_middle/src/mir/mod.rs

@@ -2001,6 +2001,13 @@ impl<'tcx> Rvalue<'tcx> {
 }
 
 impl BorrowKind {
+    pub fn mutability(&self) -> Mutability {
+        match *self {
+            BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => Mutability::Not,
+            BorrowKind::Mut { .. } => Mutability::Mut,
+        }
+    }
+
     pub fn allows_two_phase_borrow(&self) -> bool {
         match *self {
             BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => false,

Diff for: compiler/rustc_middle/src/query/mod.rs

@@ -1114,9 +1114,9 @@ rustc_queries! {
         desc { "converting literal to mir constant" }
     }
 
-    query check_match(key: DefId) {
-        desc { |tcx| "match-checking `{}`", tcx.def_path_str(key) }
-        cache_on_disk_if { key.is_local() }
+    query check_match(key: LocalDefId) {
+        desc { |tcx| "match-checking `{}`", tcx.def_path_str(key.to_def_id()) }
+        cache_on_disk_if { true }
     }
 
     /// Performs part of the privacy check and computes effective visibilities.

Diff for: compiler/rustc_middle/src/thir.rs

@@ -227,6 +227,9 @@ pub enum StmtKind<'tcx> {
 
         /// The lint level for this `let` statement.
         lint_level: LintLevel,
+
+        /// Span of the `let <PAT> = <INIT>` part.
+        span: Span,
     },
 }
 
@@ -594,6 +597,55 @@ impl<'tcx> Pat<'tcx> {
             _ => None,
         }
     }
+
+    /// Call `f` on every "binding" in a pattern, e.g., on `a` in
+    /// `match foo() { Some(a) => (), None => () }`
+    pub fn each_binding(&self, mut f: impl FnMut(Symbol, BindingMode, Ty<'tcx>, Span)) {
+        self.walk_always(|p| {
+            if let PatKind::Binding { name, mode, ty, .. } = p.kind {
+                f(name, mode, ty, p.span);
+            }
+        });
+    }
+
+    /// Walk the pattern in left-to-right order.
+    ///
+    /// If `it(pat)` returns `false`, the children are not visited.
+    pub fn walk(&self, mut it: impl FnMut(&Pat<'tcx>) -> bool) {
+        self.walk_(&mut it)
+    }
+
+    fn walk_(&self, it: &mut impl FnMut(&Pat<'tcx>) -> bool) {
+        if !it(self) {
+            return;
+        }
+
+        use PatKind::*;
+        match &self.kind {
+            Wild | Range(..) | Binding { subpattern: None, .. } | Constant { .. } => {}
+            AscribeUserType { subpattern, .. }
+            | Binding { subpattern: Some(subpattern), .. }
+            | Deref { subpattern } => subpattern.walk_(it),
+            Leaf { subpatterns } | Variant { subpatterns, .. } => {
+                subpatterns.iter().for_each(|field| field.pattern.walk_(it))
+            }
+            Or { pats } => pats.iter().for_each(|p| p.walk_(it)),
+            Array { box ref prefix, ref slice, box ref suffix }
+            | Slice { box ref prefix, ref slice, box ref suffix } => {
+                prefix.iter().chain(slice.iter()).chain(suffix.iter()).for_each(|p| p.walk_(it))
+            }
+        }
+    }
+
+    /// Walk the pattern in left-to-right order.
+    ///
+    /// If you always want to recurse, prefer this method over `walk`.
+    pub fn walk_always(&self, mut it: impl FnMut(&Pat<'tcx>)) {
+        self.walk(|p| {
+            it(p);
+            true
+        })
+    }
 }
 
 impl<'tcx> IntoDiagnosticArg for Pat<'tcx> {
@@ -879,7 +931,7 @@ mod size_asserts {
     static_assert_size!(ExprKind<'_>, 40);
     static_assert_size!(Pat<'_>, 72);
     static_assert_size!(PatKind<'_>, 56);
-    static_assert_size!(Stmt<'_>, 48);
-    static_assert_size!(StmtKind<'_>, 40);
+    static_assert_size!(Stmt<'_>, 56);
+    static_assert_size!(StmtKind<'_>, 48);
     // tidy-alphabetical-end
 }

Diff for: compiler/rustc_middle/src/thir/visit.rs

@@ -175,6 +175,7 @@ pub fn walk_stmt<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, stmt: &Stm
             ref pattern,
             lint_level: _,
             else_block,
+            span: _,
         } => {
             if let Some(init) = initializer {
                 visitor.visit_expr(&visitor.thir()[*init]);

Diff for: compiler/rustc_mir_build/messages.ftl

@@ -239,19 +239,9 @@ mir_build_trailing_irrefutable_let_patterns = trailing irrefutable {$count ->
     } into the body
 
 mir_build_bindings_with_variant_name =
-    pattern binding `{$ident}` is named the same as one of the variants of the type `{$ty_path}`
+    pattern binding `{$name}` is named the same as one of the variants of the type `{$ty_path}`
     .suggestion = to match on the variant, qualify the path
 
-mir_build_irrefutable_let_patterns_generic_let = irrefutable `let` {$count ->
-        [one] pattern
-        *[other] patterns
-    }
-    .note = {$count ->
-        [one] this pattern
-        *[other] these patterns
-    } will always match, so the `let` is useless
-    .help = consider removing `let`
-
 mir_build_irrefutable_let_patterns_if_let = irrefutable `if let` {$count ->
         [one] pattern
         *[other] patterns
@@ -357,15 +347,13 @@ mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern",
 
 mir_build_more_information = for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
 
-mir_build_res_defined_here = {$res} defined here
-
 mir_build_adt_defined_here = `{$ty}` defined here
 
 mir_build_variant_defined_here = not covered
 
 mir_build_interpreted_as_const = introduce a variable instead
 
-mir_build_confused = missing patterns are not covered because `{$variable}` is interpreted as {$article} {$res} pattern, not a new variable
+mir_build_confused = missing patterns are not covered because `{$variable}` is interpreted as a constant pattern, not a new variable
 
 mir_build_suggest_if_let = you might want to use `if let` to ignore the {$count ->
         [one] variant that isn't

Diff for: compiler/rustc_mir_build/src/build/block.rs

@@ -115,6 +115,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     initializer: Some(initializer),
                     lint_level,
                     else_block: Some(else_block),
+                    span: _,
                 } => {
                     // When lowering the statement `let <pat> = <expr> else { <else> };`,
                     // the `<else>` block is nested in the parent scope enclosing this statement.
@@ -278,6 +279,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     initializer,
                     lint_level,
                     else_block: None,
+                    span: _,
                 } => {
                     let ignores_expr_result = matches!(pattern.kind, PatKind::Wild);
                     this.block_context.push(BlockFrame::Statement { ignores_expr_result });

Diff for: compiler/rustc_mir_build/src/build/mod.rs

@@ -58,6 +58,7 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
         ty::WithOptConstParam { did, const_param_did: None } => {
             tcx.ensure_with_value().thir_check_unsafety(did);
             tcx.ensure_with_value().thir_abstract_const(did);
+            tcx.ensure_with_value().check_match(did);
         }
     }
 

Diff for: compiler/rustc_mir_build/src/errors.rs

@@ -6,11 +6,11 @@ use rustc_errors::{
     error_code, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
     Handler, IntoDiagnostic, MultiSpan, SubdiagnosticMessage,
 };
-use rustc_hir::def::Res;
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::thir::Pat;
 use rustc_middle::ty::{self, Ty};
-use rustc_span::{symbol::Ident, Span};
+use rustc_span::symbol::Symbol;
+use rustc_span::Span;
 
 #[derive(LintDiagnostic)]
 #[diag(mir_build_unconditional_recursion)]
@@ -534,18 +534,10 @@ pub struct TrailingIrrefutableLetPatterns {
 #[derive(LintDiagnostic)]
 #[diag(mir_build_bindings_with_variant_name, code = "E0170")]
 pub struct BindingsWithVariantName {
-    #[suggestion(code = "{ty_path}::{ident}", applicability = "machine-applicable")]
+    #[suggestion(code = "{ty_path}::{name}", applicability = "machine-applicable")]
     pub suggestion: Option<Span>,
     pub ty_path: String,
-    pub ident: Ident,
-}
-
-#[derive(LintDiagnostic)]
-#[diag(mir_build_irrefutable_let_patterns_generic_let)]
-#[note]
-#[help]
-pub struct IrrefutableLetPatternsGenericLet {
-    pub count: usize,
+    pub name: Symbol,
 }
 
 #[derive(LintDiagnostic)]
@@ -584,13 +576,12 @@ pub struct IrrefutableLetPatternsWhileLet {
 #[diag(mir_build_borrow_of_moved_value)]
 pub struct BorrowOfMovedValue<'tcx> {
     #[primary_span]
-    pub span: Span,
     #[label]
     #[label(mir_build_occurs_because_label)]
     pub binding_span: Span,
     #[label(mir_build_value_borrowed_label)]
     pub conflicts_ref: Vec<Span>,
-    pub name: Ident,
+    pub name: Symbol,
     pub ty: Ty<'tcx>,
     #[suggestion(code = "ref ", applicability = "machine-applicable")]
     pub suggest_borrowing: Option<Span>,
@@ -638,19 +629,19 @@ pub enum Conflict {
     Mut {
         #[primary_span]
         span: Span,
-        name: Ident,
+        name: Symbol,
     },
     #[label(mir_build_borrow)]
     Ref {
         #[primary_span]
         span: Span,
-        name: Ident,
+        name: Symbol,
     },
     #[label(mir_build_moved)]
     Moved {
         #[primary_span]
         span: Span,
-        name: Ident,
+        name: Symbol,
     },
 }
 
@@ -802,8 +793,6 @@ pub(crate) struct PatternNotCovered<'s, 'tcx> {
     pub let_suggestion: Option<SuggestLet>,
     #[subdiagnostic]
     pub misc_suggestion: Option<MiscPatternSuggestion>,
-    #[subdiagnostic]
-    pub res_defined_here: Option<ResDefinedHere>,
 }
 
 #[derive(Subdiagnostic)]
@@ -837,14 +826,6 @@ impl<'tcx> AddToDiagnostic for AdtDefinedHere<'tcx> {
     }
 }
 
-#[derive(Subdiagnostic)]
-#[label(mir_build_res_defined_here)]
-pub struct ResDefinedHere {
-    #[primary_span]
-    pub def_span: Span,
-    pub res: Res,
-}
-
 #[derive(Subdiagnostic)]
 #[suggestion(
     mir_build_interpreted_as_const,
@@ -855,9 +836,7 @@ pub struct ResDefinedHere {
 pub struct InterpretedAsConst {
     #[primary_span]
     pub span: Span,
-    pub article: &'static str,
     pub variable: String,
-    pub res: Res,
 }
 
 #[derive(Subdiagnostic)]

Diff for: compiler/rustc_mir_build/src/thir/cx/block.rs

@@ -105,6 +105,10 @@ impl<'tcx> Cx<'tcx> {
                             }
                         }
 
+                        let span = match local.init {
+                            Some(init) => local.span.with_hi(init.span.hi()),
+                            None => local.span,
+                        };
                         let stmt = Stmt {
                             kind: StmtKind::Let {
                                 remainder_scope,
@@ -116,6 +120,7 @@ impl<'tcx> Cx<'tcx> {
                                 initializer: local.init.map(|init| self.mirror_expr(init)),
                                 else_block,
                                 lint_level: LintLevel::Explicit(local.hir_id),
+                                span,
                             },
                             opt_destruction_scope: opt_dxn_ext,
                         };