When matching against a pattern (either via match
or let
) that
contains ref-bindings, do not permit any upcasting from the type of the value being matched. Similarly, do not permit coercion in a `let`. This is a [breaking-change] in that it closes a type hole that previously existed, and in that coercion is not performed. You should be able to work around the latter by converting: ```rust let ref mut x: T = expr; ``` into ```rust let x: T = expr; let ref mut x = x; ``` Restricting coercion not to apply in the case of `let ref` or `let ref mut` is sort of unexciting to me, but seems the best solution: 1. Mixing coercion and `let ref` or `let ref mut` is a bit odd, because you are taking the address of a (coerced) temporary, but only sometimes. It's not syntactically evident, in other words, what's going on. When you're doing a coercion, you're kind of 2. Put another way, I would like to preserve the relationship that `equality <= subtyping <= coercion <= as-coercion`, where this is an indication of the number of `(T1,T2)` pairs that are accepted by the various relations. Trying to mix `let ref mut` and coercion would create another kind of relation that is like coercion, but acts differently in the case where a precise match is needed. 3. In any case, this is strictly more conservative than what we had before and we can undo it in the future if we find a way to make coercion mix with type equality. The change to match I feel ok about but similarly unthrilled. There is some subtle text already concerning whether to use eqtype or subtype for identifier bindings. The best fix I think would be to always have match use strict equality but use subtyping on identifier bindings, but the comment `(*)` explains why that's not working at the moment. As above, I think we can change this as we clean up the code there.
This commit is contained in:
parent
b0aad7dd4f
commit
45fae88256
6 changed files with 116 additions and 10 deletions
|
@ -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 {
|
||||
|
|
|
@ -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;
|
||||
|
@ -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,
|
||||
|
|
|
@ -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) {
|
||||
|
@ -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.
|
||||
|
|
|
@ -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) {
|
||||
|
@ -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);
|
||||
|
|
24
src/test/compile-fail/match-ref-mut-invariance.rs
Normal file
24
src/test/compile-fail/match-ref-mut-invariance.rs
Normal file
|
@ -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
src/test/compile-fail/match-ref-mut-let-invariance.rs
Normal file
25
src/test/compile-fail/match-ref-mut-let-invariance.rs
Normal file
|
@ -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() {}
|
Loading…
Add table
Reference in a new issue