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.

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

Check pattern refutability on THIR #108504

Merged
merged 13 commits into from
Apr 6, 2023
11 changes: 0 additions & 11 deletions compiler/rustc_hir/src/pat_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
});
},
{
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a DepKind::dep_kind variant. The former version did not handle hygiene correctly, so did not fire in case of macro expansion. The new version just ignores hygiene, so fires. (In that case, it's actually the more correct behaviour.)

}
}
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1984,6 +1984,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,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
56 changes: 54 additions & 2 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
16 changes: 2 additions & 14 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 });
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
37 changes: 8 additions & 29 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
Expand All @@ -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)]
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/src/thir/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
};
Expand Down
Loading