From f5569ecd7682a22f9ae3293a89479c7b99b6d941 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 17 Sep 2015 00:29:26 -0500 Subject: [PATCH] address Niko's comments --- src/librustc/middle/expr_use_visitor.rs | 9 +-- src/librustc_typeck/check/op.rs | 25 +++--- src/librustc_typeck/check/writeback.rs | 76 ++++++++++--------- .../augmented-assignments-trait.rs | 24 ++++++ 4 files changed, 82 insertions(+), 52 deletions(-) create mode 100644 src/test/compile-fail/augmented-assignments-trait.rs diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 113dc6f577a10..297084940486e 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -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); } diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 35bbf10d06cad..7be61327f81ff 100644 --- 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) { 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 @@ 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 diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 747d5ca6b723a..22edd7e2c537f 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -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"); } } } + } _ => {}, } } diff --git a/src/test/compile-fail/augmented-assignments-trait.rs b/src/test/compile-fail/augmented-assignments-trait.rs new file mode 100644 index 0000000000000..83e8d1f3b3870 --- /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 or the MIT license +// , 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() {}