Skip to content

Commit f5569ec

Browse files
author
Jorge Aparicio
committed
address Niko's comments
1 parent 3ae3a5f commit f5569ec

File tree

4 files changed

+82
-52
lines changed

4 files changed

+82
-52
lines changed

‎src/librustc/middle/expr_use_visitor.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -526,13 +526,10 @@ impl<'d,'t,'a,'tcx> ExprUseVisitor<'d,'t,'a,'tcx> {
526526
}
527527

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

535-
if !self.walk_overloaded_operator(expr, &**lhs, vec![&**rhs], pass_args) {
532+
if !self.walk_overloaded_operator(expr, lhs, vec![rhs], PassArgs::ByValue) {
536533
self.mutate_expr(expr, &**lhs, WriteAndRead);
537534
self.consume_expr(&**rhs);
538535
}

‎src/librustc_typeck/check/op.rs

+16-9
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub fn check_binop_assign<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>,
3636

3737
let lhs_ty = fcx.resolve_type_vars_if_possible(fcx.expr_ty(lhs_expr));
3838
let (rhs_ty, return_ty) =
39-
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, true);
39+
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, IsAssign::Yes);
4040
let rhs_ty = fcx.resolve_type_vars_if_possible(rhs_ty);
4141

4242
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>,
8383
// overloaded. This is the way to be most flexible w/r/t
8484
// types that get inferred.
8585
let (rhs_ty, return_ty) =
86-
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, false);
86+
check_overloaded_binop(fcx, expr, lhs_expr, lhs_ty, rhs_expr, op, IsAssign::No);
8787

8888
// Supply type inference hints if relevant. Probably these
8989
// hints should be enforced during select as part of the
@@ -156,15 +156,15 @@ fn check_overloaded_binop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
156156
lhs_ty: Ty<'tcx>,
157157
rhs_expr: &'tcx hir::Expr,
158158
op: hir::BinOp,
159-
assign: bool)
159+
is_assign: IsAssign)
160160
-> (Ty<'tcx>, Ty<'tcx>)
161161
{
162-
debug!("check_overloaded_binop(expr.id={}, lhs_ty={:?}, assign={})",
162+
debug!("check_overloaded_binop(expr.id={}, lhs_ty={:?}, is_assign={:?})",
163163
expr.id,
164164
lhs_ty,
165-
assign);
165+
is_assign);
166166

167-
let (name, trait_def_id) = name_and_trait_def_id(fcx, op, assign);
167+
let (name, trait_def_id) = name_and_trait_def_id(fcx, op, is_assign);
168168

169169
// NB: As we have not yet type-checked the RHS, we don't have the
170170
// 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>,
181181
Err(()) => {
182182
// error types are considered "builtin"
183183
if !lhs_ty.references_error() {
184-
if assign {
184+
if let IsAssign::Yes = is_assign {
185185
span_err!(fcx.tcx().sess, lhs_expr.span, E0368,
186186
"binary assignment operation `{}=` cannot be applied to type `{}`",
187187
hir_util::binop_to_string(op.node),
@@ -230,11 +230,11 @@ pub fn check_user_unop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
230230

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

237-
if assign {
237+
if let IsAssign::Yes = is_assign {
238238
match op.node {
239239
hir::BiAdd => ("add_assign", lang.add_assign_trait()),
240240
hir::BiSub => ("sub_assign", lang.sub_assign_trait()),
@@ -383,6 +383,13 @@ impl BinOpCategory {
383383
}
384384
}
385385

386+
/// Whether the binary operation is an assignment (`a += b`), or not (`a + b`)
387+
#[derive(Clone, Copy, Debug)]
388+
enum IsAssign {
389+
No,
390+
Yes,
391+
}
392+
386393
/// Returns true if this is a built-in arithmetic operation (e.g. u32
387394
/// + u32, i16x4 == i16x4) and false if these types would have to be
388395
/// overloaded to be legal. There are two reasons that we distinguish

‎src/librustc_typeck/check/writeback.rs

+39-37
Original file line numberDiff line numberDiff line change
@@ -93,48 +93,50 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
9393
fn fix_scalar_binary_expr(&mut self, e: &hir::Expr) {
9494
match e.node {
9595
hir::ExprBinary(ref op, ref lhs, ref rhs) |
96-
hir::ExprAssignOp(ref op, ref lhs, ref rhs) => {
97-
let lhs_ty = self.fcx.node_ty(lhs.id);
98-
let lhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&lhs_ty);
99-
100-
let rhs_ty = self.fcx.node_ty(rhs.id);
101-
let rhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&rhs_ty);
102-
103-
if lhs_ty.is_scalar() && rhs_ty.is_scalar() {
104-
self.fcx.inh.tables.borrow_mut().method_map.remove(&MethodCall::expr(e.id));
105-
106-
// weird but true: the by-ref binops put an
107-
// adjustment on the lhs but not the rhs; the
108-
// adjustment for rhs is kind of baked into the
109-
// system.
110-
match e.node {
111-
hir::ExprBinary(..) => {
112-
if !hir_util::is_by_value_binop(op.node) {
113-
self.fcx.inh.tables.borrow_mut().adjustments.remove(&lhs.id);
114-
}
115-
},
116-
hir::ExprAssignOp(..) => {
96+
hir::ExprAssignOp(ref op, ref lhs, ref rhs) => {
97+
let lhs_ty = self.fcx.node_ty(lhs.id);
98+
let lhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&lhs_ty);
99+
100+
let rhs_ty = self.fcx.node_ty(rhs.id);
101+
let rhs_ty = self.fcx.infcx().resolve_type_vars_if_possible(&rhs_ty);
102+
103+
if lhs_ty.is_scalar() && rhs_ty.is_scalar() {
104+
self.fcx.inh.tables.borrow_mut().method_map.remove(&MethodCall::expr(e.id));
105+
106+
// weird but true: the by-ref binops put an
107+
// adjustment on the lhs but not the rhs; the
108+
// adjustment for rhs is kind of baked into the
109+
// system.
110+
match e.node {
111+
hir::ExprBinary(..) => {
112+
if !hir_util::is_by_value_binop(op.node) {
117113
self.fcx.inh.tables.borrow_mut().adjustments.remove(&lhs.id);
118-
},
119-
_ => {},
120-
}
121-
} else {
122-
let tcx = self.tcx();
123-
124-
if let hir::ExprAssignOp(..) = e.node {
125-
if !tcx.sess.features.borrow().augmented_assignments &&
126-
!self.fcx.expr_ty(e).references_error() {
127-
tcx.sess.span_err(
128-
e.span,
129-
"overloaded augmented assignments are not stable");
130-
fileline_help!(
131-
tcx.sess, e.span,
132-
"add #![feature(augmented_assignments)] to the crate features \
133-
to enable");
134114
}
115+
},
116+
hir::ExprAssignOp(..) => {
117+
self.fcx.inh.tables.borrow_mut().adjustments.remove(&lhs.id);
118+
},
119+
_ => {},
120+
}
121+
} else {
122+
let tcx = self.tcx();
123+
124+
if let hir::ExprAssignOp(..) = e.node {
125+
if
126+
!tcx.sess.features.borrow().augmented_assignments &&
127+
!self.fcx.expr_ty(e).references_error()
128+
{
129+
tcx.sess.span_err(
130+
e.span,
131+
"overloaded augmented assignments are not stable");
132+
fileline_help!(
133+
tcx.sess, e.span,
134+
"add #![feature(augmented_assignments)] to the crate features \
135+
to enable");
135136
}
136137
}
137138
}
139+
}
138140
_ => {},
139141
}
140142
}

‎src/test/compile-fail/augmented-assignments-trait.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use std::ops::AddAssign;
12+
//~^ error: use of unstable library feature 'op_assign_traits'
13+
14+
struct Int(i32);
15+
16+
impl AddAssign for Int {
17+
//~^ error: use of unstable library feature 'op_assign_traits'
18+
fn add_assign(&mut self, _: Int) {
19+
//~^ error: use of unstable library feature 'op_assign_traits'
20+
unimplemented!()
21+
}
22+
}
23+
24+
fn main() {}

0 commit comments

Comments
 (0)