Skip to content

Commit

Permalink
try to suggest eliding redundant binding modifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
dianne committed Feb 5, 2025
1 parent 060cc37 commit b32a533
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 49 deletions.
23 changes: 21 additions & 2 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2786,7 +2786,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let mut typeck_results = self.typeck_results.borrow_mut();
let mut table = typeck_results.rust_2024_migration_desugared_pats_mut();
let info = table.entry(pat_id).or_default();
// 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.
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.
// In the case that it's from a macro, we'll add a more detailed note in the emitter.
Expand All @@ -2797,11 +2807,20 @@ 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 matches!(subpat.kind, PatKind::Binding(_, _, _, _)) {
let pat_kind = if let PatKind::Binding(user_bind_annot, _, _, _) = subpat.kind {
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
4 changes: 3 additions & 1 deletion compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,12 +814,14 @@ impl<'tcx> std::fmt::Display for UserTypeKind<'tcx> {

/// Information on a pattern incompatible with Rust 2024, for use by the error/migration diagnostic
/// emitted during THIR construction.
#[derive(TyEncodable, TyDecodable, Debug, HashStable, Default)]
#[derive(TyEncodable, TyDecodable, Debug, HashStable)]
pub struct Rust2024IncompatiblePatInfo {
/// Labeled spans for `&`s, `&mut`s, and binding modifiers incompatible with Rust 2024.
pub primary_labels: Vec<(Span, String)>,
/// Whether any binding modifiers occur under a non-`move` default binding mode.
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,
}
23 changes: 14 additions & 9 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,9 @@ pub(crate) struct Rust2024IncompatiblePat {
}

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) suggestion: Vec<(Span, String)>,
pub(crate) ref_pattern_count: usize,
pub(crate) binding_mode_count: usize,
Expand Down Expand Up @@ -1144,16 +1147,18 @@ impl Subdiagnostic for Rust2024IncompatiblePatSugg {
} else {
Applicability::MaybeIncorrect
};
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 self.suggest_eliding_modes {
let plural_modes = pluralize!(self.binding_mode_count);
format!("remove the unnecessary binding modifier{plural_modes}")
} else {
String::new()
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))
} else {
String::new()
};
format!("make the implied reference pattern{plural_derefs}{and_modes} explicit")
};
diag.multipart_suggestion_verbose(
format!("make the implied reference pattern{plural_derefs}{and_modes} explicit"),
self.suggestion,
applicability,
);
diag.multipart_suggestion_verbose(msg, self.suggestion, applicability);
}
}
58 changes: 31 additions & 27 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ pub(super) fn pat_from_hir<'a, 'tcx>(
tcx,
typing_env,
typeck_results,
rust_2024_migration_suggestion: migration_info.and(Some(Rust2024IncompatiblePatSugg {
suggestion: Vec::new(),
ref_pattern_count: 0,
binding_mode_count: 0,
default_mode_span: None,
default_mode_labels: Default::default(),
})),
rust_2024_migration_suggestion: migration_info.and_then(|info| {
Some(Rust2024IncompatiblePatSugg {
suggest_eliding_modes: info.suggest_eliding_modes,
suggestion: Vec::new(),
ref_pattern_count: 0,
binding_mode_count: 0,
default_mode_span: None,
default_mode_labels: Default::default(),
})
}),
};
let result = pcx.lower_pattern(pat);
debug!("pat_from_hir({:?}) = {:?}", pat, result);
Expand Down Expand Up @@ -107,27 +110,22 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
if let Some(s) = &mut self.rust_2024_migration_suggestion
&& !adjustments.is_empty()
{
let mut min_mutbl = Mutability::Mut;
let suggestion_str: String = adjustments
.iter()
.map(|ref_ty| {
let &ty::Ref(_, _, mutbl) = ref_ty.kind() else {
span_bug!(pat.span, "pattern implicitly dereferences a non-ref type");
};

match mutbl {
Mutability::Not => {
min_mutbl = Mutability::Not;
"&"
}
Mutability::Mut => "&mut ",
}
})
.collect();
s.suggestion.push((pat.span.shrink_to_lo(), suggestion_str));
s.ref_pattern_count += adjustments.len();
let implicit_deref_mutbls = adjustments.iter().map(|ref_ty| {
let &ty::Ref(_, _, mutbl) = ref_ty.kind() else {
span_bug!(pat.span, "pattern implicitly dereferences a non-ref type");
};
mutbl
});

if !s.suggest_eliding_modes {
let suggestion_str: String =
implicit_deref_mutbls.clone().map(|mutbl| mutbl.ref_prefix_str()).collect();
s.suggestion.push((pat.span.shrink_to_lo(), suggestion_str));
s.ref_pattern_count += adjustments.len();
}

// Remember if this changed the default binding mode, in case we want to label it.
let min_mutbl = implicit_deref_mutbls.min().unwrap();
if s.default_mode_span.is_none_or(|(_, old_mutbl)| min_mutbl < old_mutbl) {
opt_old_mode_span = Some(s.default_mode_span);
s.default_mode_span = Some((pat.span, min_mutbl));
Expand Down Expand Up @@ -413,8 +411,14 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
{
// If this overrides a by-ref default binding mode, label the binding mode.
s.default_mode_labels.insert(default_mode_span, default_ref_mutbl);
// If our suggestion is to elide redundnt modes, this will be one of them.
if s.suggest_eliding_modes {
s.suggestion.push((pat.span.with_hi(ident.span.lo()), String::new()));
s.binding_mode_count += 1;
}
}
if explicit_ba.0 == ByRef::No
if !s.suggest_eliding_modes
&& explicit_ba.0 == ByRef::No
&& let ByRef::Yes(mutbl) = mode.0
{
let sugg_str = match mutbl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn main() {
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

let &Foo(ref x) = &Foo(0);
let Foo(x) = &Foo(0);
//~^ ERROR: binding modifiers may only be written when the default binding mode is `move` in Rust 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, &0u8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
|
LL | let Foo(ref x) = &Foo(0);
| ^^^^^^^^^^ this matches on type `&_`
help: make the implied reference pattern explicit
help: remove the unnecessary binding modifier
|
LL - let Foo(ref x) = &Foo(0);
LL + let Foo(x) = &Foo(0);
|
LL | let &Foo(ref x) = &Foo(0);
| +

error: binding modifiers may only be written when the default binding mode is `move` in Rust 2024
--> $DIR/migration_lint.rs:40:13
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
|
LL | test_pat_on_type![(ref x,): &(T,)];
| ^^^^^^^^ this matches on type `&_`
help: make the implied reference pattern explicit
help: remove the unnecessary binding modifier
|
LL - test_pat_on_type![(ref x,): &(T,)];
LL + test_pat_on_type![(x,): &(T,)];
|
LL | test_pat_on_type![&(ref x,): &(T,)];
| +

error: binding modifiers may only be written when the default binding mode is `move`
--> $DIR/min_match_ergonomics_fail.rs:34:20
Expand All @@ -196,10 +197,11 @@ note: matching on a reference type with a non-reference pattern changes the defa
|
LL | test_pat_on_type![(ref mut x,): &mut (T,)];
| ^^^^^^^^^^^^ this matches on type `&mut _`
help: make the implied reference pattern explicit
help: remove the unnecessary binding modifier
|
LL - test_pat_on_type![(ref mut x,): &mut (T,)];
LL + test_pat_on_type![(x,): &mut (T,)];
|
LL | test_pat_on_type![&mut (ref mut x,): &mut (T,)];
| ++++

error: reference patterns may only be written when the default binding mode is `move`
--> $DIR/min_match_ergonomics_fail.rs:43:10
Expand Down

0 comments on commit b32a533

Please sign in to comment.