Skip to content

Commit 6c8272d

Browse files
committed
don't peel ADTs the pattern could match
This is the use for the previous commits' refactors; see the messages there for more information.
1 parent 323ff3d commit 6c8272d

File tree

4 files changed

+145
-27
lines changed

4 files changed

+145
-27
lines changed

compiler/rustc_hir_typeck/src/pat.rs

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use rustc_hir::{
1616
};
1717
use rustc_infer::infer;
1818
use rustc_middle::traits::PatternOriginExpr;
19-
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
19+
use rustc_middle::ty::{self, AdtDef, Ty, TypeVisitableExt};
2020
use rustc_middle::{bug, span_bug};
2121
use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
2222
use rustc_session::parse::feature_err;
@@ -160,10 +160,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
160160

161161
/// Mode for adjusting the expected type and binding mode.
162162
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
163-
enum AdjustMode {
163+
enum AdjustMode<'tcx> {
164164
/// Peel off all immediate reference types. If the `deref_patterns` feature is enabled, this
165165
/// also peels library-defined smart pointer ADTs.
166-
Peel { kind: PeelKind },
166+
Peel { kind: PeelKind<'tcx> },
167167
/// Reset binding mode to the initial mode.
168168
/// Used for destructuring assignment, where we don't want any match ergonomics.
169169
Reset,
@@ -173,14 +173,22 @@ enum AdjustMode {
173173

174174
/// Restrictions on what types to peel when adjusting the expected type and binding mode.
175175
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
176-
enum PeelKind {
176+
enum PeelKind<'tcx> {
177177
/// Only peel reference types. This is used for explicit deref patterns.
178178
RefOnly,
179179
/// If `deref_patterns` is enabled, this also peels library-defined smart pointer ADTs, except
180180
/// for `until_adt` if the pattern is a constructor. Otherwise this is the same as `RefOnly`.
181181
/// See [`ResolvedPat`] for more information.
182-
// TODO: add `ResolvedPat` and `until_adt`.
183-
Overloaded,
182+
Overloaded { until_adt: Option<AdtDef<'tcx>> },
183+
}
184+
185+
impl<'tcx> AdjustMode<'tcx> {
186+
const fn peel_until_adt(opt_adt_def: Option<AdtDef<'tcx>>) -> AdjustMode<'tcx> {
187+
AdjustMode::Peel { kind: PeelKind::Overloaded { until_adt: opt_adt_def } }
188+
}
189+
const fn peel_all() -> AdjustMode<'tcx> {
190+
AdjustMode::peel_until_adt(None)
191+
}
184192
}
185193

186194
/// `ref mut` bindings (explicit or match-ergonomics) are not allowed behind an `&` reference.
@@ -266,11 +274,12 @@ enum InheritedRefMatchRule {
266274
///
267275
/// `ResolvedPat` contains the information from resolution needed to determine match ergonomics
268276
/// adjustments, plus a callback to finish checking the pattern once we know its adjusted type.
269-
// TODO: add an `adt_def: Option<DefId>` field to handle the `Cow` case above.
270-
struct ResolvedPat<F: ?Sized> {
277+
struct ResolvedPat<'tcx, F: ?Sized> {
271278
/// For path patterns, this must be `Some(res)`, where `res` is the resolution of the pattern's
272279
/// `QPath`. Otherwise, this is `None`.
273280
path_res: Option<Res>,
281+
/// If applicable, identifies the ADT that the pattern matches against.
282+
pat_adt_def: Option<AdtDef<'tcx>>,
274283
/// Given the expected type of the scrutinee and the [`PatInfo`] after applying match ergonomics
275284
/// adjustments, finish checking the pattern.
276285
check: F,
@@ -356,7 +365,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
356365

357366
// For patterns containing paths, we need the path's resolution to determine whether to
358367
// implicitly dereference the scrutinee before matching.
359-
let resolved_pat: Option<&ResolvedPat<dyn Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>>> =
368+
let resolved_pat: Option<&ResolvedPat<'tcx, dyn Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>>> =
360369
match pat.kind {
361370
PatKind::Expr(PatExpr { kind: PatExprKind::Path(qpath), hir_id, span }) => {
362371
Some { 0: &self.check_pat_path(*hir_id, pat.hir_id, *span, qpath, &ti) }
@@ -369,7 +378,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
369378
}
370379
_ => None,
371380
};
372-
let adjust_mode = self.calc_adjust_mode(pat, resolved_pat.and_then(|r| r.path_res));
381+
let adjust_mode = self.calc_adjust_mode(
382+
pat,
383+
resolved_pat.and_then(|r| r.path_res),
384+
resolved_pat.and_then(|r| r.pat_adt_def),
385+
);
373386
let (expected, binding_mode, max_ref_mutbl) =
374387
self.calc_default_binding_mode(pat, expected, binding_mode, adjust_mode, max_ref_mutbl);
375388
let pat_info = PatInfo {
@@ -484,7 +497,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
484497
pat: &'tcx Pat<'tcx>,
485498
expected: Ty<'tcx>,
486499
def_br: ByRef,
487-
adjust_mode: AdjustMode,
500+
adjust_mode: AdjustMode<'tcx>,
488501
max_ref_mutbl: MutblCap,
489502
) -> (Ty<'tcx>, ByRef, MutblCap) {
490503
#[cfg(debug_assertions)]
@@ -506,7 +519,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
506519
/// How should the binding mode and expected type be adjusted?
507520
///
508521
/// When the pattern is a path pattern, `opt_path_res` must be `Some(res)`.
509-
fn calc_adjust_mode(&self, pat: &'tcx Pat<'tcx>, opt_path_res: Option<Res>) -> AdjustMode {
522+
/// When the pattern contains a path (such as a struct pattern or tuple struct pattern),
523+
/// `pat_adt_def` should be `Some(adt)` if the path resolved to an ADT constructor.
524+
fn calc_adjust_mode(
525+
&self,
526+
pat: &'tcx Pat<'tcx>,
527+
opt_path_res: Option<Res>,
528+
pat_adt_def: Option<AdtDef<'tcx>>,
529+
) -> AdjustMode<'tcx> {
510530
// When we perform destructuring assignment, we disable default match bindings, which are
511531
// unintuitive in this context.
512532
if !pat.default_binding_modes {
@@ -519,14 +539,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
519539
| PatKind::TupleStruct(..)
520540
| PatKind::Tuple(..)
521541
| PatKind::Range(..)
522-
| PatKind::Slice(..) => AdjustMode::Peel { kind: PeelKind::Overloaded },
542+
| PatKind::Slice(..) => AdjustMode::peel_until_adt(pat_adt_def),
523543
// When checking an explicit deref pattern, only peel reference types.
524544
// FIXME(deref_patterns): If box patterns and deref patterns need to coexist, box
525545
// patterns may want `PeelKind::Overloaded`, stopping on encountering a box.
526546
| PatKind::Box(_)
527547
| PatKind::Deref(_) => AdjustMode::Peel { kind: PeelKind::RefOnly },
528548
// A never pattern behaves somewhat like a literal or unit variant.
529-
PatKind::Never => AdjustMode::Peel { kind: PeelKind::Overloaded },
549+
PatKind::Never => AdjustMode::peel_all(),
530550
PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), .. }) => match opt_path_res.unwrap() {
531551
// These constants can be of a reference type, e.g. `const X: &u8 = &0;`.
532552
// Peeling the reference types too early will cause type checking failures.
@@ -536,7 +556,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
536556
// could successfully compile. The former being `Self` requires a unit struct.
537557
// In either case, and unlike constants, the pattern itself cannot be
538558
// a reference type wherefore peeling doesn't give up any expressiveness.
539-
_ => AdjustMode::Peel { kind: PeelKind::Overloaded },
559+
_ => AdjustMode::peel_until_adt(pat_adt_def),
540560
},
541561

542562
// String and byte-string literals result in types `&str` and `&[u8]` respectively.
@@ -546,7 +566,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
546566
// Call `resolve_vars_if_possible` here for inline const blocks.
547567
PatKind::Expr(lt) => match self.resolve_vars_if_possible(self.check_pat_expr_unadjusted(lt)).kind() {
548568
ty::Ref(..) => AdjustMode::Pass,
549-
_ => AdjustMode::Peel { kind: PeelKind::Overloaded },
569+
// If an inline const pattern has a library-defined pointer-like type and
570+
// `deref_patterns` is enabled, don't implicitly add deref patterns for its ADT.
571+
// Nothing in the library that implements `DerefPure` supports structural equality,
572+
// but we don't check that until `const_to_pat` in THIR construction. By avoiding
573+
// type errors here, we can get a more meaningful error there.
574+
ty::Adt(adt, _) => AdjustMode::peel_until_adt(Some(*adt)),
575+
_ => AdjustMode::peel_all()
550576
},
551577

552578
// Ref patterns are complicated, we handle them in `check_pat_ref`.
@@ -576,7 +602,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
576602
fn peel_off_references(
577603
&self,
578604
pat: &'tcx Pat<'tcx>,
579-
peel_kind: PeelKind,
605+
peel_kind: PeelKind<'tcx>,
580606
expected: Ty<'tcx>,
581607
mut def_br: ByRef,
582608
mut max_ref_mutbl: MutblCap,
@@ -610,13 +636,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
610636
});
611637
inner_ty
612638
} else if deref_patterns
613-
&& peel_kind == PeelKind::Overloaded
639+
&& let PeelKind::Overloaded { until_adt } = peel_kind
614640
// For simplicity, only apply overloaded derefs if `expected` is a known ADT.
615641
// FIXME(deref_patterns): we'll get better diagnostics for users trying to
616642
// implicitly deref generics if we allow them here, but primitives, tuples, and
617643
// inference vars definitely should be stopped. Figure out what makes most sense.
618-
// TODO: stop peeling if the pattern is a constructor for the scrutinee type
619-
&& expected.is_adt()
644+
&& let ty::Adt(scrutinee_adt, _) = *expected.kind()
645+
// Don't peel if the pattern type already matches the scrutinee. E.g., stop here if
646+
// matching on a `Cow<'a, T>` scrutinee with a `Cow::Owned(_)` pattern.
647+
&& until_adt != Some(scrutinee_adt)
620648
{
621649
// At this point, the pattern isn't able to match `expected` without peeling. Check
622650
// that it implements `Deref` before assuming it's a smart pointer, to get a normal
@@ -1236,9 +1264,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12361264
qpath: &hir::QPath<'tcx>,
12371265
fields: &'tcx [hir::PatField<'tcx>],
12381266
has_rest_pat: bool,
1239-
) -> ResolvedPat<impl Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>> {
1267+
) -> ResolvedPat<'tcx, impl Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>> {
12401268
// Resolve the path and check the definition for errors.
12411269
let variant_and_pat_ty = self.check_struct_path(qpath, pat.hir_id);
1270+
let pat_adt_def = variant_and_pat_ty.ok().and_then(|(_, pat_ty)| pat_ty.ty_adt_def());
12421271

12431272
let check = move |expected: Ty<'tcx>, pat_info: PatInfo<'tcx>| -> Ty<'tcx> {
12441273
let (variant, pat_ty) = match variant_and_pat_ty {
@@ -1263,7 +1292,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12631292
}
12641293
};
12651294

1266-
ResolvedPat { path_res: None, check }
1295+
ResolvedPat { path_res: None, pat_adt_def, check }
12671296
}
12681297

12691298
fn check_pat_path(
@@ -1273,7 +1302,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12731302
span: Span,
12741303
qpath: &'tcx hir::QPath<'tcx>,
12751304
ti: &TopInfo<'tcx>,
1276-
) -> ResolvedPat<impl Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>> {
1305+
) -> ResolvedPat<'tcx, impl Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>> {
12771306
let tcx = self.tcx;
12781307

12791308
let (res, opt_ty, segments) =
@@ -1322,6 +1351,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13221351
// Type-check the path.
13231352
let pat_ty_and_res =
13241353
res_ok.map(|_| self.instantiate_value_path(segments, opt_ty, res, span, span, path_id));
1354+
let pat_adt_def = pat_ty_and_res.ok().and_then(|(pat_ty, _)| pat_ty.ty_adt_def());
13251355

13261356
let check = move |expected, _pat_info| -> Ty<'tcx> {
13271357
let (pat_ty, pat_res) = match pat_ty_and_res {
@@ -1336,7 +1366,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13361366
}
13371367
pat_ty
13381368
};
1339-
ResolvedPat { path_res: Some(res), check }
1369+
ResolvedPat { path_res: Some(res), pat_adt_def, check }
13401370
}
13411371

13421372
fn maybe_suggest_range_literal(
@@ -1452,14 +1482,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14521482
qpath: &'tcx hir::QPath<'tcx>,
14531483
subpats: &'tcx [Pat<'tcx>],
14541484
ddpos: hir::DotDotPos,
1455-
) -> ResolvedPat<impl Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>> {
1485+
) -> ResolvedPat<'tcx, impl Fn(Ty<'tcx>, PatInfo<'tcx>) -> Ty<'tcx>> {
14561486
let tcx = self.tcx;
14571487
let report_unexpected_res = |res: Res| {
14581488
let expected = "tuple struct or tuple variant";
14591489
report_unexpected_variant_res(tcx, res, None, qpath, pat.span, E0164, expected)
14601490
};
14611491

1462-
let pat_ty_and_res_and_variant = try {
1492+
let pat_ty_and_res_and_variant: Result<(Ty<'_>, Res, &VariantDef), ErrorGuaranteed> = try {
14631493
// Resolve the path and check the definition for errors.
14641494
let (res, opt_ty, segments) =
14651495
self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span);
@@ -1493,6 +1523,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14931523

14941524
(pat_ty, res, variant)
14951525
};
1526+
let pat_adt_def = pat_ty_and_res_and_variant.ok().and_then(|(ty, ..)| ty.ty_adt_def());
14961527

14971528
let check = move |expected: Ty<'tcx>, pat_info: PatInfo<'tcx>| -> Ty<'tcx> {
14981529
let on_error = |e| {
@@ -1552,7 +1583,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15521583
pat_ty
15531584
};
15541585

1555-
ResolvedPat { path_res: None, check }
1586+
ResolvedPat { path_res: None, pat_adt_def, check }
15561587
}
15571588

15581589
fn emit_err_pat_wrong_number_of_fields(
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//! Test that we get an error about structural equality rather than a type error when attempting to
2+
//! use const patterns of library pointer types. I.e. test that we don't implicitly peel it.
3+
#![feature(deref_patterns, inline_const_pat)]
4+
#![allow(incomplete_features)]
5+
6+
const EMPTY: Vec<()> = Vec::new();
7+
8+
fn main() {
9+
match vec![()] {
10+
EMPTY => {}
11+
//~^ ERROR: constant of non-structural type `Vec<()>` in a pattern
12+
const { Vec::new() } => {}
13+
//~^ ERROR: constant of non-structural type `Vec<()>` in a pattern
14+
_ => {}
15+
}
16+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
error: constant of non-structural type `Vec<()>` in a pattern
2+
--> $DIR/implicit-const-deref.rs:10:9
3+
|
4+
LL | const EMPTY: Vec<()> = Vec::new();
5+
| -------------------- constant defined here
6+
...
7+
LL | EMPTY => {}
8+
| ^^^^^ constant of non-structural type
9+
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
10+
|
11+
= note: `Vec<()>` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
12+
|
13+
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
14+
15+
error: constant of non-structural type `Vec<()>` in a pattern
16+
--> $DIR/implicit-const-deref.rs:12:9
17+
|
18+
LL | const { Vec::new() } => {}
19+
| ^^^^^^^^^^^^^^^^^^^^ constant of non-structural type
20+
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
21+
|
22+
= note: `Vec<()>` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
23+
|
24+
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
25+
26+
error: aborting due to 2 previous errors
27+
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//@ run-pass
2+
//! Test that implicit deref patterns interact as expected with `Cow` constructor patterns.
3+
#![feature(deref_patterns)]
4+
#![allow(incomplete_features)]
5+
6+
use std::borrow::Cow;
7+
8+
fn main() {
9+
let cow: Cow<'static, [u8]> = Cow::from(&[1, 2, 3]);
10+
11+
match cow {
12+
[..] => {}
13+
_ => unreachable!(),
14+
}
15+
16+
match cow {
17+
Cow::Borrowed(_) => {}
18+
Cow::Owned(_) => unreachable!(),
19+
}
20+
21+
match Box::new(&cow) {
22+
Cow::Borrowed { 0: _ } => {}
23+
Cow::Owned { 0: _ } => unreachable!(),
24+
_ => unreachable!(),
25+
}
26+
27+
let cow_of_cow: Cow<'_, Cow<'static, [u8]>> = Cow::Owned(cow);
28+
29+
match cow_of_cow {
30+
[..] => {}
31+
_ => unreachable!(),
32+
}
33+
34+
match cow_of_cow {
35+
Cow::Borrowed(_) => unreachable!(),
36+
Cow::Owned(_) => {}
37+
}
38+
39+
match Box::new(&cow_of_cow) {
40+
Cow::Borrowed { 0: _ } => unreachable!(),
41+
Cow::Owned { 0: _ } => {}
42+
_ => unreachable!(),
43+
}
44+
}

0 commit comments

Comments
 (0)