Skip to content

Commit

Permalink
address Niko's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Jorge Aparicio committed Sep 19, 2015
1 parent 3ae3a5f commit f5569ec
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 52 deletions.
9 changes: 3 additions & 6 deletions src/librustc/middle/expr_use_visitor.rs
Expand Up @@ -526,13 +526,10 @@ impl<'d,'t,'a,'tcx> ExprUseVisitor<'d,'t,'a,'tcx> {
}

hir::ExprAssignOp(op, ref lhs, ref rhs) => {
let pass_args = if ::rustc_front::util::is_by_value_binop(op.node) {
PassArgs::ByValue
} else {
PassArgs::ByRef
};
// NB All our assignment operations take the RHS by value
assert!(::rustc_front::util::is_by_value_binop(op.node));

if !self.walk_overloaded_operator(expr, &**lhs, vec![&**rhs], pass_args) {
if !self.walk_overloaded_operator(expr, lhs, vec![rhs], PassArgs::ByValue) {
self.mutate_expr(expr, &**lhs, WriteAndRead);
self.consume_expr(&**rhs);
}
Expand Down
25 changes: 16 additions & 9 deletions src/librustc_typeck/check/op.rs
Expand Up @@ -36,7 +36,7 @@ pub fn check_binop_assign<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>,

let lhs_ty = fcx.resolve_type_vars_if_possible(fcx.expr_ty(lhs_expr));
let (rhs_ty, return_ty) =
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, true);
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, IsAssign::Yes);
let rhs_ty = fcx.resolve_type_vars_if_possible(rhs_ty);

if !lhs_ty.is_ty_var() && !rhs_ty.is_ty_var() && is_builtin_binop(lhs_ty, rhs_ty, op) {
Expand Down Expand Up @@ -83,7 +83,7 @@ pub fn check_binop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
// overloaded. This is the way to be most flexible w/r/t
// types that get inferred.
let (rhs_ty, return_ty) =
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, false);
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, IsAssign::No);

// Supply type inference hints if relevant. Probably these
// hints should be enforced during select as part of the
Expand Down Expand Up @@ -156,15 +156,15 @@ fn check_overloaded_binop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
lhs_ty: Ty<'tcx>,
rhs_expr: &'tcx hir::Expr,
op: hir::BinOp,
assign: bool)
is_assign: IsAssign)
-> (Ty<'tcx>, Ty<'tcx>)
{
debug!("check_overloaded_binop(expr.id={}, lhs_ty={:?}, assign={})",
debug!("check_overloaded_binop(expr.id={}, lhs_ty={:?}, is_assign={:?})",
expr.id,
lhs_ty,
assign);
is_assign);

let (name, trait_def_id) = name_and_trait_def_id(fcx, op, assign);
let (name, trait_def_id) = name_and_trait_def_id(fcx, op, is_assign);

// NB: As we have not yet type-checked the RHS, we don't have the
// type at hand. Make a variable to represent it. The whole reason
Expand All @@ -181,7 +181,7 @@ fn check_overloaded_binop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
Err(()) => {
// error types are considered "builtin"
if !lhs_ty.references_error() {
if assign {
if let IsAssign::Yes = is_assign {
span_err!(fcx.tcx().sess, lhs_expr.span, E0368,
"binary assignment operation `{}=` cannot be applied to type `{}`",
hir_util::binop_to_string(op.node),
Expand Down Expand Up @@ -230,11 +230,11 @@ pub fn check_user_unop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,

fn name_and_trait_def_id(fcx: &FnCtxt,
op: hir::BinOp,
assign: bool)
is_assign: IsAssign)
-> (&'static str, Option<DefId>) {
let lang = &fcx.tcx().lang_items;

if assign {
if let IsAssign::Yes = is_assign {
match op.node {
hir::BiAdd => ("add_assign", lang.add_assign_trait()),
hir::BiSub => ("sub_assign", lang.sub_assign_trait()),
Expand Down Expand Up @@ -383,6 +383,13 @@ impl BinOpCategory {
}
}

/// Whether the binary operation is an assignment (`a += b`), or not (`a + b`)
#[derive(Clone, Copy, Debug)]
enum IsAssign {
No,
Yes,
}

/// Returns true if this is a built-in arithmetic operation (e.g. u32
/// + u32, i16x4 == i16x4) and false if these types would have to be
/// overloaded to be legal. There are two reasons that we distinguish
Expand Down
76 changes: 39 additions & 37 deletions src/librustc_typeck/check/writeback.rs
Expand Up @@ -93,48 +93,50 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
fn fix_scalar_binary_expr(&mut self, e: &hir::Expr) {
match e.node {
hir::ExprBinary(ref op, ref lhs, ref rhs) |
hir::ExprAssignOp(ref op, ref lhs, ref rhs) => {
let lhs_ty = self.fcx.node_ty(lhs.id);
let lhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&lhs_ty);

let rhs_ty = self.fcx.node_ty(rhs.id);
let rhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&rhs_ty);

if lhs_ty.is_scalar() && rhs_ty.is_scalar() {
self.fcx.inh.tables.borrow_mut().method_map.remove(&MethodCall::expr(e.id));

// weird but true: the by-ref binops put an
// adjustment on the lhs but not the rhs; the
// adjustment for rhs is kind of baked into the
// system.
match e.node {
hir::ExprBinary(..) => {
if !hir_util::is_by_value_binop(op.node) {
self.fcx.inh.tables.borrow_mut().adjustments.remove(&lhs.id);
}
},
hir::ExprAssignOp(..) => {
hir::ExprAssignOp(ref op, ref lhs, ref rhs) => {
let lhs_ty = self.fcx.node_ty(lhs.id);
let lhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&lhs_ty);

let rhs_ty = self.fcx.node_ty(rhs.id);
let rhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&rhs_ty);

if lhs_ty.is_scalar() && rhs_ty.is_scalar() {
self.fcx.inh.tables.borrow_mut().method_map.remove(&MethodCall::expr(e.id));

// weird but true: the by-ref binops put an
// adjustment on the lhs but not the rhs; the
// adjustment for rhs is kind of baked into the
// system.
match e.node {
hir::ExprBinary(..) => {
if !hir_util::is_by_value_binop(op.node) {
self.fcx.inh.tables.borrow_mut().adjustments.remove(&lhs.id);
},
_ => {},
}
} else {
let tcx = self.tcx();

if let hir::ExprAssignOp(..) = e.node {
if !tcx.sess.features.borrow().augmented_assignments &&
!self.fcx.expr_ty(e).references_error() {
tcx.sess.span_err(
e.span,
"overloaded augmented assignments are not stable");
fileline_help!(
tcx.sess, e.span,
"add #![feature(augmented_assignments)] to the crate features \
to enable");
}
},
hir::ExprAssignOp(..) => {
self.fcx.inh.tables.borrow_mut().adjustments.remove(&lhs.id);
},
_ => {},
}
} else {
let tcx = self.tcx();

if let hir::ExprAssignOp(..) = e.node {
if
!tcx.sess.features.borrow().augmented_assignments &&
!self.fcx.expr_ty(e).references_error()
{
tcx.sess.span_err(
e.span,
"overloaded augmented assignments are not stable");
fileline_help!(
tcx.sess, e.span,
"add #![feature(augmented_assignments)] to the crate features \
to enable");
}
}
}
}
_ => {},
}
}
Expand Down
24 changes: 24 additions & 0 deletions src/test/compile-fail/augmented-assignments-trait.rs
@@ -0,0 +1,24 @@
// 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 <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.

use std::ops::AddAssign;
//~^ error: use of unstable library feature 'op_assign_traits'

struct Int(i32);

impl AddAssign for Int {
//~^ error: use of unstable library feature 'op_assign_traits'
fn add_assign(&mut self, _: Int) {
//~^ error: use of unstable library feature 'op_assign_traits'
unimplemented!()
}
}

fn main() {}

0 comments on commit f5569ec

Please sign in to comment.