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

Pattern Migration 2024: suggest nicer patterns #136496

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
15 changes: 2 additions & 13 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2797,13 +2797,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// FIXME(ref_pat_eat_one_layer_2024): The migration diagnostic doesn't know how to track the
// default binding mode in the presence of Rule 3 or Rule 5. As a consequence, the labels it
// gives for default binding modes are wrong, as well as suggestions based on the default
// binding mode. This keeps it from making those suggestions, as doing so could panic.
// binding mode.
let info = table.entry(pat_id).or_insert_with(|| ty::Rust2024IncompatiblePatInfo {
primary_labels: Vec::new(),
bad_modifiers: false,
bad_ref_pats: false,
suggest_eliding_modes: !self.tcx.features().ref_pat_eat_one_layer_2024()
&& !self.tcx.features().ref_pat_eat_one_layer_2024_structural(),
});

// Only provide a detailed label if the problematic subpattern isn't from an expansion.
Expand All @@ -2815,20 +2813,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// so, we may want to inspect the span's source callee or macro backtrace.
"occurs within macro expansion".to_owned()
} else {
let pat_kind = if let PatKind::Binding(user_bind_annot, _, _, _) = subpat.kind {
let pat_kind = if matches!(subpat.kind, PatKind::Binding(_, _, _, _)) {
info.bad_modifiers |= true;
// If the user-provided binding modifier doesn't match the default binding mode, we'll
// need to suggest reference patterns, which can affect other bindings.
// For simplicity, we opt to suggest making the pattern fully explicit.
info.suggest_eliding_modes &=
user_bind_annot == BindingMode(ByRef::Yes(def_br_mutbl), Mutability::Not);
"binding modifier"
} else {
info.bad_ref_pats |= true;
// For simplicity, we don't try to suggest eliding reference patterns. Thus, we'll
// suggest adding them instead, which can affect the types assigned to bindings.
// As such, we opt to suggest making the pattern fully explicit.
info.suggest_eliding_modes = false;
"reference pattern"
};
let dbm_str = match def_br_mutbl {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,4 @@ pub struct Rust2024IncompatiblePatInfo {
pub bad_modifiers: bool,
/// Whether any `&` or `&mut` patterns occur under a non-`move` default binding mode.
pub bad_ref_pats: bool,
/// If `true`, we can give a simpler suggestion solely by eliding explicit binding modifiers.
pub suggest_eliding_modes: bool,
}
60 changes: 40 additions & 20 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1098,37 +1098,41 @@ pub(crate) enum MiscPatternSuggestion {

#[derive(LintDiagnostic)]
#[diag(mir_build_rust_2024_incompatible_pat)]
pub(crate) struct Rust2024IncompatiblePat {
pub(crate) struct Rust2024IncompatiblePat<'m> {
#[subdiagnostic]
pub(crate) sugg: Rust2024IncompatiblePatSugg,
pub(crate) sugg: Rust2024IncompatiblePatSugg<'m>,
pub(crate) bad_modifiers: bool,
pub(crate) bad_ref_pats: bool,
pub(crate) is_hard_error: bool,
}

pub(crate) struct Rust2024IncompatiblePatSugg {
/// If true, our suggestion is to elide explicit binding modifiers.
/// If false, our suggestion is to make the pattern fully explicit.
pub(crate) suggest_eliding_modes: bool,
pub(crate) struct Rust2024IncompatiblePatSugg<'m> {
pub(crate) suggestion: Vec<(Span, String)>,
/// If `Some(..)`, we provide a suggestion about either adding or removing syntax.
/// If `None`, we suggest both additions and removals; use a generic wording for simplicity.
pub(crate) kind: Option<Rust2024IncompatiblePatSuggKind>,
pub(crate) ref_pattern_count: usize,
pub(crate) binding_mode_count: usize,
/// Internal state: the ref-mutability of the default binding mode at the subpattern being
/// lowered, with the span where it was introduced. `None` for a by-value default mode.
pub(crate) default_mode_span: Option<(Span, ty::Mutability)>,
/// Labels for where incompatibility-causing by-ref default binding modes were introduced.
pub(crate) default_mode_labels: FxIndexMap<Span, ty::Mutability>,
pub(crate) default_mode_labels: &'m FxIndexMap<Span, ty::Mutability>,
}

impl Subdiagnostic for Rust2024IncompatiblePatSugg {
pub(crate) enum Rust2024IncompatiblePatSuggKind {
Subtractive,
Additive,
}

impl<'m> Subdiagnostic for Rust2024IncompatiblePatSugg<'m> {
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
_f: &F,
) {
use Rust2024IncompatiblePatSuggKind::*;

// Format and emit explanatory notes about default binding modes. Reversing the spans' order
// means if we have nested spans, the innermost ones will be visited first.
for (span, def_br_mutbl) in self.default_mode_labels.into_iter().rev() {
for (&span, &def_br_mutbl) in self.default_mode_labels.iter().rev() {
// Don't point to a macro call site.
if !span.from_expansion() {
let note_msg = "matching on a reference type with a non-reference pattern changes the default binding mode";
Expand All @@ -1147,17 +1151,33 @@ impl Subdiagnostic for Rust2024IncompatiblePatSugg {
} else {
Applicability::MaybeIncorrect
};
let msg = if self.suggest_eliding_modes {
let plural_modes = pluralize!(self.binding_mode_count);
format!("remove the unnecessary binding modifier{plural_modes}")
} else {
let plural_derefs = pluralize!(self.ref_pattern_count);
let and_modes = if self.binding_mode_count > 0 {
format!(" and variable binding mode{}", pluralize!(self.binding_mode_count))
let msg = if let Some(kind) = self.kind {
let derefs = if self.ref_pattern_count > 0 {
format!("reference pattern{}", pluralize!(self.ref_pattern_count))
} else {
String::new()
};
format!("make the implied reference pattern{plural_derefs}{and_modes} explicit")
let modes = if self.binding_mode_count > 0 {
match kind {
Subtractive => {
format!("binding modifier{}", pluralize!(self.binding_mode_count))
}
Additive => {
format!("variable binding mode{}", pluralize!(self.binding_mode_count))
}
}
} else {
String::new()
};
let and = if !derefs.is_empty() && !modes.is_empty() { " and " } else { "" };
match kind {
Subtractive => format!("remove the unnecessary {derefs}{and}{modes}"),
Additive => {
format!("make the implied {derefs}{and}{modes} explicit")
}
}
} else {
"rewrite the pattern".to_owned()
};
// FIXME(dianne): for peace of mind, don't risk emitting a 0-part suggestion (that panics!)
if !self.suggestion.is_empty() {
Expand Down
Loading
Loading