Skip to content

clarify wording of match ergonomics diagnostics (rust_2024_incompatible_pat lint and error) #144006

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3113,20 +3113,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 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_modifiers: false,
bad_mut_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(),
});

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"
if user_bind_annot == BindingMode(ByRef::No, Mutability::Mut) {
info.bad_mut_modifiers = true;
"`mut` binding modifier"
} else {
info.bad_ref_modifiers = true;
match user_bind_annot.1 {
Mutability::Not => "explicit `ref` binding modifier",
Mutability::Mut => "explicit `ref mut` binding modifier",
}
}
} else {
info.bad_ref_pats = true;
// For simplicity, we don't try to suggest eliding reference patterns. Thus, we'll
Expand All @@ -3145,11 +3154,7 @@ 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 dbm_str = match def_br_mutbl {
Mutability::Not => "ref",
Mutability::Mut => "ref mut",
};
format!("{pat_kind} not allowed under `{dbm_str}` default binding mode")
format!("{pat_kind} not allowed when implicitly borrowing")
};
info.primary_labels.push((trimmed_span, primary_label));
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ declare_lint! {
"detects patterns whose meaning will change in Rust 2024",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
reference: "<https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>",
reference: "<https://doc.rust-lang.org/edition-guide/rust-2024/match-ergonomics.html>",
};
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,8 +841,10 @@ impl<'tcx> std::fmt::Display for UserTypeKind<'tcx> {
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 `mut` binding modifiers occur under a non-`move` default binding mode.
pub bad_mut_modifiers: bool,
/// Whether any `ref`/`ref mut` binding modifiers occur under a non-`move` default binding mode.
pub bad_ref_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.
Expand Down
11 changes: 0 additions & 11 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -318,17 +318,6 @@ mir_build_pointer_pattern = function pointers and raw pointers not derived from

mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future

mir_build_rust_2024_incompatible_pat = {$bad_modifiers ->
*[true] binding modifiers{$bad_ref_pats ->
*[true] {" "}and reference patterns
[false] {""}
}
[false] reference patterns
} may only be written when the default binding mode is `move`{$is_hard_error ->
*[true] {""}
[false] {" "}in Rust 2024
}

mir_build_static_in_pattern = statics cannot be referenced in patterns
.label = can't be used in patterns
mir_build_static_in_pattern_def = `static` defined here
Expand Down
66 changes: 1 addition & 65 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level,
MultiSpan, Subdiagnostic, pluralize,
MultiSpan, Subdiagnostic,
};
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::{self, Ty};
Expand Down Expand Up @@ -1096,69 +1095,6 @@ pub(crate) enum MiscPatternSuggestion {
},
}

#[derive(LintDiagnostic)]
#[diag(mir_build_rust_2024_incompatible_pat)]
pub(crate) struct Rust2024IncompatiblePat {
#[subdiagnostic]
pub(crate) sugg: Rust2024IncompatiblePatSugg,
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) suggestion: Vec<(Span, String)>,
pub(crate) ref_pattern_count: usize,
pub(crate) binding_mode_count: usize,
/// Labels for where incompatibility-causing by-ref default binding modes were introduced.
pub(crate) default_mode_labels: FxIndexMap<Span, ty::Mutability>,
}

impl Subdiagnostic for Rust2024IncompatiblePatSugg {
fn add_to_diag<G: EmissionGuarantee>(self, diag: &mut Diag<'_, G>) {
// 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() {
// 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";
let label_msg =
format!("this matches on type `{}_`", def_br_mutbl.ref_prefix_str());
let mut label = MultiSpan::from(span);
label.push_span_label(span, label_msg);
diag.span_note(label, note_msg);
}
}

// Format and emit the suggestion.
let applicability =
if self.suggestion.iter().all(|(span, _)| span.can_be_used_for_suggestions()) {
Applicability::MachineApplicable
} 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))
} else {
String::new()
};
format!("make the implied reference pattern{plural_derefs}{and_modes} explicit")
};
// FIXME(dianne): for peace of mind, don't risk emitting a 0-part suggestion (that panics!)
if !self.suggestion.is_empty() {
diag.multipart_suggestion_verbose(msg, self.suggestion, applicability);
}
}
}

#[derive(Diagnostic)]
#[diag(mir_build_loop_match_invalid_update)]
pub(crate) struct LoopMatchInvalidUpdate {
Expand Down
107 changes: 79 additions & 28 deletions compiler/rustc_mir_build/src/thir/pattern/migration.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
//! Automatic migration of Rust 2021 patterns to a form valid in both Editions 2021 and 2024.

use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::MultiSpan;
use rustc_errors::{Applicability, Diag, EmissionGuarantee, MultiSpan, pluralize};
use rustc_hir::{BindingMode, ByRef, HirId, Mutability};
use rustc_lint as lint;
use rustc_middle::ty::{self, Rust2024IncompatiblePatInfo, TyCtxt};
use rustc_span::{Ident, Span};

use crate::errors::{Rust2024IncompatiblePat, Rust2024IncompatiblePatSugg};
use crate::fluent_generated as fluent;

/// For patterns flagged for migration during HIR typeck, this handles constructing and emitting
/// a diagnostic suggestion.
pub(super) struct PatMigration<'a> {
Expand Down Expand Up @@ -49,39 +46,93 @@ impl<'a> PatMigration<'a> {
for (span, label) in self.info.primary_labels.iter() {
spans.push_span_label(*span, label.clone());
}
let sugg = Rust2024IncompatiblePatSugg {
suggest_eliding_modes: self.info.suggest_eliding_modes,
suggestion: self.suggestion,
ref_pattern_count: self.ref_pattern_count,
binding_mode_count: self.binding_mode_count,
default_mode_labels: self.default_mode_labels,
};
// If a relevant span is from at least edition 2024, this is a hard error.
let is_hard_error = spans.primary_spans().iter().any(|span| span.at_least_rust_2024());
let primary_message = self.primary_message(is_hard_error);
if is_hard_error {
let mut err =
tcx.dcx().struct_span_err(spans, fluent::mir_build_rust_2024_incompatible_pat);
let mut err = tcx.dcx().struct_span_err(spans, primary_message);
if let Some(info) = lint::builtin::RUST_2024_INCOMPATIBLE_PAT.future_incompatible {
// provide the same reference link as the lint
err.note(format!("for more information, see {}", info.reference));
Comment on lines 55 to 56
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point, maybe the hard error version of this would be better serviced by linking to the Reference rather than the Edition Guide? Specifically https://doc.rust-lang.org/reference/patterns.html#r-patterns.ident.binding. There's less highlighting of the error cases, but being less focused on Edition migration could be helpful going forward. It also uses (and elaborates on!) the "non-reference pattern" wording and is very methodical about explaining match ergonomics, which I think is helpful. It's a bit easier for me to follow than the Edition Guide's presentation, too. Of course, if we want to be consistent with terminology and presentation, we'll probably also want to update the Edition Guide; #143557 indicates that it may need some rewriting.

}
err.arg("bad_modifiers", self.info.bad_modifiers);
err.arg("bad_ref_pats", self.info.bad_ref_pats);
err.arg("is_hard_error", true);
err.subdiagnostic(sugg);
self.format_subdiagnostics(&mut err);
err.emit();
} else {
tcx.emit_node_span_lint(
lint::builtin::RUST_2024_INCOMPATIBLE_PAT,
pat_id,
spans,
Rust2024IncompatiblePat {
sugg,
bad_modifiers: self.info.bad_modifiers,
bad_ref_pats: self.info.bad_ref_pats,
is_hard_error,
},
);
tcx.node_span_lint(lint::builtin::RUST_2024_INCOMPATIBLE_PAT, pat_id, spans, |diag| {
diag.primary_message(primary_message);
self.format_subdiagnostics(diag);
});
}
}

fn primary_message(&self, is_hard_error: bool) -> String {
let verb1 = match (self.info.bad_mut_modifiers, self.info.bad_ref_modifiers) {
(true, true) => "write explicit binding modifiers",
(true, false) => "mutably bind by value",
(false, true) => "explicitly borrow",
(false, false) => "explicitly dereference",
};
let or_verb2 = match (
self.info.bad_mut_modifiers,
self.info.bad_ref_modifiers,
self.info.bad_ref_pats,
) {
// We only need two verb phrases if mentioning both modifiers and reference patterns.
(false, false, _) | (_, _, false) => "",
// If mentioning `mut`, we don't have an "explicitly" yet.
(true, _, true) => " or explicitly dereference",
// If mentioning `ref`/`ref mut` but not `mut`, we already have an "explicitly".
(false, true, true) => " or dereference",
};
let in_rust_2024 = if is_hard_error { "" } else { " in Rust 2024" };
format!("cannot {verb1}{or_verb2} within an implicitly-borrowing pattern{in_rust_2024}")
}

fn format_subdiagnostics(self, diag: &mut Diag<'_, impl EmissionGuarantee>) {
// 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() {
// 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 implicitly borrows the contents";
let label_msg = format!(
"this non-reference pattern matches on a reference type `{}_`",
def_br_mutbl.ref_prefix_str()
);
let mut label = MultiSpan::from(span);
label.push_span_label(span, label_msg);
diag.span_note(label, note_msg);
}
}

// Format and emit the suggestion.
let applicability =
if self.suggestion.iter().all(|(span, _)| span.can_be_used_for_suggestions()) {
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
};
let plural_modes = pluralize!(self.binding_mode_count);
let msg = if self.info.suggest_eliding_modes {
format!("remove the unnecessary binding modifier{plural_modes}")
} else {
let match_on_these_references = if self.ref_pattern_count == 1 {
"match on the reference with a reference pattern"
} else {
"match on these references with reference patterns"
};
let and_explain_modes = if self.binding_mode_count > 0 {
let a = if self.binding_mode_count == 1 { "a " } else { "" };
format!(" and borrow explicitly using {a}variable binding mode{plural_modes}")
} else {
" to avoid implicitly borrowing".to_owned()
};
format!("{match_on_these_references}{and_explain_modes}")
};
// FIXME(dianne): for peace of mind, don't risk emitting a 0-part suggestion (that panics!)
debug_assert!(!self.suggestion.is_empty());
if !self.suggestion.is_empty() {
diag.multipart_suggestion_verbose(msg, self.suggestion, applicability);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,20 @@ LL | let [&bind_ref_mut!(x)] = &mut [0];
| |
| help: replace this `&` with `&mut`: `&mut`

error: binding modifiers may only be written when the default binding mode is `move`
error: cannot explicitly borrow within an implicitly-borrowing pattern
--> $DIR/mixed-editions.rs:30:10
|
LL | let [bind_ref!(y)] = &[0];
| ^^^^^^^^^^^^ occurs within macro expansion
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
= note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern implicitly borrows the contents
--> $DIR/mixed-editions.rs:30:9
|
LL | let [bind_ref!(y)] = &[0];
| ^^^^^^^^^^^^^^ this matches on type `&_`
| ^^^^^^^^^^^^^^ this non-reference pattern matches on a reference type `&_`
= note: this error originates in the macro `bind_ref` (in Nightly builds, run with -Z macro-backtrace for more info)
help: make the implied reference pattern explicit
help: match on the reference with a reference pattern to avoid implicitly borrowing
|
LL | let &[bind_ref!(y)] = &[0];
| +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ LL | let [&bind_ref_mut!(x)] = &mut [0];
| |
| help: replace this `&` with `&mut`: `&mut`

error: binding modifiers may only be written when the default binding mode is `move`
error: cannot explicitly borrow within an implicitly-borrowing pattern
--> $DIR/mixed-editions.rs:26:21
|
LL | let match_ctor!(ref x) = &[0];
| ^^^ binding modifier not allowed under `ref` default binding mode
| ^^^ explicit `ref` binding modifier not allowed when implicitly borrowing
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
help: make the implied reference pattern explicit
= note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/match-ergonomics.html>
help: match on the reference with a reference pattern to avoid implicitly borrowing
--> $DIR/auxiliary/mixed-editions-macros.rs:11:9
|
LL | &[$p]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ fn assert_type_eq<T, U: Eq<T>>(_: T, _: U) {}
/// only when the binding is from edition 2024.
fn ref_binding_tests() {
let match_ctor!(ref x) = &[0];
//[classic2024,structural2024]~^ ERROR: binding modifiers may only be written when the default binding mode is `move`
//[classic2024,structural2024]~^ ERROR: cannot explicitly borrow within an implicitly-borrowing pattern
#[cfg(any(classic2021, structural2021))] assert_type_eq(x, &0u32);

let [bind_ref!(y)] = &[0];
//[classic2021,structural2021]~^ ERROR: binding modifiers may only be written when the default binding mode is `move`
//[classic2021,structural2021]~^ ERROR: cannot explicitly borrow within an implicitly-borrowing pattern
#[cfg(any(classic2024, structural2024))] assert_type_eq(y, &0u32);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,20 @@ LL | let [&bind_ref_mut!(x)] = &mut [0];
| |
| help: replace this `&` with `&mut`: `&mut`

error: binding modifiers may only be written when the default binding mode is `move`
error: cannot explicitly borrow within an implicitly-borrowing pattern
--> $DIR/mixed-editions.rs:30:10
|
LL | let [bind_ref!(y)] = &[0];
| ^^^^^^^^^^^^ occurs within macro expansion
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
= note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern implicitly borrows the contents
--> $DIR/mixed-editions.rs:30:9
|
LL | let [bind_ref!(y)] = &[0];
| ^^^^^^^^^^^^^^ this matches on type `&_`
| ^^^^^^^^^^^^^^ this non-reference pattern matches on a reference type `&_`
= note: this error originates in the macro `bind_ref` (in Nightly builds, run with -Z macro-backtrace for more info)
help: make the implied reference pattern explicit
help: match on the reference with a reference pattern to avoid implicitly borrowing
|
LL | let &[bind_ref!(y)] = &[0];
| +
Expand Down
Loading
Loading