address Niko's comments

diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs
index 113dc6f577a107613e7cd0cdd7b9941ff7d7af7a..297084940486e38db86137103c43c095500873bb 100644 (file)
--- a/src/librustc/middle/expr_use_visitor.rs
+++ b/src/librustc/middle/expr_use_visitor.rs
@@ -526,13 +526,10 @@ pub fn walk_expr(&mut self, expr: &hir::Expr) {
             }
 
             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);
                 }

diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs
index 35bbf10d06cad192205052c44b36c054c11cfb00..7be61327f81ffbc5ed52a4c7b473cf70be4d859b 100644 (file)
--- a/src/librustc_typeck/check/op.rs
+++ b/src/librustc_typeck/check/op.rs
@@ -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) {
@@ -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
@@ -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
@@ -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),
@@ -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()),
@@ -383,6 +383,13 @@ fn from(op: hir::BinOp) -> 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

diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs
index 747d5ca6b723a6f33f2e0d38363229229d5c62b0..22edd7e2c537fff31d73437f58209f40464d8add 100644 (file)
--- a/src/librustc_typeck/check/writeback.rs
+++ b/src/librustc_typeck/check/writeback.rs
@@ -93,48 +93,50 @@ fn tcx(&self) -> &'cx ty::ctxt<'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");
                         }
                     }
                 }
+            }
             _ => {},
         }
     }

diff --git a/src/test/compile-fail/augmented-assignments-trait.rs b/src/test/compile-fail/augmented-assignments-trait.rs
new file mode 100644 (file)
index 0000000..83e8d1f
--- /dev/null
+++ b/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() {}