Skip to content

Don't allow upcasting to a supertype in the type of the match #23119

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

Merged
merged 1 commit into from
Mar 24, 2015
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
18 changes: 18 additions & 0 deletions src/librustc/middle/pat_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,24 @@ pub fn pat_contains_bindings(dm: &DefMap, pat: &ast::Pat) -> bool {
contains_bindings
}

/// Checks if the pattern contains any `ref` or `ref mut` bindings.
pub fn pat_contains_ref_binding(dm: &DefMap, pat: &ast::Pat) -> bool {
let mut result = false;
pat_bindings(dm, pat, |mode, _, _, _| {
match mode {
ast::BindingMode::BindByRef(_) => { result = true; }
ast::BindingMode::BindByValue(_) => { }
}
});
result
}

/// Checks if the patterns for this arm contain any `ref` or `ref mut`
/// bindings.
pub fn arm_contains_ref_binding(dm: &DefMap, arm: &ast::Arm) -> bool {
arm.pats.iter().any(|pat| pat_contains_ref_binding(dm, pat))
}

/// Checks if the pattern contains any patterns that bind something to
/// an ident or wildcard, e.g. `foo`, or `Foo(_)`, `foo @ Bar(..)`,
pub fn pat_contains_bindings_or_wild(dm: &DefMap, pat: &ast::Pat) -> bool {
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ use middle::mem_categorization as mc;
use middle::region;
use middle::resolve_lifetime;
use middle::infer;
use middle::pat_util;
use middle::stability;
use middle::subst::{self, ParamSpace, Subst, Substs, VecPerParamSpace};
use middle::traits;
Expand Down Expand Up @@ -2684,6 +2685,14 @@ impl<'tcx> ctxt<'tcx> {
{
self.ty_param_defs.borrow()[node_id].clone()
}

pub fn pat_contains_ref_binding(&self, pat: &ast::Pat) -> bool {
pat_util::pat_contains_ref_binding(&self.def_map, pat)
}

pub fn arm_contains_ref_binding(&self, arm: &ast::Arm) -> bool {
pat_util::arm_contains_ref_binding(&self.def_map, arm)
}
}

// Interns a type/name combination, stores the resulting box in cx.interner,
Expand Down
26 changes: 20 additions & 6 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,11 @@ pub fn check_pat<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
// (nmatsakis) an hour or two debugging to remember, so I thought
// I'd write them down this time.
//
// 1. Most importantly, there is no loss of expressiveness
// here. What we are saying is that the type of `x`
// becomes *exactly* what is expected. This might seem
// like it will cause errors in a case like this:
// 1. There is no loss of expressiveness here, though it does
// cause some inconvenience. What we are saying is that the type
// of `x` becomes *exactly* what is expected. This can cause unnecessary
// errors in some cases, such as this one:
// it will cause errors in a case like this:
//
// ```
// fn foo<'x>(x: &'x int) {
Expand Down Expand Up @@ -361,8 +362,21 @@ pub fn check_match<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
match_src: ast::MatchSource) {
let tcx = fcx.ccx.tcx;

let discrim_ty = fcx.infcx().next_ty_var();
check_expr_has_type(fcx, discrim, discrim_ty);
// Not entirely obvious: if matches may create ref bindings, we
// want to use the *precise* type of the discriminant, *not* some
// supertype, as the "discriminant type" (issue #23116).
let contains_ref_bindings = arms.iter().any(|a| tcx.arm_contains_ref_binding(a));
let discrim_ty;
if contains_ref_bindings {
check_expr(fcx, discrim);
discrim_ty = fcx.expr_ty(discrim);
} else {
// ...but otherwise we want to use any supertype of the
// discriminant. This is sort of a workaround, see note (*) in
// `check_pat` for some details.
discrim_ty = fcx.infcx().next_ty_var();
check_expr_has_type(fcx, discrim, discrim_ty);
};

// Typecheck the patterns first, so that we get types for all the
// bindings.
Expand Down
24 changes: 20 additions & 4 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4242,11 +4242,27 @@ impl<'tcx> Repr<'tcx> for Expectation<'tcx> {
}

pub fn check_decl_initializer<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>,
nid: ast::NodeId,
local: &'tcx ast::Local,
init: &'tcx ast::Expr)
{
let local_ty = fcx.local_ty(init.span, nid);
check_expr_coercable_to_type(fcx, init, local_ty)
let ref_bindings = fcx.tcx().pat_contains_ref_binding(&local.pat);

let local_ty = fcx.local_ty(init.span, local.id);
if !ref_bindings {
check_expr_coercable_to_type(fcx, init, local_ty)
} else {
// Somewhat subtle: if we have a `ref` binding in the pattern,
// we want to avoid introducing coercions for the RHS. This is
// both because it helps preserve sanity and, in the case of
// ref mut, for soundness (issue #23116). In particular, in
// the latter case, we need to be clear that the type of the
// referent for the reference that results is *equal to* the
// type of the lvalue it is referencing, and not some
// supertype thereof.
check_expr(fcx, init);
let init_ty = fcx.expr_ty(init);
demand::eqtype(fcx, init.span, init_ty, local_ty);
};
}

pub fn check_decl_local<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, local: &'tcx ast::Local) {
Expand All @@ -4256,7 +4272,7 @@ pub fn check_decl_local<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, local: &'tcx ast::Local)
fcx.write_ty(local.id, t);

if let Some(ref init) = local.init {
check_decl_initializer(fcx, local.id, &**init);
check_decl_initializer(fcx, local, &**init);
let init_ty = fcx.expr_ty(&**init);
if ty::type_is_error(init_ty) {
fcx.write_ty(local.id, init_ty);
Expand Down
24 changes: 24 additions & 0 deletions src/test/compile-fail/match-ref-mut-invariance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Check that when making a ref mut binding with type `&mut T`, the
// type `T` must match precisely the type `U` of the value being
// matched, and in particular cannot be some supertype of `U`. Issue
// #23116. This test focuses on a `match`.

#![allow(dead_code)]
struct S<'b>(&'b i32);
impl<'b> S<'b> {
fn bar<'a>(&'a mut self) -> &'a mut &'a i32 {
match self.0 { ref mut x => x } //~ ERROR mismatched types
}
}

fn main() {}
25 changes: 25 additions & 0 deletions src/test/compile-fail/match-ref-mut-let-invariance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Check that when making a ref mut binding with type `&mut T`, the
// type `T` must match precisely the type `U` of the value being
// matched, and in particular cannot be some supertype of `U`. Issue
// #23116. This test focuses on a `let`.

#![allow(dead_code)]
struct S<'b>(&'b i32);
impl<'b> S<'b> {
fn bar<'a>(&'a mut self) -> &'a mut &'a i32 {
let ref mut x = self.0;
x //~ ERROR mismatched types
}
}

fn main() {}