Skip to content

Overloaded augmented assignments #28345

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 2 commits into from
Sep 19, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
address Niko's comments
  • Loading branch information
Jorge Aparicio committed Sep 19, 2015
commit f5569ecd7682a22f9ae3293a89479c7b99b6d941
9 changes: 3 additions & 6 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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() {}