Skip to content

Commit

Permalink
Report error for assignment in if condition
Browse files Browse the repository at this point in the history
For code like `if x = 3 {}`, output:

```
error[E0308]: mismatched types
  --> $DIR/issue-17283.rs:25:8
   |
25 |     if x = x {
   |        ^^^^^
   |        |
   |        help: did you mean to compare equality? `x == x`
   |        expected bool, found ()
   |
   = note: expected type `bool`
              found type `()`
```
  • Loading branch information
estebank committed Jun 15, 2017
1 parent 554c685 commit 028b5f9
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 27 deletions.
9 changes: 8 additions & 1 deletion src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// Requires that the two types unify, and prints an error message if
// they don't.
pub fn demand_suptype(&self, sp: Span, expected: Ty<'tcx>, actual: Ty<'tcx>) {
self.demand_suptype_diag(sp, expected, actual).map(|mut e| e.emit());
}

pub fn demand_suptype_diag(&self, sp: Span,
expected: Ty<'tcx>,
actual: Ty<'tcx>) -> Option<DiagnosticBuilder<'tcx>> {
let cause = &self.misc(sp);
match self.at(cause, self.param_env).sup(expected, actual) {
Ok(InferOk { obligations, value: () }) => {
self.register_predicates(obligations);
None
},
Err(e) => {
self.report_mismatched_types(&cause, expected, actual, e).emit();
Some(self.report_mismatched_types(&cause, expected, actual, e))
}
}
}
Expand Down
68 changes: 51 additions & 17 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ pub enum Expectation<'tcx> {
/// We know nothing about what type this expression should have.
NoExpectation,

/// This expression is an `if` condition, it must resolve to `bool`.
ExpectIfCondition,

/// This expression should have the type given (or some subtype)
ExpectHasType(Ty<'tcx>),

Expand Down Expand Up @@ -310,9 +313,8 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
// no constraints yet present), just returns `None`.
fn resolve(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Expectation<'tcx> {
match self {
NoExpectation => {
NoExpectation
}
NoExpectation => NoExpectation,
ExpectIfCondition => ExpectIfCondition,
ExpectCastableToType(t) => {
ExpectCastableToType(fcx.resolve_type_vars_if_possible(&t))
}
Expand All @@ -328,6 +330,7 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
fn to_option(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Option<Ty<'tcx>> {
match self.resolve(fcx) {
NoExpectation => None,
ExpectIfCondition => Some(fcx.tcx.types.bool),
ExpectCastableToType(ty) |
ExpectHasType(ty) |
ExpectRvalueLikeUnsized(ty) => Some(ty),
Expand All @@ -341,6 +344,7 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
fn only_has_type(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Option<Ty<'tcx>> {
match self.resolve(fcx) {
ExpectHasType(ty) => Some(ty),
ExpectIfCondition => Some(fcx.tcx.types.bool),
_ => None
}
}
Expand Down Expand Up @@ -2646,7 +2650,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
pub fn check_expr_has_type(&self,
expr: &'gcx hir::Expr,
expected: Ty<'tcx>) -> Ty<'tcx> {
let mut ty = self.check_expr_with_hint(expr, expected);
self.check_expr_expect_type(expr, ExpectHasType(expected))
}

fn check_expr_expect_type(&self,
expr: &'gcx hir::Expr,
expected: Expectation<'tcx>) -> Ty<'tcx> {
let expected_ty = expected.to_option(&self).unwrap_or(self.tcx.types.bool);
let mut ty = self.check_expr_with_expectation(expr, expected);

// While we don't allow *arbitrary* coercions here, we *do* allow
// coercions from ! to `expected`.
Expand All @@ -2662,7 +2673,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
ty = adj_ty;
}

self.demand_suptype(expr.span, expected, ty);
if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) {
// Add help to type error if this is an `if` condition with an assignment
match (expected, &expr.node) {
(ExpectIfCondition, &hir::ExprAssign(ref lhs, ref rhs)) => {
let msg = "did you mean to compare equality?";
if let (Ok(left), Ok(right)) = (
self.tcx.sess.codemap().span_to_snippet(lhs.span),
self.tcx.sess.codemap().span_to_snippet(rhs.span))
{
err.span_suggestion(expr.span, msg, format!("{} == {}", left, right));
} else {
err.help(msg);
}
}
_ => (),
}
err.emit();
}
ty
}

Expand Down Expand Up @@ -2837,7 +2865,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
opt_else_expr: Option<&'gcx hir::Expr>,
sp: Span,
expected: Expectation<'tcx>) -> Ty<'tcx> {
let cond_ty = self.check_expr_has_type(cond_expr, self.tcx.types.bool);
let cond_ty = self.check_expr_expect_type(cond_expr, ExpectIfCondition);
let cond_diverges = self.diverges.get();
self.diverges.set(Diverges::Maybe);

Expand Down Expand Up @@ -3637,19 +3665,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
hir::ExprAssign(ref lhs, ref rhs) => {
let lhs_ty = self.check_expr_with_lvalue_pref(&lhs, PreferMutLvalue);

let tcx = self.tcx;
if !tcx.expr_is_lval(&lhs) {
struct_span_err!(
tcx.sess, expr.span, E0070,
"invalid left-hand side expression")
.span_label(
expr.span,
"left-hand of expression not valid")
.emit();
}

let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty);

match expected {
ExpectIfCondition => (),
_ => {
// Only check this if not in an `if` condition, as the
// mistyped comparison help is more appropriate.
if !self.tcx.expr_is_lval(&lhs) {
struct_span_err!(
self.tcx.sess, expr.span, E0070,
"invalid left-hand side expression")
.span_label(
expr.span,
"left-hand of expression not valid")
.emit();
}
}
}

self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized);

if lhs_ty.references_error() || rhs_ty.references_error() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,25 @@ fn main() {
// `x { ... }` should not be interpreted as a struct literal here
if x = x {
//~^ ERROR mismatched types
//~| expected type `bool`
//~| found type `()`
//~| expected bool, found ()
//~| HELP did you mean to compare equality?
println!("{}", x);
}
// Explicit parentheses on the left should match behavior of above
if (x = x) {
//~^ ERROR mismatched types
//~| expected type `bool`
//~| found type `()`
//~| expected bool, found ()
//~| HELP did you mean to compare equality?
println!("{}", x);
}
// The struct literal interpretation is fine with explicit parentheses on the right
if y = (Foo { foo: x }) {
//~^ ERROR mismatched types
//~| expected type `bool`
//~| found type `()`
//~| expected bool, found ()
//~| HELP did you mean to compare equality?
println!("{}", x);
}
// "invalid left-hand side expression" error is suppresed
if 3 = x {
//~^ ERROR mismatched types
//~| HELP did you mean to compare equality?
println!("{}", x);
}
}
50 changes: 50 additions & 0 deletions src/test/ui/type-check/issue-17283.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
error[E0308]: mismatched types
--> $DIR/issue-17283.rs:25:8
|
25 | if x = x {
| ^^^^^
| |
| help: did you mean to compare equality? `x == x`
| expected bool, found ()
|
= note: expected type `bool`
found type `()`

error[E0308]: mismatched types
--> $DIR/issue-17283.rs:31:8
|
31 | if (x = x) {
| ^^^^^^^
| |
| help: did you mean to compare equality? `x == x`
| expected bool, found ()
|
= note: expected type `bool`
found type `()`

error[E0308]: mismatched types
--> $DIR/issue-17283.rs:37:8
|
37 | if y = (Foo { foo: x }) {
| ^^^^^^^^^^^^^^^^^^^^
| |
| help: did you mean to compare equality? `y == (Foo { foo: x })`
| expected bool, found ()
|
= note: expected type `bool`
found type `()`

error[E0308]: mismatched types
--> $DIR/issue-17283.rs:43:8
|
43 | if 3 = x {
| ^^^^^
| |
| help: did you mean to compare equality? `3 == x`
| expected bool, found ()
|
= note: expected type `bool`
found type `()`

error: aborting due to previous error(s)

0 comments on commit 028b5f9

Please sign in to comment.