From e1fc4a997ddac2b949ef6bb4037f1fb196c44f08 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 28 Jul 2024 15:12:14 +1000 Subject: Don't store `thir::Pat` in error structs In several cases this avoids the need to clone the underlying pattern, and then print the clone later. --- compiler/rustc_pattern_analysis/src/errors.rs | 53 +++++++++++++-------------- 1 file changed, 26 insertions(+), 27 deletions(-) (limited to 'compiler/rustc_pattern_analysis/src/errors.rs') diff --git a/compiler/rustc_pattern_analysis/src/errors.rs b/compiler/rustc_pattern_analysis/src/errors.rs index 27f227e6d9c..bad41ac77f0 100644 --- a/compiler/rustc_pattern_analysis/src/errors.rs +++ b/compiler/rustc_pattern_analysis/src/errors.rs @@ -1,6 +1,5 @@ use rustc_errors::{Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic}; use rustc_macros::{LintDiagnostic, Subdiagnostic}; -use rustc_middle::thir::Pat; use rustc_middle::ty::Ty; use rustc_span::Span; @@ -8,18 +7,18 @@ use crate::rustc::{RustcPatCtxt, WitnessPat}; #[derive(Subdiagnostic)] #[label(pattern_analysis_uncovered)] -pub struct Uncovered<'tcx> { +pub struct Uncovered { #[primary_span] span: Span, count: usize, - witness_1: Pat<'tcx>, - witness_2: Pat<'tcx>, - witness_3: Pat<'tcx>, + witness_1: String, // a printed pattern + witness_2: String, // a printed pattern + witness_3: String, // a printed pattern remainder: usize, } -impl<'tcx> Uncovered<'tcx> { - pub fn new<'p>( +impl Uncovered { + pub fn new<'p, 'tcx>( span: Span, cx: &RustcPatCtxt<'p, 'tcx>, witnesses: Vec>, @@ -27,19 +26,19 @@ impl<'tcx> Uncovered<'tcx> { where 'tcx: 'p, { - let witness_1 = cx.hoist_witness_pat(witnesses.get(0).unwrap()); + let witness_1 = cx.hoist_witness_pat(witnesses.get(0).unwrap()).to_string(); Self { span, count: witnesses.len(), // Substitute dummy values if witnesses is smaller than 3. These will never be read. witness_2: witnesses .get(1) - .map(|w| cx.hoist_witness_pat(w)) - .unwrap_or_else(|| witness_1.clone()), + .map(|w| cx.hoist_witness_pat(w).to_string()) + .unwrap_or_default(), witness_3: witnesses .get(2) - .map(|w| cx.hoist_witness_pat(w)) - .unwrap_or_else(|| witness_1.clone()), + .map(|w| cx.hoist_witness_pat(w).to_string()) + .unwrap_or_default(), witness_1, remainder: witnesses.len().saturating_sub(3), } @@ -49,19 +48,19 @@ impl<'tcx> Uncovered<'tcx> { #[derive(LintDiagnostic)] #[diag(pattern_analysis_overlapping_range_endpoints)] #[note] -pub struct OverlappingRangeEndpoints<'tcx> { +pub struct OverlappingRangeEndpoints { #[label] pub range: Span, #[subdiagnostic] - pub overlap: Vec>, + pub overlap: Vec, } -pub struct Overlap<'tcx> { +pub struct Overlap { pub span: Span, - pub range: Pat<'tcx>, + pub range: String, // a printed pattern } -impl<'tcx> Subdiagnostic for Overlap<'tcx> { +impl Subdiagnostic for Overlap { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, @@ -78,38 +77,38 @@ impl<'tcx> Subdiagnostic for Overlap<'tcx> { #[derive(LintDiagnostic)] #[diag(pattern_analysis_excluside_range_missing_max)] -pub struct ExclusiveRangeMissingMax<'tcx> { +pub struct ExclusiveRangeMissingMax { #[label] #[suggestion(code = "{suggestion}", applicability = "maybe-incorrect")] /// This is an exclusive range that looks like `lo..max` (i.e. doesn't match `max`). pub first_range: Span, /// Suggest `lo..=max` instead. pub suggestion: String, - pub max: Pat<'tcx>, + pub max: String, // a printed pattern } #[derive(LintDiagnostic)] #[diag(pattern_analysis_excluside_range_missing_gap)] -pub struct ExclusiveRangeMissingGap<'tcx> { +pub struct ExclusiveRangeMissingGap { #[label] #[suggestion(code = "{suggestion}", applicability = "maybe-incorrect")] /// This is an exclusive range that looks like `lo..gap` (i.e. doesn't match `gap`). pub first_range: Span, - pub gap: Pat<'tcx>, + pub gap: String, // a printed pattern /// Suggest `lo..=gap` instead. pub suggestion: String, #[subdiagnostic] /// All these ranges skipped over `gap` which we think is probably a mistake. - pub gap_with: Vec>, + pub gap_with: Vec, } -pub struct GappedRange<'tcx> { +pub struct GappedRange { pub span: Span, - pub gap: Pat<'tcx>, - pub first_range: Pat<'tcx>, + pub gap: String, // a printed pattern + pub first_range: String, // a printed pattern } -impl<'tcx> Subdiagnostic for GappedRange<'tcx> { +impl Subdiagnostic for GappedRange { fn add_to_diag_with>( self, diag: &mut Diag<'_, G>, @@ -134,7 +133,7 @@ impl<'tcx> Subdiagnostic for GappedRange<'tcx> { pub(crate) struct NonExhaustiveOmittedPattern<'tcx> { pub scrut_ty: Ty<'tcx>, #[subdiagnostic] - pub uncovered: Uncovered<'tcx>, + pub uncovered: Uncovered, } #[derive(LintDiagnostic)] -- cgit 1.4.1-3-g733a5 From db05b0fd343e8204460b0e1f3e261cc4ae940e0a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 28 Jul 2024 15:14:26 +1000 Subject: Encapsulate the printing of `WitnessPat` This hides the fact that we print `WitnessPat` by converting it to `thir::Pat` and then printing that. --- compiler/rustc_mir_build/src/thir/pattern/check_match.rs | 8 ++++---- compiler/rustc_pattern_analysis/src/errors.rs | 12 +++--------- compiler/rustc_pattern_analysis/src/rustc.rs | 14 +++++++++++--- 3 files changed, 18 insertions(+), 16 deletions(-) (limited to 'compiler/rustc_pattern_analysis/src/errors.rs') diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index 5e904057e73..0632911671e 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -1077,7 +1077,7 @@ fn report_non_exhaustive_match<'p, 'tcx>( let suggested_arm = if suggest_the_witnesses { let pattern = witnesses .iter() - .map(|witness| cx.hoist_witness_pat(witness).to_string()) + .map(|witness| cx.print_witness_pat(witness)) .collect::>() .join(" | "); if witnesses.iter().all(|p| p.is_never_pattern()) && cx.tcx.features().never_patterns { @@ -1195,13 +1195,13 @@ fn joined_uncovered_patterns<'p, 'tcx>( witnesses: &[WitnessPat<'p, 'tcx>], ) -> String { const LIMIT: usize = 3; - let pat_to_str = |pat: &WitnessPat<'p, 'tcx>| cx.hoist_witness_pat(pat).to_string(); + let pat_to_str = |pat: &WitnessPat<'p, 'tcx>| cx.print_witness_pat(pat); match witnesses { [] => bug!(), - [witness] => format!("`{}`", cx.hoist_witness_pat(witness)), + [witness] => format!("`{}`", cx.print_witness_pat(witness)), [head @ .., tail] if head.len() < LIMIT => { let head: Vec<_> = head.iter().map(pat_to_str).collect(); - format!("`{}` and `{}`", head.join("`, `"), cx.hoist_witness_pat(tail)) + format!("`{}` and `{}`", head.join("`, `"), cx.print_witness_pat(tail)) } _ => { let (head, tail) = witnesses.split_at(LIMIT); diff --git a/compiler/rustc_pattern_analysis/src/errors.rs b/compiler/rustc_pattern_analysis/src/errors.rs index bad41ac77f0..1f7852e5190 100644 --- a/compiler/rustc_pattern_analysis/src/errors.rs +++ b/compiler/rustc_pattern_analysis/src/errors.rs @@ -26,19 +26,13 @@ impl Uncovered { where 'tcx: 'p, { - let witness_1 = cx.hoist_witness_pat(witnesses.get(0).unwrap()).to_string(); + let witness_1 = cx.print_witness_pat(witnesses.get(0).unwrap()); Self { span, count: witnesses.len(), // Substitute dummy values if witnesses is smaller than 3. These will never be read. - witness_2: witnesses - .get(1) - .map(|w| cx.hoist_witness_pat(w).to_string()) - .unwrap_or_default(), - witness_3: witnesses - .get(2) - .map(|w| cx.hoist_witness_pat(w).to_string()) - .unwrap_or_default(), + witness_2: witnesses.get(1).map(|w| cx.print_witness_pat(w)).unwrap_or_default(), + witness_3: witnesses.get(2).map(|w| cx.print_witness_pat(w)).unwrap_or_default(), witness_1, remainder: witnesses.len().saturating_sub(3), } diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index ceddafa2f4c..b04c21a384c 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -744,7 +744,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { /// Note: it is possible to get `isize/usize::MAX+1` here, as explained in the doc for /// [`IntRange::split`]. This cannot be represented as a `Const`, so we represent it with /// `PosInfinity`. - pub(crate) fn hoist_pat_range_bdy( + fn hoist_pat_range_bdy( &self, miint: MaybeInfiniteInt, ty: RevealedTy<'tcx>, @@ -775,7 +775,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { } /// Convert back to a `thir::Pat` for diagnostic purposes. - pub(crate) fn hoist_pat_range(&self, range: &IntRange, ty: RevealedTy<'tcx>) -> Pat<'tcx> { + fn hoist_pat_range(&self, range: &IntRange, ty: RevealedTy<'tcx>) -> Pat<'tcx> { use MaybeInfiniteInt::*; let cx = self; let kind = if matches!((range.lo, range.hi), (NegInfinity, PosInfinity)) { @@ -811,9 +811,17 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> { Pat { ty: ty.inner(), span: DUMMY_SP, kind } } + + /// Prints a [`WitnessPat`] to an owned string, for diagnostic purposes. + pub fn print_witness_pat(&self, pat: &WitnessPat<'p, 'tcx>) -> String { + // This works by converting the witness pattern back to a `thir::Pat` + // and then printing that, but callers don't need to know that. + self.hoist_witness_pat(pat).to_string() + } + /// Convert back to a `thir::Pat` for diagnostic purposes. This panics for patterns that don't /// appear in diagnostics, like float ranges. - pub fn hoist_witness_pat(&self, pat: &WitnessPat<'p, 'tcx>) -> Pat<'tcx> { + fn hoist_witness_pat(&self, pat: &WitnessPat<'p, 'tcx>) -> Pat<'tcx> { let cx = self; let is_wildcard = |pat: &Pat<'_>| matches!(pat.kind, PatKind::Wild); let mut subpatterns = pat.iter_fields().map(|p| Box::new(cx.hoist_witness_pat(p))); -- cgit 1.4.1-3-g733a5