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.

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

typeck: clarify def_bm adjustments & add tests for or-patterns #68856

Merged
merged 4 commits into from
Feb 15, 2020
Merged
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
113 changes: 64 additions & 49 deletions src/librustc_typeck/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
}
}

const INITIAL_BM: BindingMode = BindingMode::BindByValue(hir::Mutability::Not);

/// Mode for adjusting the expected type and binding mode.
enum AdjustMode {
/// Peel off all immediate reference types.
Peel,
/// Reset binding mode to the inital mode.
Reset,
/// Pass on the input binding mode and expected type.
Pass,
}

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Type check the given top level pattern against the `expected` type.
///
Expand All @@ -105,8 +117,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: Option<Span>,
origin_expr: bool,
) {
let def_bm = BindingMode::BindByValue(hir::Mutability::Not);
self.check_pat(pat, expected, def_bm, TopInfo { expected, origin_expr, span });
self.check_pat(pat, expected, INITIAL_BM, TopInfo { expected, origin_expr, span });
}

/// Type check the given `pat` against the `expected` type
Expand All @@ -123,12 +134,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) {
debug!("check_pat(pat={:?},expected={:?},def_bm={:?})", pat, expected, def_bm);

let path_resolution = match &pat.kind {
let path_res = match &pat.kind {
PatKind::Path(qpath) => Some(self.resolve_ty_and_res_ufcs(qpath, pat.hir_id, pat.span)),
_ => None,
};
let is_nrp = self.is_non_ref_pat(pat, path_resolution.map(|(res, ..)| res));
let (expected, def_bm) = self.calc_default_binding_mode(pat, expected, def_bm, is_nrp);
let adjust_mode = self.calc_adjust_mode(pat, path_res.map(|(res, ..)| res));
let (expected, def_bm) = self.calc_default_binding_mode(pat, expected, def_bm, adjust_mode);

let ty = match pat.kind {
PatKind::Wild => expected,
Expand All @@ -141,7 +152,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_pat_tuple_struct(pat, qpath, subpats, ddpos, expected, def_bm, ti)
}
PatKind::Path(ref qpath) => {
self.check_pat_path(pat, path_resolution.unwrap(), qpath, expected)
self.check_pat_path(pat, path_res.unwrap(), qpath, expected)
}
PatKind::Struct(ref qpath, fields, etc) => {
self.check_pat_struct(pat, qpath, fields, etc, expected, def_bm, ti)
Expand Down Expand Up @@ -223,64 +234,68 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pat: &'tcx Pat<'tcx>,
expected: Ty<'tcx>,
def_bm: BindingMode,
is_non_ref_pat: bool,
adjust_mode: AdjustMode,
) -> (Ty<'tcx>, BindingMode) {
if is_non_ref_pat {
debug!("pattern is non reference pattern");
self.peel_off_references(pat, expected, def_bm)
} else {
// When you encounter a `&pat` pattern, reset to "by
// value". This is so that `x` and `y` here are by value,
// as they appear to be:
//
// ```
// match &(&22, &44) {
// (&x, &y) => ...
// }
// ```
//
// See issue #46688.
let def_bm = match pat.kind {
PatKind::Ref(..) => ty::BindByValue(hir::Mutability::Not),
_ => def_bm,
};
(expected, def_bm)
match adjust_mode {
AdjustMode::Pass => (expected, def_bm),
AdjustMode::Reset => (expected, INITIAL_BM),
AdjustMode::Peel => self.peel_off_references(pat, expected, def_bm),
}
}

/// Is the pattern a "non reference pattern"?
/// How should the binding mode and expected type be adjusted?
///
/// When the pattern is a path pattern, `opt_path_res` must be `Some(res)`.
fn is_non_ref_pat(&self, pat: &'tcx Pat<'tcx>, opt_path_res: Option<Res>) -> bool {
match pat.kind {
fn calc_adjust_mode(&self, pat: &'tcx Pat<'tcx>, opt_path_res: Option<Res>) -> AdjustMode {
match &pat.kind {
// Type checking these product-like types successfully always require
// that the expected type be of those types and not reference types.
PatKind::Struct(..)
| PatKind::TupleStruct(..)
| PatKind::Tuple(..)
| PatKind::Box(_)
| PatKind::Range(..)
| PatKind::Slice(..) => true,
PatKind::Lit(ref lt) => {
let ty = self.check_expr(lt);
match ty.kind {
ty::Ref(..) => false,
_ => true,
}
}
| PatKind::Slice(..) => AdjustMode::Peel,
// String and byte-string literals result in types `&str` and `&[u8]` respectively.
// All other literals result in non-reference types.
// As a result, we allow `if let 0 = &&0 {}` but not `if let "foo" = &&"foo {}`.
PatKind::Lit(lt) => match self.check_expr(lt).kind {
ty::Ref(..) => AdjustMode::Pass,
_ => AdjustMode::Peel,
},
PatKind::Path(_) => match opt_path_res.unwrap() {
Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => false,
_ => true,
// These constants can be of a reference type, e.g. `const X: &u8 = &0;`.
// Peeling the reference types too early will cause type checking failures.
// Although it would be possible to *also* peel the types of the constants too.
Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => AdjustMode::Pass,
// In the `ValueNS`, we have `SelfCtor(..) | Ctor(_, Const), _)` remaining which
// could successfully compile. The former being `Self` requires a unit struct.
// In either case, and unlike constants, the pattern itself cannot be
// a reference type wherefore peeling doesn't give up any expressivity.
_ => AdjustMode::Peel,
},
// FIXME(or_patterns; Centril | dlrobertson): To keep things compiling
// for or-patterns at the top level, we need to make `p_0 | ... | p_n`
// a "non reference pattern". For example the following currently compiles:
// When encountering a `& mut? pat` pattern, reset to "by value".
// This is so that `x` and `y` here are by value, as they appear to be:
//
// ```
// match &1 {
// e @ &(1...2) | e @ &(3...4) => {}
// _ => {}
// match &(&22, &44) {
// (&x, &y) => ...
// }
// ```
//
// We should consider whether we should do something special in nested or-patterns.
PatKind::Or(_) | PatKind::Wild | PatKind::Binding(..) | PatKind::Ref(..) => false,
// See issue #46688.
PatKind::Ref(..) => AdjustMode::Reset,
// A `_` pattern works with any expected type, so there's no need to do anything.
PatKind::Wild
// Bindings also work with whatever the expected type is,
// and moreover if we peel references off, that will give us the wrong binding type.
// Also, we can have a subpattern `binding @ pat`.
// Each side of the `@` should be treated independently (like with OR-patterns).
| PatKind::Binding(..)
// An OR-pattern just propagates to each individual alternative.
// This is maximally flexible, allowing e.g., `Some(mut x) | &Some(mut x)`.
// In that example, `Some(mut x)` results in `Peel` whereas `&Some(mut x)` in `Reset`.
| PatKind::Or(_) => AdjustMode::Pass,
}
}

Expand Down Expand Up @@ -508,7 +523,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let local_ty = self.local_ty(pat.span, pat.hir_id).decl_ty;
let eq_ty = match bm {
ty::BindByReference(mutbl) => {
// If the binding is like `ref x | ref const x | ref mut x`
// If the binding is like `ref x | ref mut x`,
// then `x` is assigned a value of type `&M T` where M is the
// mutability and T is the expected type.
//
Expand Down
4 changes: 0 additions & 4 deletions src/test/ui/or-patterns/or-pattern-mismatch.rs

This file was deleted.

9 changes: 0 additions & 9 deletions src/test/ui/or-patterns/or-pattern-mismatch.stderr

This file was deleted.

68 changes: 68 additions & 0 deletions src/test/ui/or-patterns/or-patterns-binding-type-mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Here we test type checking of bindings when combined with or-patterns.
// Specifically, we ensure that introducing bindings of different types result in type errors.

#![feature(or_patterns)]

fn main() {
enum Blah {
A(isize, isize, usize),
B(isize, isize),
}

match Blah::A(1, 1, 2) {
Blah::A(_, x, y) | Blah::B(x, y) => {} //~ ERROR mismatched types
}

match Some(Blah::A(1, 1, 2)) {
Some(Blah::A(_, x, y) | Blah::B(x, y)) => {} //~ ERROR mismatched types
}

match (0u8, 1u16) {
(x, y) | (y, x) => {} //~ ERROR mismatched types
//~^ ERROR mismatched types
}

match Some((0u8, Some((1u16, 2u32)))) {
Some((x, Some((y, z)))) | Some((y, Some((x, z) | (z, x)))) => {}
//~^ ERROR mismatched types
//~| ERROR mismatched types
//~| ERROR mismatched types
//~| ERROR mismatched types
_ => {}
}

if let Blah::A(_, x, y) | Blah::B(x, y) = Blah::A(1, 1, 2) {
//~^ ERROR mismatched types
}

if let Some(Blah::A(_, x, y) | Blah::B(x, y)) = Some(Blah::A(1, 1, 2)) {
//~^ ERROR mismatched types
}

if let (x, y) | (y, x) = (0u8, 1u16) {
//~^ ERROR mismatched types
//~| ERROR mismatched types
}

if let Some((x, Some((y, z)))) | Some((y, Some((x, z) | (z, x))))
//~^ ERROR mismatched types
//~| ERROR mismatched types
//~| ERROR mismatched types
//~| ERROR mismatched types
= Some((0u8, Some((1u16, 2u32))))
{}

let Blah::A(_, x, y) | Blah::B(x, y) = Blah::A(1, 1, 2);
//~^ ERROR mismatched types

let (x, y) | (y, x) = (0u8, 1u16);
//~^ ERROR mismatched types
//~| ERROR mismatched types

fn f1((Blah::A(_, x, y) | Blah::B(x, y)): Blah) {}
//~^ ERROR mismatched types

fn f2(((x, y) | (y, x)): (u8, u16)) {}
//~^ ERROR mismatched types
//~| ERROR mismatched types
}
Loading