Skip to content

librustc: Fix up mutability in method autoderefs if incorrect, and #17501

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
Oct 1, 2014
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
2 changes: 1 addition & 1 deletion src/libgreen/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
11 changes: 11 additions & 0 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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);
}
Expand All @@ -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))
}

Expand Down
90 changes: 83 additions & 7 deletions src/librustc/middle/typeck/check/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop should also consider recurse on field accesses and indexing, I think, and consider autoderefs on those expressions we pass through. This is why expressions like guard.access.pending.remove() require an explicit &mut (which I think they should not).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted the loop above in this way.

self_expr = &**expr
}
_ => break,
}
}
}

fn enforce_object_limitations(&self, candidate: &Candidate) {
/*!
* There are some limitations to calling functions through an
Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2078,6 +2078,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

#[deriving(Show)]
pub enum LvaluePreference {
PreferMutLvalue,
NoPreference
Expand Down
5 changes: 5 additions & 0 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
1 change: 0 additions & 1 deletion src/test/run-pass/dst-deref-mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
52 changes: 52 additions & 0 deletions src/test/run-pass/fixup-deref-mut.rs
Original file line number Diff line number Diff line change
@@ -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() {}

2 changes: 1 addition & 1 deletion src/test/run-pass/overloaded-autoderef-count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down