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.

Sign up for GitHub

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

Jump to bottom

Use a separate pattern type for rustc_pattern_analysis diagnostics #128430

Merged
merged 3 commits into from
Jul 31, 2024
Merged

Use a separate pattern type for rustc_pattern_analysis diagnostics #128430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2b3256
Print `thir::PatRange`, not its surrounding `thir::Pat`
Zalathar Jul 30, 2024
a9ea85e
Revert "Make `thir::Pat` not implement `fmt::Display` directly"
Zalathar Jul 30, 2024
dd5a8d7
Use a separate pattern type for `rustc_pattern_analysis` diagnostics
Zalathar Jul 30, 2024
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
186 changes: 1 addition & 185 deletions compiler/rustc_middle/src/thir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use rustc_middle::ty::{
TyCtxt, UpvarArgs,
};
use rustc_span::def_id::LocalDefId;
use rustc_span::{sym, ErrorGuaranteed, Span, Symbol, DUMMY_SP};
use rustc_span::{ErrorGuaranteed, Span, Symbol};
use rustc_target::abi::{FieldIdx, Integer, Size, VariantIdx};
use rustc_target::asm::InlineAsmRegOrRegClass;
use tracing::instrument;
Expand Down Expand Up @@ -597,10 +597,6 @@ pub struct Pat<'tcx> {
}

impl<'tcx> Pat<'tcx> {
pub fn wildcard_from_ty(ty: Ty<'tcx>) -> Self {
Pat { ty, span: DUMMY_SP, kind: PatKind::Wild }
}

pub fn simple_ident(&self) -> Option<Symbol> {
match self.kind {
PatKind::Binding {
Expand Down Expand Up @@ -1073,186 +1069,6 @@ impl<'tcx> PatRangeBoundary<'tcx> {
}
}

impl<'tcx> Pat<'tcx> {
/// Prints a [`Pat`] to an owned string, for user-facing diagnostics.
///
/// If we ever switch over to storing subpatterns as `PatId`, this will also
/// need to take a context that can resolve IDs to subpatterns.
pub fn to_string(&self) -> String {
format!("{}", self.display())
}

/// Used internally by [`fmt::Display`] for [`PatDisplay`].
fn display(&self) -> PatDisplay<'_, 'tcx> {
PatDisplay { pat: self }
}
}

/// Wrapper around [`&Pat<'tcx>`][`Pat`] that implements [`fmt::Display`].
///
/// If we ever switch over to storing subpatterns as `PatId`, this will also
/// need to hold a context that can resolve IDs to subpatterns.
struct PatDisplay<'pat, 'tcx> {
pat: &'pat Pat<'tcx>,
}

impl<'pat, 'tcx> fmt::Display for PatDisplay<'pat, 'tcx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let &Self { pat } = self;

// Printing lists is a chore.
let mut first = true;
let mut start_or_continue = |s| {
if first {
first = false;
""
} else {
s
}
};
let mut start_or_comma = || start_or_continue(", ");

match pat.kind {
PatKind::Wild => write!(f, "_"),
PatKind::Never => write!(f, "!"),
PatKind::AscribeUserType { ref subpattern, .. } => {
write!(f, "{}: _", subpattern.display())
}
PatKind::Binding { name, mode, ref subpattern, .. } => {
f.write_str(mode.prefix_str())?;
write!(f, "{name}")?;
if let Some(ref subpattern) = *subpattern {
write!(f, " @ {}", subpattern.display())?;
}
Ok(())
}
PatKind::Variant { ref subpatterns, .. } | PatKind::Leaf { ref subpatterns } => {
let variant_and_name = match pat.kind {
PatKind::Variant { adt_def, variant_index, .. } => ty::tls::with(|tcx| {
let variant = adt_def.variant(variant_index);
let adt_did = adt_def.did();
let name = if tcx.get_diagnostic_item(sym::Option) == Some(adt_did)
|| tcx.get_diagnostic_item(sym::Result) == Some(adt_did)
{
variant.name.to_string()
} else {
format!("{}::{}", tcx.def_path_str(adt_def.did()), variant.name)
};
Some((variant, name))
}),
_ => pat.ty.ty_adt_def().and_then(|adt_def| {
if !adt_def.is_enum() {
ty::tls::with(|tcx| {
Some((adt_def.non_enum_variant(), tcx.def_path_str(adt_def.did())))
})
} else {
None
}
}),
};

if let Some((variant, name)) = &variant_and_name {
write!(f, "{name}")?;

// Only for Adt we can have `S {...}`,
// which we handle separately here.
if variant.ctor.is_none() {
write!(f, " {{ ")?;

let mut printed = 0;
for p in subpatterns {
if let PatKind::Wild = p.pattern.kind {
continue;
}
let name = variant.fields[p.field].name;
write!(f, "{}{}: {}", start_or_comma(), name, p.pattern.display())?;
printed += 1;
}

let is_union = pat.ty.ty_adt_def().is_some_and(|adt| adt.is_union());
if printed < variant.fields.len() && (!is_union || printed == 0) {
write!(f, "{}..", start_or_comma())?;
}

return write!(f, " }}");
}
}

let num_fields =
variant_and_name.as_ref().map_or(subpatterns.len(), |(v, _)| v.fields.len());
if num_fields != 0 || variant_and_name.is_none() {
write!(f, "(")?;
for i in 0..num_fields {
write!(f, "{}", start_or_comma())?;

// Common case: the field is where we expect it.
if let Some(p) = subpatterns.get(i) {
if p.field.index() == i {
write!(f, "{}", p.pattern.display())?;
continue;
}
}

// Otherwise, we have to go looking for it.
if let Some(p) = subpatterns.iter().find(|p| p.field.index() == i) {
write!(f, "{}", p.pattern.display())?;
} else {
write!(f, "_")?;
}
}
write!(f, ")")?;
}

Ok(())
}
PatKind::Deref { ref subpattern } => {
match pat.ty.kind() {
ty::Adt(def, _) if def.is_box() => write!(f, "box ")?,
ty::Ref(_, _, mutbl) => {
write!(f, "&{}", mutbl.prefix_str())?;
}
_ => bug!("{} is a bad Deref pattern type", pat.ty),
}
write!(f, "{}", subpattern.display())
}
PatKind::DerefPattern { ref subpattern, .. } => {
write!(f, "deref!({})", subpattern.display())
}
PatKind::Constant { value } => write!(f, "{value}"),
PatKind::InlineConstant { def: _, ref subpattern } => {
write!(f, "{} (from inline const)", subpattern.display())
}
PatKind::Range(ref range) => write!(f, "{range}"),
PatKind::Slice { ref prefix, ref slice, ref suffix }
| PatKind::Array { ref prefix, ref slice, ref suffix } => {
write!(f, "[")?;
for p in prefix.iter() {
write!(f, "{}{}", start_or_comma(), p.display())?;
}
if let Some(ref slice) = *slice {
write!(f, "{}", start_or_comma())?;
match slice.kind {
PatKind::Wild => {}
_ => write!(f, "{}", slice.display())?,
}
write!(f, "..")?;
}
for p in suffix.iter() {
write!(f, "{}{}", start_or_comma(), p.display())?;
}
write!(f, "]")
}
PatKind::Or { ref pats } => {
for pat in pats.iter() {
write!(f, "{}{}", start_or_continue(" | "), pat.display())?;
}
Ok(())
}
PatKind::Error(_) => write!(f, "<error>"),
}
}
}

// Some nodes are used a lot. Make sure they don't unintentionally get bigger.
#[cfg(target_pointer_width = "64")]
mod size_asserts {
Expand Down
41 changes: 22 additions & 19 deletions compiler/rustc_pattern_analysis/src/rustc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_hir::HirId;
use rustc_index::{Idx, IndexVec};
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::mir::{self, Const};
use rustc_middle::thir::{self, FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::thir::{self, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::{
self, FieldDef, OpaqueTypeKey, ScalarInt, Ty, TyCtxt, TypeVisitableExt, VariantDef,
Expand All @@ -26,6 +26,8 @@ use crate::pat_column::PatternColumn;
use crate::usefulness::{compute_match_usefulness, PlaceValidity};
use crate::{errors, Captures, PatCx, PrivateUninhabitedField};

mod print;

// Re-export rustc-specific versions of all these types.
pub type Constructor<'p, 'tcx> = crate::constructor::Constructor<RustcPatCtxt<'p, 'tcx>>;
pub type ConstructorSet<'p, 'tcx> = crate::constructor::ConstructorSet<RustcPatCtxt<'p, 'tcx>>;
Expand Down Expand Up @@ -773,8 +775,9 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
}
}

/// Convert back to a `thir::Pat` for diagnostic purposes.
fn hoist_pat_range(&self, range: &IntRange, ty: RevealedTy<'tcx>) -> Pat<'tcx> {
/// Convert to a [`print::Pat`] for diagnostic purposes.
fn hoist_pat_range(&self, range: &IntRange, ty: RevealedTy<'tcx>) -> print::Pat<'tcx> {
use print::{Pat, PatKind};
use MaybeInfiniteInt::*;
let cx = self;
let kind = if matches!((range.lo, range.hi), (NegInfinity, PosInfinity)) {
Expand Down Expand Up @@ -808,19 +811,20 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
PatKind::Range(Box::new(PatRange { lo, hi, end, ty: ty.inner() }))
};

Pat { ty: ty.inner(), span: DUMMY_SP, kind }
Pat { ty: ty.inner(), 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`
// This works by converting the witness pattern to a `print::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
/// Convert to a [`print::Pat`] for diagnostic purposes. This panics for patterns that don't
/// appear in diagnostics, like float ranges.
fn hoist_witness_pat(&self, pat: &WitnessPat<'p, 'tcx>) -> Pat<'tcx> {
fn hoist_witness_pat(&self, pat: &WitnessPat<'p, 'tcx>) -> print::Pat<'tcx> {
use print::{FieldPat, Pat, PatKind};
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)));
Expand All @@ -840,15 +844,15 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
// the pattern is a box pattern.
PatKind::Deref { subpattern: subpatterns.next().unwrap() }
}
ty::Adt(adt_def, args) => {
ty::Adt(adt_def, _args) => {
let variant_index = RustcPatCtxt::variant_index_for_adt(&pat.ctor(), *adt_def);
let subpatterns = subpatterns
.enumerate()
.map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern })
.collect();

if adt_def.is_enum() {
PatKind::Variant { adt_def: *adt_def, args, variant_index, subpatterns }
PatKind::Variant { adt_def: *adt_def, variant_index, subpatterns }
} else {
PatKind::Leaf { subpatterns }
}
Expand Down Expand Up @@ -885,7 +889,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
}
}
let suffix: Box<[_]> = subpatterns.collect();
let wild = Pat::wildcard_from_ty(pat.ty().inner());
let wild = Pat { ty: pat.ty().inner(), kind: PatKind::Wild };
PatKind::Slice {
prefix: prefix.into_boxed_slice(),
slice: Some(Box::new(wild)),
Expand All @@ -906,7 +910,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
}
};

Pat { ty: pat.ty().inner(), span: DUMMY_SP, kind }
Pat { ty: pat.ty().inner(), kind }
}
}

Expand Down Expand Up @@ -1003,12 +1007,11 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
}
// `pat` is an exclusive range like `lo..gap`. `gapped_with` contains ranges that start with
// `gap+1`.
let suggested_range: thir::Pat<'_> = {
let suggested_range: String = {
// Suggest `lo..=gap` instead.
let mut suggested_range = thir_pat.clone();
let thir::PatKind::Range(range) = &mut suggested_range.kind else { unreachable!() };
range.end = rustc_hir::RangeEnd::Included;
suggested_range
let mut suggested_range = PatRange::clone(range);
suggested_range.end = rustc_hir::RangeEnd::Included;
suggested_range.to_string()
};
let gap_as_pat = self.hoist_pat_range(&gap, *pat.ty());
if gapped_with.is_empty() {
Expand All @@ -1023,7 +1026,7 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
// That's the gap that isn't covered.
max: gap_as_pat.to_string(),
// Suggest `lo..=max` instead.
suggestion: suggested_range.to_string(),
suggestion: suggested_range,
},
);
} else {
Expand All @@ -1037,15 +1040,15 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
// That's the gap that isn't covered.
gap: gap_as_pat.to_string(),
// Suggest `lo..=gap` instead.
suggestion: suggested_range.to_string(),
suggestion: suggested_range,
// All these ranges skipped over `gap` which we think is probably a
// mistake.
gap_with: gapped_with
.iter()
.map(|pat| errors::GappedRange {
span: pat.data().span,
gap: gap_as_pat.to_string(),
first_range: thir_pat.to_string(),
first_range: range.to_string(),
})
.collect(),
},
Expand Down
Loading
Loading