From 496cc4c0d4a06047c4f78965c8bc6e2c812c7812 Mon Sep 17 00:00:00 2001 From: Patrick Walton <pcwalton@mimiga.net> Date: Tue, 23 Sep 2014 16:07:21 -0700 Subject: [PATCH] librustc: Fix up mutability in method autoderefs if incorrect, and prefer `Deref` over `DerefMut` in all other circumstances. Closes #12825. --- src/libgreen/basic.rs | 2 +- src/librustc/middle/mem_categorization.rs | 11 +++ src/librustc/middle/typeck/check/method.rs | 90 +++++++++++++++++-- src/librustc/middle/typeck/check/mod.rs | 1 + src/libsyntax/codemap.rs | 5 ++ src/test/run-pass/dst-deref-mut.rs | 1 - src/test/run-pass/fixup-deref-mut.rs | 52 +++++++++++ .../run-pass/overloaded-autoderef-count.rs | 2 +- 8 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 src/test/run-pass/fixup-deref-mut.rs diff --git a/src/libgreen/basic.rs b/src/libgreen/basic.rs index 27c71cb3e7c..129d37638a3 100644 --- a/src/libgreen/basic.rs +++ b/src/libgreen/basic.rs @@ -116,7 +116,7 @@ impl EventLoop for BasicLoop { } unsafe { - let mut messages = self.messages.lock(); + let messages = self.messages.lock(); // We block here if we have no messages to process and we may // receive a message at a later date if self.remotes.len() > 0 && messages.len() == 0 && diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 3b831dd6847..73f36e3bd29 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -405,6 +405,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { Some(adjustment) => { match *adjustment { ty::AdjustAddEnv(..) => { + debug!("cat_expr(AdjustAddEnv): {}", + expr.repr(self.tcx())); // Convert a bare fn to a closure by adding NULL env. // Result is an rvalue. let expr_ty = if_ok!(self.expr_ty_adjusted(expr)); @@ -414,6 +416,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { ty::AdjustDerefRef( ty::AutoDerefRef { autoref: Some(_), ..}) => { + debug!("cat_expr(AdjustDerefRef): {}", + expr.repr(self.tcx())); // Equivalent to &*expr or something similar. // Result is an rvalue. let expr_ty = if_ok!(self.expr_ty_adjusted(expr)); @@ -436,6 +440,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { autoderefs: uint) -> McResult<cmt> { let mut cmt = if_ok!(self.cat_expr_unadjusted(expr)); + debug!("cat_expr_autoderefd: autoderefs={}, cmt={}", + autoderefs, + cmt.repr(self.tcx())); for deref in range(1u, autoderefs + 1) { cmt = self.cat_deref(expr, cmt, deref, false); } @@ -454,6 +461,10 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { ast::ExprField(ref base, f_name, _) => { let base_cmt = if_ok!(self.cat_expr(&**base)); + debug!("cat_expr(cat_field): id={} expr={} base={}", + expr.id, + expr.repr(self.tcx()), + base_cmt.repr(self.tcx())); Ok(self.cat_field(expr, base_cmt, f_name.node, expr_ty)) } diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs index 141b03d9b9f..3d522a7c548 100644 --- a/src/librustc/middle/typeck/check/method.rs +++ b/src/librustc/middle/typeck/check/method.rs @@ -86,10 +86,11 @@ use middle::traits; use middle::ty::*; use middle::ty; use middle::typeck::astconv::AstConv; -use middle::typeck::check::{FnCtxt, PreferMutLvalue, impl_self_ty}; +use middle::typeck::check::{FnCtxt, NoPreference, PreferMutLvalue}; +use middle::typeck::check::{impl_self_ty}; use middle::typeck::check; use middle::typeck::infer; -use middle::typeck::MethodCallee; +use middle::typeck::{MethodCall, MethodCallee}; use middle::typeck::{MethodOrigin, MethodParam, MethodTypeParam}; use middle::typeck::{MethodStatic, MethodStaticUnboxedClosure, MethodObject, MethodTraitObject}; use middle::typeck::check::regionmanip::replace_late_bound_regions_in_fn_sig; @@ -353,11 +354,15 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> { let (_, _, result) = check::autoderef( - self.fcx, span, self_ty, self_expr_id, PreferMutLvalue, + self.fcx, span, self_ty, self_expr_id, NoPreference, |self_ty, autoderefs| self.search_step(self_ty, autoderefs)); match result { - Some(Some(result)) => Some(result), + Some(Some(result)) => { + self.fixup_derefs_on_method_receiver_if_necessary(&result, + self_ty); + Some(result) + } _ => None } } @@ -430,7 +435,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> { */ let span = self.self_expr.map_or(self.span, |e| e.span); - check::autoderef(self.fcx, span, self_ty, None, PreferMutLvalue, |self_ty, _| { + check::autoderef(self.fcx, span, self_ty, None, NoPreference, |self_ty, _| { match get(self_ty).sty { ty_trait(box TyTrait { def_id, ref substs, bounds, .. }) => { self.push_inherent_candidates_from_object( @@ -458,7 +463,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> { fn push_bound_candidates(&mut self, self_ty: ty::t, restrict_to: Option<DefId>) { let span = self.self_expr.map_or(self.span, |e| e.span); - check::autoderef(self.fcx, span, self_ty, None, PreferMutLvalue, |self_ty, _| { + check::autoderef(self.fcx, span, self_ty, None, NoPreference, |self_ty, _| { match get(self_ty).sty { ty_param(p) => { self.push_inherent_candidates_from_param(self_ty, restrict_to, p); @@ -1135,7 +1140,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> { }; // This is hokey. We should have mutability inference as a - // variable. But for now, try &const, then &, then &mut: + // variable. But for now, try &, then &mut: let region = self.infcx().next_region_var(infer::Autoref(self.span)); for mutbl in mutbls.iter() { @@ -1381,6 +1386,77 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> { } } + fn fixup_derefs_on_method_receiver_if_necessary( + &self, + method_callee: &MethodCallee, + self_ty: ty::t) { + let sig = match ty::get(method_callee.ty).sty { + ty::ty_bare_fn(ref f) => f.sig.clone(), + ty::ty_closure(ref f) => f.sig.clone(), + _ => return, + }; + + match ty::get(*sig.inputs.get(0)).sty { + ty::ty_rptr(_, ty::mt { + ty: _, + mutbl: ast::MutMutable, + }) => {} + _ => return, + } + + // Fix up autoderefs and derefs. + let mut self_expr = match self.self_expr { + Some(expr) => expr, + None => return, + }; + loop { + // Count autoderefs. + let autoderef_count = match self.fcx + .inh + .adjustments + .borrow() + .find(&self_expr.id) { + Some(&ty::AdjustDerefRef(ty::AutoDerefRef { + autoderefs: autoderef_count, + autoref: _ + })) if autoderef_count > 0 => autoderef_count, + Some(_) | None => return, + }; + + check::autoderef(self.fcx, + self_expr.span, + self.fcx.expr_ty(self_expr), + Some(self_expr.id), + PreferMutLvalue, + |_, autoderefs| { + if autoderefs == autoderef_count + 1 { + Some(()) + } else { + None + } + }); + + match self_expr.node { + ast::ExprParen(ref expr) | + ast::ExprIndex(ref expr, _) | + ast::ExprField(ref expr, _, _) | + ast::ExprTupField(ref expr, _, _) | + ast::ExprSlice(ref expr, _, _, _) => self_expr = &**expr, + ast::ExprUnary(ast::UnDeref, ref expr) => { + drop(check::try_overloaded_deref( + self.fcx, + self_expr.span, + Some(MethodCall::expr(self_expr.id)), + Some(self_expr), + self_ty, + PreferMutLvalue)); + self_expr = &**expr + } + _ => break, + } + } + } + fn enforce_object_limitations(&self, candidate: &Candidate) { /*! * There are some limitations to calling functions through an diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 0bf22d97345..97bbc8a9bc8 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -2078,6 +2078,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } +#[deriving(Show)] pub enum LvaluePreference { PreferMutLvalue, NoPreference diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index e9b2556c53e..cd215c1d68c 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -290,6 +290,9 @@ impl FileMap { } /// get a line from the list of pre-computed line-beginnings + /// + /// NOTE(stage0, pcwalton): Remove `#[allow(unused_mut)]` after snapshot. + #[allow(unused_mut)] pub fn get_line(&self, line: int) -> String { let mut lines = self.lines.borrow_mut(); let begin: BytePos = *lines.get(line as uint) - self.start_pos; @@ -512,6 +515,8 @@ impl CodeMap { return a; } + // NOTE(stage0, pcwalton): Remove `#[allow(unused_mut)]` after snapshot. + #[allow(unused_mut)] fn lookup_line(&self, pos: BytePos) -> FileMapAndLine { let idx = self.lookup_filemap_idx(pos); diff --git a/src/test/run-pass/dst-deref-mut.rs b/src/test/run-pass/dst-deref-mut.rs index d59c24e07b8..465529ac909 100644 --- a/src/test/run-pass/dst-deref-mut.rs +++ b/src/test/run-pass/dst-deref-mut.rs @@ -27,7 +27,6 @@ impl DerefMut<[uint]> for Arr { } pub fn foo(arr: &mut Arr) { - assert!(arr.len() == 3); let x: &mut [uint] = &mut **arr; assert!(x[0] == 1); assert!(x[1] == 2); diff --git a/src/test/run-pass/fixup-deref-mut.rs b/src/test/run-pass/fixup-deref-mut.rs new file mode 100644 index 00000000000..d2812ce1d2c --- /dev/null +++ b/src/test/run-pass/fixup-deref-mut.rs @@ -0,0 +1,52 @@ +// 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. + +// Generic unique/owned smaht pointer. +struct Own<T> { + value: *mut T +} + +impl<T> Deref<T> for Own<T> { + fn deref<'a>(&'a self) -> &'a T { + unsafe { &*self.value } + } +} + +impl<T> DerefMut<T> for Own<T> { + fn deref_mut<'a>(&'a mut self) -> &'a mut T { + unsafe { &mut *self.value } + } +} + +struct Point { + x: int, + y: int +} + +impl Point { + fn get(&mut self) -> (int, int) { + (self.x, self.y) + } +} + +fn test0(mut x: Own<Point>) { + let _ = x.get(); +} + +fn test1(mut x: Own<Own<Own<Point>>>) { + let _ = x.get(); +} + +fn test2(mut x: Own<Own<Own<Point>>>) { + let _ = (**x).get(); +} + +fn main() {} + diff --git a/src/test/run-pass/overloaded-autoderef-count.rs b/src/test/run-pass/overloaded-autoderef-count.rs index 24146c4a6ea..4cf5e074139 100644 --- a/src/test/run-pass/overloaded-autoderef-count.rs +++ b/src/test/run-pass/overloaded-autoderef-count.rs @@ -74,7 +74,7 @@ pub fn main() { assert_eq!(p.counts(), (2, 2)); p.get(); - assert_eq!(p.counts(), (2, 3)); + assert_eq!(p.counts(), (3, 2)); // Check the final state. assert_eq!(*p, Point {x: 3, y: 0});