Skip to content

Clarify the match ergonomics 2024 migration lint's output #134394

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

Merged
merged 4 commits into from
Dec 18, 2024
Merged
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
71 changes: 53 additions & 18 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,25 +718,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
BindingMode(def_br, Mutability::Mut)
} else {
// `mut` resets the binding mode on edition <= 2021
*self
.typeck_results
.borrow_mut()
.rust_2024_migration_desugared_pats_mut()
.entry(pat_info.top_info.hir_id)
.or_default() |= pat.span.at_least_rust_2024();
self.add_rust_2024_migration_desugared_pat(
pat_info.top_info.hir_id,
pat.span,
ident.span,
"requires binding by-value, but the implicit default is by-reference",
);
BindingMode(ByRef::No, Mutability::Mut)
}
}
BindingMode(ByRef::No, mutbl) => BindingMode(def_br, mutbl),
BindingMode(ByRef::Yes(_), _) => {
if matches!(def_br, ByRef::Yes(_)) {
// `ref`/`ref mut` overrides the binding mode on edition <= 2021
*self
.typeck_results
.borrow_mut()
.rust_2024_migration_desugared_pats_mut()
.entry(pat_info.top_info.hir_id)
.or_default() |= pat.span.at_least_rust_2024();
self.add_rust_2024_migration_desugared_pat(
pat_info.top_info.hir_id,
pat.span,
ident.span,
"cannot override to bind by-reference when that is the implicit default",
);
}
user_bind_annot
}
Expand Down Expand Up @@ -2266,12 +2266,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Reset binding mode on old editions
if pat_info.binding_mode != ByRef::No {
pat_info.binding_mode = ByRef::No;
*self
.typeck_results
.borrow_mut()
.rust_2024_migration_desugared_pats_mut()
.entry(pat_info.top_info.hir_id)
.or_default() |= pat.span.at_least_rust_2024();
self.add_rust_2024_migration_desugared_pat(
pat_info.top_info.hir_id,
pat.span,
inner.span,
"cannot implicitly match against multiple layers of reference",
)
}
}

Expand Down Expand Up @@ -2630,4 +2630,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => (false, ty),
}
}

/// Record a pattern that's invalid under Rust 2024 match ergonomics, along with a problematic
/// span, so that the pattern migration lint can desugar it during THIR construction.
fn add_rust_2024_migration_desugared_pat(
&self,
pat_id: HirId,
subpat_span: Span,
cutoff_span: Span,
detailed_label: &str,
) {
// Try to trim the span we're labeling to just the `&` or binding mode that's an issue.
// If the subpattern's span is is from an expansion, the emitted label will not be trimmed.
let source_map = self.tcx.sess.source_map();
let cutoff_span = source_map
.span_extend_prev_while(cutoff_span, char::is_whitespace)
.unwrap_or(cutoff_span);
// Ensure we use the syntax context and thus edition of `subpat_span`; this will be a hard
// error if the subpattern is of edition >= 2024.
let trimmed_span = subpat_span.until(cutoff_span).with_ctxt(subpat_span.ctxt());

// 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.
let desc = if subpat_span.from_expansion() {
"default binding mode is reset within expansion"
} else {
detailed_label
};

self.typeck_results
.borrow_mut()
.rust_2024_migration_desugared_pats_mut()
.entry(pat_id)
.or_default()
.push((trimmed_span, desc.to_owned()));
Copy link
Member

@compiler-errors compiler-errors Dec 16, 2024

Choose a reason for hiding this comment

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

I'm not totally certain if the span logic is resilient enough to ensure that a trimmed/modified span will always have the same edition as one of the input spans. Like, pat_span may be from edition 2024 and inner_span may be from 2021, and it's not totally deterministic what we'd be doing in that case.

To avoid worrying about this subtle change in behavior, could you record the pat_span (which is the span from which we get the definition in the implementation today) or perhaps just its edition in this table too, so we don't implicitly rely on trimmed_span's edition which may be totally random depending on the impl of Span::until and SourceMap::span_extend_prev_while.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, yeah. good point. I've opted to re-add the syntax context from pat.span to the trimmed span, to avoid needing a separate span or boolean field (and thus a tuple or struct). using Span::trim_end would be a less roundabout way of doing this, since it just uses SpanData::with_hi to do the trimming, but that would rely on Span::trim_end keeping this behavior. we could also inline it to use with_hi directly, but that's less readable.

}
}
13 changes: 8 additions & 5 deletions compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ pub struct TypeckResults<'tcx> {
pat_binding_modes: ItemLocalMap<BindingMode>,

/// Top-level patterns whose match ergonomics need to be desugared by the Rust 2021 -> 2024
/// migration lint. The boolean indicates whether the emitted diagnostic should be a hard error
/// (if any of the incompatible pattern elements are in edition 2024).
rust_2024_migration_desugared_pats: ItemLocalMap<bool>,
/// migration lint. Problematic subpatterns are stored in the `Vec` for the lint to highlight.
rust_2024_migration_desugared_pats: ItemLocalMap<Vec<(Span, String)>>,

/// Stores the types which were implicitly dereferenced in pattern binding modes
/// for later usage in THIR lowering. For example,
Expand Down Expand Up @@ -419,14 +418,18 @@ impl<'tcx> TypeckResults<'tcx> {
LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.pat_adjustments }
}

pub fn rust_2024_migration_desugared_pats(&self) -> LocalTableInContext<'_, bool> {
pub fn rust_2024_migration_desugared_pats(
&self,
) -> LocalTableInContext<'_, Vec<(Span, String)>> {
LocalTableInContext {
hir_owner: self.hir_owner,
data: &self.rust_2024_migration_desugared_pats,
}
}

pub fn rust_2024_migration_desugared_pats_mut(&mut self) -> LocalTableInContextMut<'_, bool> {
pub fn rust_2024_migration_desugared_pats_mut(
&mut self,
) -> LocalTableInContextMut<'_, Vec<(Span, String)>> {
LocalTableInContextMut {
hir_owner: self.hir_owner,
data: &mut self.rust_2024_migration_desugared_pats,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ 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 = patterns are not allowed to reset the default binding mode in edition 2024
mir_build_rust_2024_incompatible_pat = this pattern relies on behavior which may change in edition 2024

mir_build_rustc_box_attribute_error = `#[rustc_box]` attribute used incorrectly
.attributes = no other attributes may be applied
Expand Down
28 changes: 20 additions & 8 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level,
MultiSpan, SubdiagMessageOp, Subdiagnostic,
MultiSpan, SubdiagMessageOp, Subdiagnostic, pluralize,
};
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::{self, Ty};
Expand Down Expand Up @@ -1089,18 +1089,20 @@ pub(crate) enum RustcBoxAttrReason {

#[derive(LintDiagnostic)]
#[diag(mir_build_rust_2024_incompatible_pat)]
pub(crate) struct Rust2024IncompatiblePat {
pub(crate) struct Rust2024IncompatiblePat<'a> {
#[subdiagnostic]
pub(crate) sugg: Rust2024IncompatiblePatSugg,
pub(crate) sugg: Rust2024IncompatiblePatSugg<'a>,
}

pub(crate) struct Rust2024IncompatiblePatSugg {
pub(crate) struct Rust2024IncompatiblePatSugg<'a> {
pub(crate) suggestion: Vec<(Span, String)>,
/// Whether the incompatibility is a hard error because a relevant span is in edition 2024.
pub(crate) is_hard_error: bool,
pub(crate) ref_pattern_count: usize,
pub(crate) binding_mode_count: usize,
/// Labeled spans for subpatterns invalid in Rust 2024.
pub(crate) labels: &'a [(Span, String)],
}

impl Subdiagnostic for Rust2024IncompatiblePatSugg {
impl<'a> Subdiagnostic for Rust2024IncompatiblePatSugg<'a> {
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
Expand All @@ -1112,6 +1114,16 @@ impl Subdiagnostic for Rust2024IncompatiblePatSugg {
} else {
Applicability::MaybeIncorrect
};
diag.multipart_suggestion("desugar the match ergonomics", self.suggestion, applicability);
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()
};
diag.multipart_suggestion_verbose(
format!("make the implied reference pattern{plural_derefs}{and_modes} explicit"),
self.suggestion,
applicability,
);
}
}
29 changes: 22 additions & 7 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod const_to_pat;
use std::cmp::Ordering;

use rustc_abi::{FieldIdx, Integer};
use rustc_errors::MultiSpan;
use rustc_errors::codes::*;
use rustc_hir::def::{CtorOf, DefKind, Res};
use rustc_hir::pat_util::EnumerateAndAdjustIterator;
Expand Down Expand Up @@ -34,7 +35,7 @@ struct PatCtxt<'a, 'tcx> {
typeck_results: &'a ty::TypeckResults<'tcx>,

/// Used by the Rust 2024 migration lint.
rust_2024_migration_suggestion: Option<Rust2024IncompatiblePatSugg>,
rust_2024_migration_suggestion: Option<Rust2024IncompatiblePatSugg<'a>>,
}

pub(super) fn pat_from_hir<'a, 'tcx>(
Expand All @@ -50,24 +51,36 @@ pub(super) fn pat_from_hir<'a, 'tcx>(
rust_2024_migration_suggestion: typeck_results
.rust_2024_migration_desugared_pats()
.get(pat.hir_id)
.map(|&is_hard_error| Rust2024IncompatiblePatSugg {
.map(|labels| Rust2024IncompatiblePatSugg {
suggestion: Vec::new(),
is_hard_error,
ref_pattern_count: 0,
binding_mode_count: 0,
labels: labels.as_slice(),
}),
};
let result = pcx.lower_pattern(pat);
debug!("pat_from_hir({:?}) = {:?}", pat, result);
if let Some(sugg) = pcx.rust_2024_migration_suggestion {
if sugg.is_hard_error {
let mut spans = MultiSpan::from_spans(sugg.labels.iter().map(|(span, _)| *span).collect());
for (span, label) in sugg.labels {
spans.push_span_label(*span, label.clone());
}
// 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());
if is_hard_error {
let mut err =
tcx.dcx().struct_span_err(pat.span, fluent::mir_build_rust_2024_incompatible_pat);
tcx.dcx().struct_span_err(spans, fluent::mir_build_rust_2024_incompatible_pat);
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));
}
err.subdiagnostic(sugg);
err.emit();
} else {
tcx.emit_node_span_lint(
lint::builtin::RUST_2024_INCOMPATIBLE_PAT,
pat.hir_id,
pat.span,
spans,
Rust2024IncompatiblePat { sugg },
);
}
Expand Down Expand Up @@ -133,6 +146,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
})
.collect();
s.suggestion.push((pat.span.shrink_to_lo(), suggestion_str));
s.ref_pattern_count += adjustments.len();
};

adjusted_pat
Expand Down Expand Up @@ -371,7 +385,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
s.suggestion.push((
pat.span.with_lo(ident.span.lo()).shrink_to_lo(),
sugg_str.to_owned(),
))
));
s.binding_mode_count += 1;
}

// A ref x pattern is the same node used for x, and as such it has
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ fn main() {
assert_type_eq(x, &mut 0u8);

let &Foo(mut x) = &Foo(0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

let &mut Foo(mut x) = &mut Foo(0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

let &Foo(ref x) = &Foo(0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, &0u8);

let &mut Foo(ref x) = &mut Foo(0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, &0u8);

Expand All @@ -55,22 +55,22 @@ fn main() {
assert_type_eq(x, &0u8);

let &Foo(&x) = &Foo(&0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

let &Foo(&mut x) = &Foo(&mut 0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

let &mut Foo(&x) = &mut Foo(&0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

let &mut Foo(&mut x) = &mut Foo(&mut 0);
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);

Expand All @@ -79,25 +79,25 @@ fn main() {
}

if let &&&&&Some(&x) = &&&&&Some(&0u8) {
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);
}

if let &&&&&Some(&mut x) = &&&&&Some(&mut 0u8) {
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);
}

if let &&&&&mut Some(&x) = &&&&&mut Some(&0u8) {
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, 0u8);
}

if let &mut Some(&mut Some(&mut Some(ref mut x))) = &mut Some(&mut Some(&mut Some(0u8))) {
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(x, &mut 0u8);
}
Expand All @@ -109,20 +109,20 @@ fn main() {
}

let &Struct { ref a, mut b, ref c } = &Struct { a: 0, b: 0, c: 0 };
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(a, &0u32);
assert_type_eq(b, 0u32);

let &Struct { a: &a, ref b, ref c } = &Struct { a: &0, b: &0, c: &0 };
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
assert_type_eq(a, 0u32);
assert_type_eq(b, &&0u32);
assert_type_eq(c, &&0u32);

if let &Struct { a: &Some(a), b: &Some(&b), c: &Some(ref c) } =
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
//~| WARN: this changes meaning in Rust 2024
&(Struct { a: &Some(&0), b: &Some(&0), c: &Some(&0) })
{
Expand All @@ -135,7 +135,7 @@ fn main() {
// The two patterns are the same syntactically, but because they're defined in different
// editions they don't mean the same thing.
&(Some(mut x), migration_lint_macros::mixed_edition_pat!(y)) => {
//~^ ERROR: patterns are not allowed to reset the default binding mode
//~^ ERROR: this pattern relies on behavior which may change in edition 2024
assert_type_eq(x, 0u32);
assert_type_eq(y, 0u32);
}
Expand Down
Loading
Loading