Skip to content
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
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,14 @@ pub struct PatExtra<'tcx> {
/// the pattern node back to the `DefId` of its original constant.
pub expanded_const: Option<DefId>,

/// If present, the original constant value that this array or slice
/// pattern node was expanded from by `const_to_pat`.
///
/// Match lowering uses this to compare the scrutinee against the original
/// constant as a whole via `PartialEq::eq`, rather than element by
/// element.
pub expanded_const_value: Option<ty::Value<'tcx>>,

/// User-written types that must be preserved into MIR so that they can be
/// checked.
pub ascriptions: Vec<Ascription<'tcx>>,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/src/builder/matches/buckets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
value: case_val,
kind: PatConstKind::Float | PatConstKind::Other,
},
)
| (
TestKind::AggregateEq { value: test_val, .. },
TestableCase::Constant { value: case_val, kind: PatConstKind::Aggregate },
) => {
if test_val == case_val {
fully_matched = true;
Expand Down Expand Up @@ -353,6 +357,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| TestKind::Range { .. }
| TestKind::StringEq { .. }
| TestKind::ScalarEq { .. }
| TestKind::AggregateEq { .. }
| TestKind::Deref { .. },
_,
) => {
Expand Down
126 changes: 100 additions & 26 deletions compiler/rustc_mir_build/src/builder/matches/match_pair.rs
Comment thread
dianne marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,40 @@ use crate::builder::matches::{
FlatPat, MatchPairTree, PatConstKind, PatternExtraData, SliceLenOp, TestableCase,
};

/// Below this length, an array or slice pattern is compared element by element
/// rather than as a single aggregate, since the per-element comparisons are
/// unlikely to be more expensive than a `PartialEq::eq` call.
const AGGREGATE_EQ_MIN_LEN: usize = 4;

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Check if we can use aggregate `PartialEq::eq` comparisons for constant array/slice patterns.
/// This is not possible in const contexts, because `PartialEq` is not const-stable yet.
fn can_use_aggregate_eq(&self) -> bool {
let in_const_context = self.tcx.is_const_fn(self.def_id.to_def_id())
|| !self.tcx.hir_body_owner_kind(self.def_id).is_fn_or_closure();
!in_const_context
}
Comment thread
jakubadamw marked this conversation as resolved.

/// If the given array or slice pattern node was expanded from a constant
/// by `const_to_pat` and an aggregate comparison is both possible and
/// worthwhile, returns the original constant value, so that the scrutinee
/// can be compared against it as a whole via `PartialEq::eq`.
///
/// Note that this deliberately does not apply to hand-written array or
/// slice patterns, which only ever match element by element.
fn aggregate_const_value(
&self,
pattern: &Pat<'tcx>,
element_count: usize,
) -> Option<ty::Value<'tcx>> {
let value = pattern.extra.as_deref()?.expanded_const_value?;
if element_count < AGGREGATE_EQ_MIN_LEN || !self.can_use_aggregate_eq() {
return None;
}
Some(value)
Comment on lines +42 to +46

@dianne dianne Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to check that the element type is such that we'll be using a known PartialEq impl, since we're relying on it being correct and not panicking. For simplicity, I'd suggest keeping this to arrays and slices of bytewise-comparable primitives for now; in those cases, we know that PartialEq will compare aggregates directly:

// SAFETY: All the ordinary integer types have no padding, and are not pointers.
is_bytewise_comparable!(u8, u16, u32, u64, u128, usize, i8, i16, i32, i64, i128, isize);
// SAFETY: These have *niches*, but no *padding* and no *provenance*,
// so we can compare them directly.
is_bytewise_comparable!(bool, char, super::Ordering);

Potentially this could be extended to arbitrary BytewiseEq types, but my understanding is that the most important cases to handle are primitives.

View changes since the review

}
}

/// For an array or slice pattern's subpatterns (prefix/slice/suffix), returns a list
/// of those subpatterns, each paired with a suitably-projected [`PlaceBuilder`].
fn prefix_slice_suffix<'a, 'tcx>(
Expand Down Expand Up @@ -344,10 +378,26 @@ impl<'tcx> InterPat<'tcx> {
_ => None,
};
if let Some(array_len) = array_len {
for (subplace, subpat) in
prefix_slice_suffix(&place_builder, Some(array_len), prefix, slice, suffix)
{
subpats.push(InterPat::lower_thir_pat(cx, subplace, subpat));
// If this pattern was expanded from a constant, compare
// the whole array against that constant at once via
// `PartialEq::eq` rather than element by element.
if let Some(aggregate_value) = cx.aggregate_const_value(pattern, prefix.len()) {
debug_assert!(slice.is_none() && suffix.is_empty());
Some(TestableCase::Constant {
value: aggregate_value,
kind: PatConstKind::Aggregate,
})
} else {
for (subplace, subpat) in prefix_slice_suffix(
&place_builder,
Some(array_len),
prefix,
slice,
suffix,
) {
subpats.push(InterPat::lower_thir_pat(cx, subplace, subpat));
}
None
}
} else {
// If the array length couldn't be determined, ignore the
Expand All @@ -359,33 +409,57 @@ impl<'tcx> InterPat<'tcx> {
pattern.ty
),
);
None
}

None
}
PatKind::Slice { ref prefix, ref slice, ref suffix } => {
for (subplace, subpat) in
prefix_slice_suffix(&place_builder, None, prefix, slice, suffix)
{
subpats.push(InterPat::lower_thir_pat(cx, subplace, subpat));
}

if prefix.is_empty() && slice.is_some() && suffix.is_empty() {
// A slice pattern shaped like `[..]` is irrefutable.
// It can match a slice of any length, so no length test is needed.
None
} else {
// Any other shape of slice pattern requires a length test.
// Slice patterns with a `..` subpattern require a minimum
// length; those without `..` require an exact length.
// If this pattern was expanded from a constant, compare the
// whole slice against that constant at once via
// `PartialEq::eq` after the length check, rather than
// element by element.
if let Some(aggregate_value) = cx.aggregate_const_value(pattern, prefix.len()) {
debug_assert!(slice.is_none() && suffix.is_empty());
subpats.push(InterPat {
place,
testable_case: Some(TestableCase::Constant {
value: aggregate_value,
kind: PatConstKind::Aggregate,
}),
subpats: Vec::new(),
or_subpats: None,
ascriptions: Vec::new(),
binding: None,
pattern_span: pattern.span,
is_never: false,
});
Some(TestableCase::Slice {
len: u64::try_from(prefix.len() + suffix.len()).unwrap(),
op: if slice.is_some() {
SliceLenOp::GreaterOrEqual
} else {
SliceLenOp::Equal
},
len: u64::try_from(prefix.len()).unwrap(),
op: SliceLenOp::Equal,
})
} else {
for (subplace, subpat) in
prefix_slice_suffix(&place_builder, None, prefix, slice, suffix)
{
subpats.push(InterPat::lower_thir_pat(cx, subplace, subpat));
}

if prefix.is_empty() && slice.is_some() && suffix.is_empty() {
// A slice pattern shaped like `[..]` is irrefutable.
// It can match a slice of any length, so no length test is needed.
None
} else {
// Any other shape of slice pattern requires a length test.
// Slice patterns with a `..` subpattern require a minimum
// length; those without `..` require an exact length.
Some(TestableCase::Slice {
len: u64::try_from(prefix.len() + suffix.len()).unwrap(),
op: if slice.is_some() {
SliceLenOp::GreaterOrEqual
} else {
SliceLenOp::Equal
},
})
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_mir_build/src/builder/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,10 @@ enum PatConstKind {
Float,
/// Constant string values, tested via string equality.
String,
/// Constant array or slice values that array/slice patterns were expanded
/// from. Tested by calling `PartialEq::eq` on the whole aggregate at once,
/// rather than comparing element by element.
Aggregate,
/// Any other constant-pattern is usually tested via some kind of equality
/// check. Types that might be encountered here include:
/// - raw pointers derived from integer values
Expand Down Expand Up @@ -1333,6 +1337,10 @@ enum TestKind<'tcx> {
/// Tests the place against a constant using scalar equality.
ScalarEq { value: ty::Value<'tcx> },

/// Tests the place against a constant array or slice using `PartialEq::eq`,
/// comparing the whole aggregate at once rather than element by element.
AggregateEq { value: ty::Value<'tcx> },

/// Test whether the value falls within an inclusive or exclusive range.
Range(Arc<PatRange<'tcx>>),

Expand Down
80 changes: 56 additions & 24 deletions compiler/rustc_mir_build/src/builder/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
TestableCase::Constant { value, kind: PatConstKind::String } => {
TestKind::StringEq { value }
}
TestableCase::Constant { value, kind: PatConstKind::Aggregate } => {
TestKind::AggregateEq { value }
}
TestableCase::Constant { value, kind: PatConstKind::Float | PatConstKind::Other } => {
TestKind::ScalarEq { value }
}
Expand Down Expand Up @@ -137,44 +140,59 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.terminate(block, self.source_info(match_start_span), terminator);
}

TestKind::StringEq { value } => {
TestKind::StringEq { value } | TestKind::AggregateEq { value } => {
let tcx = self.tcx;
let success_block = target_block(TestBranch::Success);
let fail_block = target_block(TestBranch::Failure);

let ref_str_ty = Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, tcx.types.str_);
assert!(ref_str_ty.is_imm_ref_str(), "{ref_str_ty:?}");

// The string constant we're testing against has type `str`, but
// calling `<str as PartialEq>::eq` requires `&str` operands.
//
// Because `str` and `&str` have the same valtree representation,
// we can "cast" to the desired type by just replacing the type.
assert!(value.ty.is_str(), "unexpected value type for StringEq test: {value:?}");
let expected_value = ty::Value { ty: ref_str_ty, valtree: value.valtree };
let inner_ty = value.ty;
if matches!(test.kind, TestKind::StringEq { .. }) {
assert!(
inner_ty.is_str(),
"unexpected value type for StringEq test: {value:?}"
);
}
let ref_ty = Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, inner_ty);

// The constant we're testing against has type `str`, `[T; N]`, or `[T]`,
// but calling `<T as PartialEq>::eq` requires a reference operand
// (`&str`, `&[T; N]`, or `&[T]`). Valtree representations are the same
// with or without the reference wrapper, so we can "cast" to the
// desired type by just replacing the type.
let expected_value = ty::Value { ty: ref_ty, valtree: value.valtree };
let expected_value_operand =
self.literal_operand(test.span, Const::from_ty_value(tcx, expected_value));

// Similarly, the scrutinized place has type `str`, but we need `&str`.
// Get a reference by doing `let actual_value_ref_place: &str = &place`.
let actual_value_ref_place = self.temp(ref_str_ty, test.span);
// Similarly, the scrutinised place has the inner type, but we need a
// reference. Get one by doing `let actual_value_ref_place = &place`.
let actual_value_ref_place = self.temp(ref_ty, test.span);
self.cfg.push_assign(
block,
self.source_info(test.span),
actual_value_ref_place,
Rvalue::Ref(tcx.lifetimes.re_erased, BorrowKind::Shared, place),
);

// Compare two strings using `<str as std::cmp::PartialEq>::eq`.
// (Interestingly this means that exhaustiveness analysis relies, for soundness,
// on the `PartialEq` impl for `str` to be correct!)
self.string_compare(
// Compare the two values using `<T as std::cmp::PartialEq>::eq`.
// (Interestingly this means that, for `str`, exhaustiveness analysis
// relies for soundness on the `PartialEq` impl for `str` to be correct!)
Comment on lines +177 to +178

@dianne dianne Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This applies to aggregates too. We need PartialEq::eq to agree with structural comparison or we may accept non-exhaustive matches.

View changes since the review

//
// The aggregate comparisons, unlike the long-standing string ones, are
// asserted not to unwind, since an unwind edge would make
// borrow-checking stricter than for the `SwitchInt`s they replace.
// That is sound because a constant is only allowed in a pattern if its
// type is structural match, so the array/slice impl and every element
// impl it delegates to are derived or primitive, and cannot panic.
Comment on lines +183 to +185

@dianne dianne Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should only be doing this with element types where we know the PartialEq impl it'll resolve to. In the cases we care about, I don't think we'll be delegating to elements' impls at all? Their PartialEqs should resolve to specialized BytewiseEq impls that compare the aggregates directly. We're relying on the intrinsics compare_bytes and raw_eq used by them not to panic.

In general, being marked StructuralPartialEq doesn't mean we'll be using a derived impl, since it's possible (unstably) to implement it on arbitrary types. It's not an unsafe trait since we always use structural comparison in matches, even if it disagrees with PartialEq.

View changes since the review

let can_unwind = matches!(test.kind, TestKind::StringEq { .. });
self.non_scalar_compare(
block,
success_block,
fail_block,
source_info,
inner_ty,
expected_value_operand,
Operand::Copy(actual_value_ref_place),
can_unwind,
);
}

Expand Down Expand Up @@ -409,19 +427,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
}

/// Compare two values of type `&str` using `<str as std::cmp::PartialEq>::eq`.
fn string_compare(
/// Compare two reference values using `<T as PartialEq>::eq`.
///
/// `compared_ty` is the *inner* type (e.g. `str`, `[u8; 64]`);
/// `expect` and `val` must already be references to that type.
///
/// When `can_unwind` is false, the call is given `UnwindAction::Unreachable`
/// and no unwind edge, asserting that the `PartialEq::eq` implementation
/// cannot panic. This matters beyond codegen: an unwinding call would make
/// borrow-checking of the surrounding match stricter, because the unwind
/// path can create drop-order conflicts that the ordinary path does not
/// have.
fn non_scalar_compare(
&mut self,
block: BasicBlock,
success_block: BasicBlock,
fail_block: BasicBlock,
source_info: SourceInfo,
compared_ty: Ty<'tcx>,
expect: Operand<'tcx>,
val: Operand<'tcx>,
can_unwind: bool,
) {
let str_ty = self.tcx.types.str_;
let eq_def_id = self.tcx.require_lang_item(LangItem::PartialEq, source_info.span);
let method = trait_method(self.tcx, eq_def_id, sym::eq, &[str_ty.into(), str_ty.into()]);
let method =
trait_method(self.tcx, eq_def_id, sym::eq, &[compared_ty.into(), compared_ty.into()]);

let bool_ty = self.tcx.types.bool;
let eq_result = self.temp(bool_ty, source_info.span);
Expand All @@ -448,12 +478,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.into(),
destination: eq_result,
target: Some(eq_block),
unwind: UnwindAction::Continue,
unwind: if can_unwind { UnwindAction::Continue } else { UnwindAction::Unreachable },
call_source: CallSource::MatchCmp,
fn_span: source_info.span,
},
);
self.diverge_from(block);
if can_unwind {
self.diverge_from(block);
}

// check the result
self.cfg.terminate(
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,15 @@ impl<'tcx> ConstToPat<'tcx> {
}
};

Box::new(Pat { span, ty, kind, extra: None })
let mut pat = Box::new(Pat { span, ty, kind, extra: None });
if matches!(ty.kind(), ty::Array(..) | ty::Slice(_)) {
// Record the original constant value on array and slice nodes, so
// that match lowering can compare the scrutinee against the whole
// constant at once via `PartialEq::eq`, rather than element by
// element.
pat.extra.get_or_insert_default().expanded_const_value = Some(value);
}
pat
}
}

Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_mir_build/src/thir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,10 +703,15 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
return;
};

let PatExtra { expanded_const, ascriptions } = extra;
let PatExtra { expanded_const, expanded_const_value, ascriptions } = extra;

print_indented!(self, "extra: PatExtra {", depth_lvl);
print_indented!(self, format_args!("expanded_const: {expanded_const:?}"), depth_lvl + 1);
print_indented!(
self,
format_args!("expanded_const_value: {expanded_const_value:?}"),
depth_lvl + 1
);
self.print_list(
"ascriptions",
ascriptions,
Expand Down
Loading
Loading