Skip to content

Commit

Permalink
rustc: adjust the RHS of comparison operators instead of assuming aut…
Browse files Browse the repository at this point in the history 
…orefs.
  • Loading branch information
@eddyb
eddyb committed Jun 1, 2017
1 parent 194fe69 commit 3ce4438
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 320 deletions.
100 changes: 13 additions & 87 deletions src/librustc/middle/expr_use_visitor.rs
View file
Expand Up @@ -263,12 +263,6 @@ macro_rules! return_if_err {
)
}

/// Whether the elements of an overloaded operation are passed by value or by reference
enum PassArgs {
ByValue,
ByRef,
}

impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
pub fn new(delegate: &'a mut (Delegate<'tcx>+'a),
region_maps: &'a RegionMaps,
Expand Down Expand Up @@ -382,9 +376,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
}

hir::ExprUnary(hir::UnDeref, ref base) => { // *base
if !self.walk_overloaded_operator(expr, &base, Vec::new(), PassArgs::ByRef) {
self.select_from_expr(&base);
}
self.select_from_expr(&base);
}

hir::ExprField(ref base, _) => { // base.f
Expand All @@ -396,13 +388,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
}

hir::ExprIndex(ref lhs, ref rhs) => { // lhs[rhs]
if !self.walk_overloaded_operator(expr,
&lhs,
vec![&rhs],
PassArgs::ByValue) {
self.select_from_expr(&lhs);
self.consume_expr(&rhs);
}
self.select_from_expr(&lhs);
self.consume_expr(&rhs);
}

hir::ExprCall(ref callee, ref args) => { // callee(args)
Expand Down Expand Up @@ -485,29 +472,13 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
self.walk_block(&blk);
}

hir::ExprUnary(op, ref lhs) => {
let pass_args = if op.is_by_value() {
PassArgs::ByValue
} else {
PassArgs::ByRef
};

if !self.walk_overloaded_operator(expr, &lhs, Vec::new(), pass_args) {
self.consume_expr(&lhs);
}
hir::ExprUnary(_, ref lhs) => {
self.consume_expr(&lhs);
}

hir::ExprBinary(op, ref lhs, ref rhs) => {
let pass_args = if op.node.is_by_value() {
PassArgs::ByValue
} else {
PassArgs::ByRef
};

if !self.walk_overloaded_operator(expr, &lhs, vec![&rhs], pass_args) {
self.consume_expr(&lhs);
self.consume_expr(&rhs);
}
hir::ExprBinary(_, ref lhs, ref rhs) => {
self.consume_expr(&lhs);
self.consume_expr(&rhs);
}

hir::ExprBlock(ref blk) => {
Expand All @@ -529,14 +500,13 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
self.consume_expr(&base);
}

hir::ExprAssignOp(op, ref lhs, ref rhs) => {
// NB All our assignment operations take the RHS by value
assert!(op.node.is_by_value());

if !self.walk_overloaded_operator(expr, lhs, vec![rhs], PassArgs::ByValue) {
hir::ExprAssignOp(_, ref lhs, ref rhs) => {
if self.mc.infcx.tables.borrow().is_method_call(expr) {
self.consume_expr(lhs);
} else {
self.mutate_expr(expr, &lhs, MutateMode::WriteAndRead);
self.consume_expr(&rhs);
}
self.consume_expr(&rhs);
}

hir::ExprRepeat(ref base, _) => {
Expand Down Expand Up @@ -784,50 +754,6 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
}
}


// When this returns true, it means that the expression *is* a
// method-call (i.e. via the operator-overload). This true result
// also implies that walk_overloaded_operator already took care of
// recursively processing the input arguments, and thus the caller
// should not do so.
fn walk_overloaded_operator(&mut self,
expr: &hir::Expr,
receiver: &hir::Expr,
rhs: Vec<&hir::Expr>,
pass_args: PassArgs)
-> bool
{
if !self.mc.infcx.tables.borrow().is_method_call(expr) {
return false;
}

match pass_args {
PassArgs::ByValue => {
self.consume_expr(receiver);
for &arg in &rhs {
self.consume_expr(arg);
}

return true;
},
PassArgs::ByRef => {},
}

self.walk_expr(receiver);

// Arguments (but not receivers) to overloaded operator
// methods are implicitly autoref'd which sadly does not use
// adjustments, so we must hardcode the borrow here.

let r = self.tcx().node_scope_region(expr.id);
let bk = ty::ImmBorrow;

for &arg in &rhs {
self.borrow_expr(arg, r, bk, OverloadedOperator);
}
return true;
}

fn arm_move_mode(&mut self, discr_cmt: mc::cmt<'tcx>, arm: &hir::Arm) -> TrackMatchMode {
let mut mode = Unknown;
for pat in &arm.pats {
Expand Down
129 changes: 18 additions & 111 deletions src/librustc_mir/hair/cx/expr.rs
View file
Expand Up @@ -21,7 +21,6 @@ use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow};
use rustc::ty::cast::CastKind as TyCastKind;
use rustc::ty::subst::Subst;
use rustc::hir;
use syntax::ptr::P;

impl<'tcx> Mirror<'tcx> for &'tcx hir::Expr {
type Output = Expr<'tcx>;
Expand Down Expand Up @@ -117,13 +116,7 @@ fn apply_adjustment<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
},
};

overloaded_lvalue(cx,
hir_expr,
adjustment.target,
Some(call),
PassArgs::ByValue,
expr.to_ref(),
vec![])
overloaded_lvalue(cx, hir_expr, adjustment.target, Some(call), vec![expr.to_ref()])
}
Adjust::Borrow(AutoBorrow::Ref(r, m)) => {
ExprKind::Borrow {
Expand Down Expand Up @@ -281,17 +274,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,

hir::ExprAssignOp(op, ref lhs, ref rhs) => {
if cx.tables().is_method_call(expr) {
let pass_args = if op.node.is_by_value() {
PassArgs::ByValue
} else {
PassArgs::ByRef
};
overloaded_operator(cx,
expr,
None,
pass_args,
lhs.to_ref(),
vec![rhs])
overloaded_operator(cx, expr, vec![lhs.to_ref(), rhs.to_ref()])
} else {
ExprKind::AssignOp {
op: bin_op(op.node),
Expand All @@ -305,17 +288,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,

hir::ExprBinary(op, ref lhs, ref rhs) => {
if cx.tables().is_method_call(expr) {
let pass_args = if op.node.is_by_value() {
PassArgs::ByValue
} else {
PassArgs::ByRef
};
overloaded_operator(cx,
expr,
None,
pass_args,
lhs.to_ref(),
vec![rhs])
overloaded_operator(cx, expr, vec![lhs.to_ref(), rhs.to_ref()])
} else {
// FIXME overflow
match (op.node, cx.constness) {
Expand Down Expand Up @@ -365,13 +338,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,

hir::ExprIndex(ref lhs, ref index) => {
if cx.tables().is_method_call(expr) {
overloaded_lvalue(cx,
expr,
expr_ty,
None,
PassArgs::ByValue,
lhs.to_ref(),
vec![index])
overloaded_lvalue(cx, expr, expr_ty, None, vec![lhs.to_ref(), index.to_ref()])
} else {
ExprKind::Index {
lhs: lhs.to_ref(),
Expand All @@ -382,26 +349,15 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,

hir::ExprUnary(hir::UnOp::UnDeref, ref arg) => {
if cx.tables().is_method_call(expr) {
overloaded_lvalue(cx,
expr,
expr_ty,
None,
PassArgs::ByValue,
arg.to_ref(),
vec![])
overloaded_lvalue(cx, expr, expr_ty, None, vec![arg.to_ref()])
} else {
ExprKind::Deref { arg: arg.to_ref() }
}
}

hir::ExprUnary(hir::UnOp::UnNot, ref arg) => {
if cx.tables().is_method_call(expr) {
overloaded_operator(cx,
expr,
None,
PassArgs::ByValue,
arg.to_ref(),
vec![])
overloaded_operator(cx, expr, vec![arg.to_ref()])
} else {
ExprKind::Unary {
op: UnOp::Not,
Expand All @@ -412,12 +368,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,

hir::ExprUnary(hir::UnOp::UnNeg, ref arg) => {
if cx.tables().is_method_call(expr) {
overloaded_operator(cx,
expr,
None,
PassArgs::ByValue,
arg.to_ref(),
vec![])
overloaded_operator(cx, expr, vec![arg.to_ref()])
} else {
// FIXME runtime-overflow
if let hir::ExprLit(_) = arg.node {
Expand Down Expand Up @@ -873,77 +824,29 @@ fn bin_op(op: hir::BinOp_) -> BinOp {
}
}

enum PassArgs {
ByValue,
ByRef,
}

fn overloaded_operator<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
expr: &'tcx hir::Expr,
custom_callee: Option<(DefId, &'tcx Substs<'tcx>)>,
pass_args: PassArgs,
receiver: ExprRef<'tcx>,
args: Vec<&'tcx P<hir::Expr>>)
args: Vec<ExprRef<'tcx>>)
-> ExprKind<'tcx> {
// the receiver has all the adjustments that are needed, so we can
// just push a reference to it
let mut argrefs = vec![receiver];

// the arguments, unfortunately, do not, so if this is a ByRef
// operator, we have to gin up the autorefs (but by value is easy)
match pass_args {
PassArgs::ByValue => argrefs.extend(args.iter().map(|arg| arg.to_ref())),

PassArgs::ByRef => {
let region = cx.tcx.node_scope_region(expr.id);
let (temp_lifetime, was_shrunk) =
cx.region_maps.temporary_scope2(expr.id);
argrefs.extend(args.iter()
.map(|arg| {
let arg_ty = cx.tables().expr_ty_adjusted(arg);
let adjusted_ty = cx.tcx.mk_ref(region,
ty::TypeAndMut {
ty: arg_ty,
mutbl: hir::MutImmutable,
});
Expr {
temp_lifetime: temp_lifetime,
temp_lifetime_was_shrunk: was_shrunk,
ty: adjusted_ty,
span: expr.span,
kind: ExprKind::Borrow {
region: region,
borrow_kind: BorrowKind::Shared,
arg: arg.to_ref(),
},
}
.to_ref()
}))
}
}

// now create the call itself
let fun = method_callee(cx, expr, custom_callee);
let fun = method_callee(cx, expr, None);
ExprKind::Call {
ty: fun.ty,
fun: fun.to_ref(),
args: argrefs,
args,
}
}

fn overloaded_lvalue<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
expr: &'tcx hir::Expr,
lvalue_ty: Ty<'tcx>,
custom_callee: Option<(DefId, &'tcx Substs<'tcx>)>,
pass_args: PassArgs,
receiver: ExprRef<'tcx>,
args: Vec<&'tcx P<hir::Expr>>)
args: Vec<ExprRef<'tcx>>)
-> ExprKind<'tcx> {
// For an overloaded *x or x[y] expression of type T, the method
// call returns an &T and we must add the deref so that the types
// line up (this is because `*x` and `x[y]` represent lvalues):

let recv_ty = match receiver {
let recv_ty = match args[0] {
ExprRef::Hair(e) => cx.tables().expr_ty_adjusted(e),
ExprRef::Mirror(ref e) => e.ty
};
Expand All @@ -963,13 +866,17 @@ fn overloaded_lvalue<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
// construct the complete expression `foo()` for the overloaded call,
// which will yield the &T type
let (temp_lifetime, was_shrunk) = cx.region_maps.temporary_scope2(expr.id);
let ref_kind = overloaded_operator(cx, expr, custom_callee, pass_args, receiver, args);
let fun = method_callee(cx, expr, custom_callee);
let ref_expr = Expr {
temp_lifetime: temp_lifetime,
temp_lifetime_was_shrunk: was_shrunk,
ty: ref_ty,
span: expr.span,
kind: ref_kind,
kind: ExprKind::Call {
ty: fun.ty,
fun: fun.to_ref(),
args,
},
};

// construct and return a deref wrapper `*foo()`
Expand Down

0 comments on commit 3ce4438

Please sign in to comment.