Skip to content

rustc_pattern_analysis: always check that deref patterns don't match on the same place as normal constructors #143472

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 2 commits 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
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/builder/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scrut_span: rustc_span::Span::default(),
refutable: true,
known_valid_scrutinee: true,
internal_state: Default::default(),
};

let valtree = match self.eval_unevaluated_mir_constant_to_valtree(constant) {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
scrut_span,
refutable,
known_valid_scrutinee,
internal_state: Default::default(),
}
}

Expand Down
50 changes: 50 additions & 0 deletions compiler/rustc_pattern_analysis/src/checks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//! Contains checks that must be run to validate matches before performing usefulness analysis.
use crate::constructor::Constructor::*;
use crate::pat_column::PatternColumn;
use crate::{MatchArm, PatCx};

/// Validate that deref patterns and normal constructors aren't used to match on the same place.
pub(crate) fn detect_mixed_deref_pat_ctors<'p, Cx: PatCx>(
cx: &Cx,
arms: &[MatchArm<'p, Cx>],
) -> Result<(), Cx::Error> {
let pat_column = PatternColumn::new(arms);
detect_mixed_deref_pat_ctors_inner(cx, &pat_column)
}

fn detect_mixed_deref_pat_ctors_inner<'p, Cx: PatCx>(
cx: &Cx,
column: &PatternColumn<'p, Cx>,
) -> Result<(), Cx::Error> {
let Some(ty) = column.head_ty() else {
return Ok(());
};

// Check for a mix of deref patterns and normal constructors.
let mut deref_pat = None;
let mut normal_pat = None;
for pat in column.iter() {
match pat.ctor() {
// The analysis can handle mixing deref patterns with wildcards and opaque patterns.
Wildcard | Opaque(_) => {}
DerefPattern(_) => deref_pat = Some(pat),
// Nothing else can be compared to deref patterns in `Constructor::is_covered_by`.
_ => normal_pat = Some(pat),
}
}
if let Some(deref_pat) = deref_pat
&& let Some(normal_pat) = normal_pat
{
return Err(cx.report_mixed_deref_pat_ctors(deref_pat, normal_pat));
}

// Specialize and recurse into the patterns' fields.
let set = column.analyze_ctors(cx, &ty)?;
for ctor in set.present {
for specialized_column in column.specialize(cx, &ty, &ctor).iter() {
detect_mixed_deref_pat_ctors_inner(cx, specialized_column)?;
}
}
Ok(())
}
15 changes: 15 additions & 0 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#![allow(unused_crate_dependencies)]
// tidy-alphabetical-end

pub(crate) mod checks;
pub mod constructor;
#[cfg(feature = "rustc")]
pub mod errors;
Expand Down Expand Up @@ -107,6 +108,20 @@ pub trait PatCx: Sized + fmt::Debug {
_gapped_with: &[&DeconstructedPat<Self>],
) {
}

/// Check if we may need to perform additional deref-pattern-specific validation.
fn match_may_contain_deref_pats(&self) -> bool {
true
}

/// The current implementation of deref patterns requires that they can't match on the same
/// place as a normal constructor. Since this isn't caught by type-checking, we check it in the
/// `PatCx` before running the analysis. This reports an error if the check fails.
fn report_mixed_deref_pat_ctors(
&self,
deref_pat: &DeconstructedPat<Self>,
normal_pat: &DeconstructedPat<Self>,
) -> Self::Error;
}

/// The arm of a match expression.
Expand Down
83 changes: 31 additions & 52 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cell::Cell;
use std::fmt;
use std::iter::once;

Expand Down Expand Up @@ -99,6 +100,16 @@ pub struct RustcPatCtxt<'p, 'tcx: 'p> {
/// Whether the data at the scrutinee is known to be valid. This is false if the scrutinee comes
/// from a union field, a pointer deref, or a reference deref (pending opsem decisions).
pub known_valid_scrutinee: bool,
pub internal_state: RustcPatCtxtState,
}

/// Private fields of [`RustcPatCtxt`], separated out to permit record initialization syntax.
#[derive(Clone, Default)]
pub struct RustcPatCtxtState {
/// Has a deref pattern been lowered? This is initialized to `false` and is updated by
/// [`RustcPatCtxt::lower_pat`] in order to avoid performing deref-pattern-specific validation
/// for everything containing patterns.
has_lowered_deref_pat: Cell<bool>,
}

impl<'p, 'tcx: 'p> fmt::Debug for RustcPatCtxt<'p, 'tcx> {
Expand Down Expand Up @@ -474,6 +485,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
fields = vec![self.lower_pat(subpattern).at_index(0)];
arity = 1;
ctor = DerefPattern(cx.reveal_opaque_ty(subpattern.ty));
self.internal_state.has_lowered_deref_pat.set(true);
}
PatKind::Leaf { subpatterns } | PatKind::Variant { subpatterns, .. } => {
match ty.kind() {
Expand Down Expand Up @@ -1027,6 +1039,25 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> {
);
}
}

fn match_may_contain_deref_pats(&self) -> bool {
self.internal_state.has_lowered_deref_pat.get()
}

fn report_mixed_deref_pat_ctors(
&self,
deref_pat: &crate::pat::DeconstructedPat<Self>,
normal_pat: &crate::pat::DeconstructedPat<Self>,
) -> Self::Error {
let deref_pattern_label = deref_pat.data().span;
let normal_constructor_label = normal_pat.data().span;
self.tcx.dcx().emit_err(errors::MixedDerefPatternConstructors {
spans: vec![deref_pattern_label, normal_constructor_label],
smart_pointer_ty: deref_pat.ty().inner(),
deref_pattern_label,
normal_constructor_label,
})
}
}

/// Recursively expand this pattern into its subpatterns. Only useful for or-patterns.
Expand Down Expand Up @@ -1055,13 +1086,6 @@ pub fn analyze_match<'p, 'tcx>(
) -> Result<UsefulnessReport<'p, 'tcx>, ErrorGuaranteed> {
let scrut_ty = tycx.reveal_opaque_ty(scrut_ty);

// The analysis doesn't support deref patterns mixed with normal constructors; error if present.
// FIXME(deref_patterns): This only needs to run when a deref pattern was found during lowering.
if tycx.tcx.features().deref_patterns() {
let pat_column = PatternColumn::new(arms);
detect_mixed_deref_pat_ctors(tycx, &pat_column)?;
}

let scrut_validity = PlaceValidity::from_bool(tycx.known_valid_scrutinee);
let report = compute_match_usefulness(
tycx,
Expand All @@ -1080,48 +1104,3 @@ pub fn analyze_match<'p, 'tcx>(

Ok(report)
}

// FIXME(deref_patterns): Currently it's the responsibility of the frontend (rustc or rust-analyzer)
// to ensure that deref patterns don't appear in the same column as normal constructors. Deref
// patterns aren't currently implemented in rust-analyzer, but should they be, the columnwise check
// here could be made generic and shared between frontends.
fn detect_mixed_deref_pat_ctors<'p, 'tcx>(
cx: &RustcPatCtxt<'p, 'tcx>,
column: &PatternColumn<'p, RustcPatCtxt<'p, 'tcx>>,
) -> Result<(), ErrorGuaranteed> {
let Some(&ty) = column.head_ty() else {
return Ok(());
};

// Check for a mix of deref patterns and normal constructors.
let mut normal_ctor_span = None;
let mut deref_pat_span = None;
for pat in column.iter() {
match pat.ctor() {
// The analysis can handle mixing deref patterns with wildcards and opaque patterns.
Wildcard | Opaque(_) => {}
DerefPattern(_) => deref_pat_span = Some(pat.data().span),
// Nothing else can be compared to deref patterns in `Constructor::is_covered_by`.
_ => normal_ctor_span = Some(pat.data().span),
}
}
if let Some(normal_constructor_label) = normal_ctor_span
&& let Some(deref_pattern_label) = deref_pat_span
{
return Err(cx.tcx.dcx().emit_err(errors::MixedDerefPatternConstructors {
spans: vec![deref_pattern_label, normal_constructor_label],
smart_pointer_ty: ty.inner(),
deref_pattern_label,
normal_constructor_label,
}));
}

// Specialize and recurse into the patterns' fields.
let set = column.analyze_ctors(cx, &ty)?;
for ctor in set.present {
for specialized_column in column.specialize(cx, &ty, &ctor).iter() {
detect_mixed_deref_pat_ctors(cx, specialized_column)?;
}
}
Ok(())
}
7 changes: 6 additions & 1 deletion compiler/rustc_pattern_analysis/src/usefulness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ use tracing::{debug, instrument};
use self::PlaceValidity::*;
use crate::constructor::{Constructor, ConstructorSet, IntRange};
use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat};
use crate::{MatchArm, PatCx, PrivateUninhabitedField};
use crate::{MatchArm, PatCx, PrivateUninhabitedField, checks};
#[cfg(not(feature = "rustc"))]
pub fn ensure_sufficient_stack<R>(f: impl FnOnce() -> R) -> R {
f()
Expand Down Expand Up @@ -1836,6 +1836,11 @@ pub fn compute_match_usefulness<'p, Cx: PatCx>(
scrut_validity: PlaceValidity,
complexity_limit: usize,
) -> Result<UsefulnessReport<'p, Cx>, Cx::Error> {
// The analysis doesn't support deref patterns mixed with normal constructors; error if present.
if tycx.match_may_contain_deref_pats() {
checks::detect_mixed_deref_pat_ctors(tycx, arms)?;
}

let mut cx = UsefulnessCtxt {
tycx,
branch_usefulness: FxHashMap::default(),
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_pattern_analysis/tests/common/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc_pattern_analysis::constructor::{
Constructor, ConstructorSet, IntRange, MaybeInfiniteInt, RangeEnd, VariantVisibility,
};
use rustc_pattern_analysis::pat::DeconstructedPat;
use rustc_pattern_analysis::usefulness::{PlaceValidity, UsefulnessReport};
use rustc_pattern_analysis::{MatchArm, PatCx, PrivateUninhabitedField};

Expand Down Expand Up @@ -184,6 +185,14 @@ impl PatCx for Cx {
fn complexity_exceeded(&self) -> Result<(), Self::Error> {
Err(())
}

fn report_mixed_deref_pat_ctors(
&self,
_deref_pat: &DeconstructedPat<Self>,
_normal_pat: &DeconstructedPat<Self>,
) -> Self::Error {
panic!("`rustc_pattern_analysis::tests` currently doesn't test deref pattern errors")
}
}

/// Construct a single pattern; see `pats!()`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,14 @@ impl PatCx for MatchCheckCtx<'_> {
fn complexity_exceeded(&self) -> Result<(), Self::Error> {
Err(())
}

fn report_mixed_deref_pat_ctors(
&self,
_deref_pat: &DeconstructedPat<'_>,
_normal_pat: &DeconstructedPat<'_>,
) {
// FIXME(deref_patterns): This could report an error comparable to the one in rustc.
}
}

impl fmt::Debug for MatchCheckCtx<'_> {
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/pattern/box-pattern-constructor-mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//! Test that `box _` patterns and `Box { .. }` patterns can't be used to match on the same place.
//! This is required for the current implementation of exhaustiveness analysis for deref patterns.

#![feature(box_patterns)]

fn main() {
match Box::new(0) {
box _ => {} //~ ERROR mix of deref patterns and normal constructors
Box { .. } => {}
}
}
10 changes: 10 additions & 0 deletions tests/ui/pattern/box-pattern-constructor-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: mix of deref patterns and normal constructors
--> $DIR/box-pattern-constructor-mismatch.rs:8:9
|
LL | box _ => {}
| ^^^^^ matches on the result of dereferencing `Box<i32>`
LL | Box { .. } => {}
| ^^^^^^^^^^ matches directly on `Box<i32>`

error: aborting due to 1 previous error

Loading