rollup merge of #23119: nikomatsakis/issue-23116-ref-mut

Don't allow upcasting to a supertype in the type of the match discriminant. Fixes #23116.

This is a [breaking-change] in that it closes a type hole that previously existed.

r? @pnkfelix

6 files changed, 116 insertions, 10 deletions

diff --git a/src/librustc/middle/pat_util.rs b/src/librustc/middle/pat_util.rs
index c5abff3b963..4f365beed21 100644
--- a/src/librustc/middle/pat_util.rs
+++ b/src/librustc/middle/pat_util.rs
@@ -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 {
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index 99c35c6e542..b490de335c5 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -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,
diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
index 8e2f4dcefa0..a66c0f351bf 100644
--- a/src/librustc_typeck/check/_match.rs
+++ b/src/librustc_typeck/check/_match.rs
@@ -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.
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index 45d4a1edc6b..09309c7bbeb 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -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);
diff --git a/src/test/compile-fail/match-ref-mut-invariance.rs b/src/test/compile-fail/match-ref-mut-invariance.rs
new file mode 100644
index 00000000000..c2b54a972bd
--- /dev/null
+++ b/src/test/compile-fail/match-ref-mut-invariance.rs
@@ -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() {}
diff --git a/src/test/compile-fail/match-ref-mut-let-invariance.rs b/src/test/compile-fail/match-ref-mut-let-invariance.rs
new file mode 100644
index 00000000000..ea16c61dfd4
--- /dev/null
+++ b/src/test/compile-fail/match-ref-mut-let-invariance.rs
@@ -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() {}