From d6b7ca041a68a9056b2295a3901800c727ccee03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20L=C3=B6bel?= Date: Sat, 6 Jun 2015 20:10:23 +0200 Subject: [PATCH] Made ref pattern bindings correctly pick Deref or DerefMut Added LvaluePreference::from_mutbl Closes #15609 --- src/librustc/middle/pat_util.rs | 26 +++-- src/librustc/middle/ty.rs | 4 +- src/librustc_typeck/check/_match.rs | 12 ++- src/librustc_typeck/check/coercion.rs | 7 +- src/librustc_typeck/check/mod.rs | 22 ++-- .../overloaded_deref_with_ref_pattern.rs | 102 ++++++++++++++++++ ...oaded_deref_with_ref_pattern_issue15609.rs | 44 ++++++++ 7 files changed, 192 insertions(+), 25 deletions(-) create mode 100644 src/test/run-pass/overloaded_deref_with_ref_pattern.rs create mode 100644 src/test/run-pass/overloaded_deref_with_ref_pattern_issue15609.rs diff --git a/src/librustc/middle/pat_util.rs b/src/librustc/middle/pat_util.rs index 27a30f5cf253c..bb4c702f3733b 100644 --- a/src/librustc/middle/pat_util.rs +++ b/src/librustc/middle/pat_util.rs @@ -135,12 +135,19 @@ 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; +/// Checks if the pattern contains any `ref` or `ref mut` bindings, +/// and if yes wether its containing mutable ones or just immutables ones. +pub fn pat_contains_ref_binding(dm: &DefMap, pat: &ast::Pat) -> Option { + let mut result = None; pat_bindings(dm, pat, |mode, _, _, _| { match mode { - ast::BindingMode::BindByRef(_) => { result = true; } + ast::BindingMode::BindByRef(m) => { + // Pick Mutable as maximum + match result { + None | Some(ast::MutImmutable) => result = Some(m), + _ => (), + } + } ast::BindingMode::BindByValue(_) => { } } }); @@ -148,9 +155,14 @@ pub fn pat_contains_ref_binding(dm: &DefMap, pat: &ast::Pat) -> bool { } /// 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)) +/// bindings, and if yes wether its containing mutable ones or just immutables ones. +pub fn arm_contains_ref_binding(dm: &DefMap, arm: &ast::Arm) -> Option { + arm.pats.iter() + .filter_map(|pat| pat_contains_ref_binding(dm, pat)) + .max_by(|m| match *m { + ast::MutMutable => 1, + ast::MutImmutable => 0, + }) } /// Checks if the pattern contains any patterns that bind something to diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 87216b1add0fa..ee2189c769aa0 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -2846,11 +2846,11 @@ impl<'tcx> ctxt<'tcx> { self.ty_param_defs.borrow().get(&node_id).unwrap().clone() } - pub fn pat_contains_ref_binding(&self, pat: &ast::Pat) -> bool { + pub fn pat_contains_ref_binding(&self, pat: &ast::Pat) -> Option { pat_util::pat_contains_ref_binding(&self.def_map, pat) } - pub fn arm_contains_ref_binding(&self, arm: &ast::Arm) -> bool { + pub fn arm_contains_ref_binding(&self, arm: &ast::Arm) -> Option { pat_util::arm_contains_ref_binding(&self.def_map, arm) } } diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 0c756cf50083f..daf48d8ee6f26 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -18,6 +18,7 @@ use middle::subst::Substs; use middle::ty::{self, Ty}; use check::{check_expr, check_expr_has_type, check_expr_with_expectation}; use check::{check_expr_coercable_to_type, demand, FnCtxt, Expectation}; +use check::{check_expr_with_lvalue_pref, LvaluePreference}; use check::{instantiate_path, resolve_ty_and_def_ufcs, structurally_resolved_type}; use require_same_types; use util::nodemap::FnvHashMap; @@ -438,10 +439,15 @@ pub fn check_match<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, // 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 contains_ref_bindings = arms.iter() + .filter_map(|a| tcx.arm_contains_ref_binding(a)) + .max_by(|m| match *m { + ast::MutMutable => 1, + ast::MutImmutable => 0, + }); let discrim_ty; - if contains_ref_bindings { - check_expr(fcx, discrim); + if let Some(m) = contains_ref_bindings { + check_expr_with_lvalue_pref(fcx, discrim, LvaluePreference::from_mutbl(m)); discrim_ty = fcx.expr_ty(discrim); } else { // ...but otherwise we want to use any supertype of the diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 92317aae08967..002bfd94a9860 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -60,7 +60,7 @@ //! sort of a minor point so I've opted to leave it for later---after all //! we may want to adjust precisely when coercions occur. -use check::{autoderef, FnCtxt, NoPreference, PreferMutLvalue, UnresolvedTypeAction}; +use check::{autoderef, FnCtxt, LvaluePreference, UnresolvedTypeAction}; use middle::infer::{self, Coercion}; use middle::traits::{self, ObligationCause}; @@ -188,10 +188,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { let r_borrow = self.tcx().mk_region(r_borrow); let autoref = Some(ty::AutoPtr(r_borrow, mutbl_b)); - let lvalue_pref = match mutbl_b { - ast::MutMutable => PreferMutLvalue, - ast::MutImmutable => NoPreference - }; + let lvalue_pref = LvaluePreference::from_mutbl(mutbl_b); let mut first_error = None; let (_, autoderefs, success) = autoderef(self.fcx, expr_a.span, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index e37856bbb2ea2..33737f40e46f0 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1908,6 +1908,15 @@ pub enum LvaluePreference { NoPreference } +impl LvaluePreference { + pub fn from_mutbl(m: ast::Mutability) -> Self { + match m { + ast::MutMutable => PreferMutLvalue, + ast::MutImmutable => NoPreference, + } + } +} + /// Whether `autoderef` requires types to resolve. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum UnresolvedTypeAction { @@ -3224,10 +3233,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, _ => NoExpectation } }); - let lvalue_pref = match mutbl { - ast::MutMutable => PreferMutLvalue, - ast::MutImmutable => NoPreference - }; + let lvalue_pref = LvaluePreference::from_mutbl(mutbl); check_expr_with_expectation_and_lvalue_pref(fcx, &**oprnd, hint, @@ -3925,9 +3931,7 @@ pub fn check_decl_initializer<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, 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 { + if let Some(m) = ref_bindings { // 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 @@ -3936,9 +3940,11 @@ pub fn check_decl_initializer<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, // 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); + check_expr_with_lvalue_pref(fcx, init, LvaluePreference::from_mutbl(m)); let init_ty = fcx.expr_ty(init); demand::eqtype(fcx, init.span, init_ty, local_ty); + } else { + check_expr_coercable_to_type(fcx, init, local_ty) }; } diff --git a/src/test/run-pass/overloaded_deref_with_ref_pattern.rs b/src/test/run-pass/overloaded_deref_with_ref_pattern.rs new file mode 100644 index 0000000000000..f72d496425138 --- /dev/null +++ b/src/test/run-pass/overloaded_deref_with_ref_pattern.rs @@ -0,0 +1,102 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we choose Deref or DerefMut appropriately based on mutability of ref bindings (#15609). + +use std::ops::{Deref, DerefMut}; + +struct DerefOk(T); +struct DerefMutOk(T); + +impl Deref for DerefOk { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for DerefOk { + fn deref_mut(&mut self) -> &mut Self::Target { + panic!() + } +} + +impl Deref for DerefMutOk { + type Target = T; + fn deref(&self) -> &Self::Target { + panic!() + } +} + +impl DerefMut for DerefMutOk { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +fn main() { + // Check that mutable ref binding in match picks DerefMut + let mut b = DerefMutOk(0); + match *b { + ref mut n => n, + }; + + // Check that mutable ref binding in let picks DerefMut + let mut y = DerefMutOk(1); + let ref mut z = *y; + + // Check that immutable ref binding in match picks Deref + let mut b = DerefOk(2); + match *b { + ref n => n, + }; + + // Check that immutable ref binding in let picks Deref + let mut y = DerefOk(3); + let ref z = *y; + + // Check that mixed mutable/immutable ref binding in match picks DerefMut + let mut b = DerefMutOk((0, 9)); + match *b { + (ref mut n, ref m) => (n, m), + }; + + let mut b = DerefMutOk((0, 9)); + match *b { + (ref n, ref mut m) => (n, m), + }; + + // Check that mixed mutable/immutable ref binding in let picks DerefMut + let mut y = DerefMutOk((1, 8)); + let (ref mut z, ref a) = *y; + + let mut y = DerefMutOk((1, 8)); + let (ref z, ref mut a) = *y; + + // Check that multiple immutable ref bindings in match picks Deref + let mut b = DerefOk((2, 7)); + match *b { + (ref n, ref m) => (n, m), + }; + + // Check that multiple immutable ref bindings in let picks Deref + let mut y = DerefOk((3, 6)); + let (ref z, ref a) = *y; + + // Check that multiple mutable ref bindings in match picks DerefMut + let mut b = DerefMutOk((4, 5)); + match *b { + (ref mut n, ref mut m) => (n, m), + }; + + // Check that multiple mutable ref bindings in let picks DerefMut + let mut y = DerefMutOk((5, 4)); + let (ref mut z, ref mut a) = *y; +} diff --git a/src/test/run-pass/overloaded_deref_with_ref_pattern_issue15609.rs b/src/test/run-pass/overloaded_deref_with_ref_pattern_issue15609.rs new file mode 100644 index 0000000000000..e5eb6ab8f610d --- /dev/null +++ b/src/test/run-pass/overloaded_deref_with_ref_pattern_issue15609.rs @@ -0,0 +1,44 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we choose Deref or DerefMut appropriately based on mutability of ref bindings (#15609). + +fn main() { + use std::cell::RefCell; + + struct S { + node: E, + } + + enum E { + Foo(u32), + Bar, + } + + // Check match + let x = RefCell::new(S { node: E::Foo(0) }); + + let mut b = x.borrow_mut(); + match b.node { + E::Foo(ref mut n) => *n += 1, + _ => (), + } + + // Check let + let x = RefCell::new(0); + let mut y = x.borrow_mut(); + let ref mut z = *y; + + fn foo(a: &mut RefCell>) { + if let Some(ref mut s) = *a.borrow_mut() { + s.push('a') + } + } +}