Skip to content
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

Generalize diagnostic for x = y where bool is the expected type #59439

Merged
merged 4 commits into from
Mar 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 6 additions & 1 deletion src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,12 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
augment_error(&mut db);
}

db.emit();
if expression.filter(|e| fcx.is_assign_to_bool(e, expected)).is_some() {
// Error reported in `check_assign` so avoid emitting error again.
db.delay_as_bug();
} else {
db.emit();
}

self.final_ty = Some(fcx.tcx.types.err);
}
Expand Down
83 changes: 52 additions & 31 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,44 +119,65 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let expr_ty = self.resolve_type_vars_with_obligations(checked_ty);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);

// If the expected type is an enum (Issue #55250) with any variants whose
// sole field is of the found type, suggest such variants. (Issue #42764)
if let ty::Adt(expected_adt, substs) = expected.sty {
if expected_adt.is_enum() {
let mut compatible_variants = expected_adt.variants
.iter()
.filter(|variant| variant.fields.len() == 1)
.filter_map(|variant| {
let sole_field = &variant.fields[0];
let sole_field_ty = sole_field.ty(self.tcx, substs);
if self.can_coerce(expr_ty, sole_field_ty) {
let variant_path = self.tcx.def_path_str(variant.def_id);
// FIXME #56861: DRYer prelude filtering
Some(variant_path.trim_start_matches("std::prelude::v1::").to_string())
} else {
None
}
}).peekable();

if compatible_variants.peek().is_some() {
let expr_text = print::to_string(print::NO_ANN, |s| s.print_expr(expr));
let suggestions = compatible_variants
.map(|v| format!("{}({})", v, expr_text));
err.span_suggestions(
expr.span,
"try using a variant of the expected type",
suggestions,
Applicability::MaybeIncorrect,
);
}
}
if self.is_assign_to_bool(expr, expected) {
// Error reported in `check_assign` so avoid emitting error again.
err.delay_as_bug();
return (expected, None)
}

self.suggest_compatible_variants(&mut err, expr, expected, expr_ty);
self.suggest_ref_or_into(&mut err, expr, expected, expr_ty);

(expected, Some(err))
}

/// Returns whether the expected type is `bool` and the expression is `x = y`.
pub fn is_assign_to_bool(&self, expr: &hir::Expr, expected: Ty<'tcx>) -> bool {
if let hir::ExprKind::Assign(..) = expr.node {
return expected == self.tcx.types.bool;
}
false
}

/// If the expected type is an enum (Issue #55250) with any variants whose
/// sole field is of the found type, suggest such variants. (Issue #42764)
fn suggest_compatible_variants(
&self,
err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr,
expected: Ty<'tcx>,
expr_ty: Ty<'tcx>,
) {
if let ty::Adt(expected_adt, substs) = expected.sty {
if !expected_adt.is_enum() {
return;
}

let mut compatible_variants = expected_adt.variants
.iter()
.filter(|variant| variant.fields.len() == 1)
.filter_map(|variant| {
let sole_field = &variant.fields[0];
let sole_field_ty = sole_field.ty(self.tcx, substs);
if self.can_coerce(expr_ty, sole_field_ty) {
let variant_path = self.tcx.def_path_str(variant.def_id);
// FIXME #56861: DRYer prelude filtering
Some(variant_path.trim_start_matches("std::prelude::v1::").to_string())
} else {
None
}
}).peekable();

if compatible_variants.peek().is_some() {
let expr_text = print::to_string(print::NO_ANN, |s| s.print_expr(expr));
let suggestions = compatible_variants
.map(|v| format!("{}({})", v, expr_text));
let msg = "try using a variant of the expected type";
err.span_suggestions(expr.span, msg, suggestions, Applicability::MaybeIncorrect);
}
}
}

pub fn get_conversion_methods(&self, span: Span, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
-> Vec<AssociatedItem> {
let mut methods = self.probe_for_return_type(span,
Expand Down
106 changes: 53 additions & 53 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,6 @@ 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 @@ -328,7 +325,6 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
fn resolve(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Expectation<'tcx> {
match self {
NoExpectation => NoExpectation,
ExpectIfCondition => ExpectIfCondition,
ExpectCastableToType(t) => {
ExpectCastableToType(fcx.resolve_type_vars_if_possible(&t))
}
Expand All @@ -344,7 +340,6 @@ 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 @@ -358,7 +353,6 @@ 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),
NoExpectation | ExpectCastableToType(_) | ExpectRvalueLikeUnsized(_) => None,
}
}
Expand Down Expand Up @@ -3150,25 +3144,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}

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.
if let (ExpectIfCondition, &ExprKind::Assign(ref lhs, ref rhs))
= (expected, &expr.node)
{
let msg = "try comparing for equality";
if let (Ok(left), Ok(right)) = (
self.tcx.sess.source_map().span_to_snippet(lhs.span),
self.tcx.sess.source_map().span_to_snippet(rhs.span))
{
err.span_suggestion(
expr.span,
msg,
format!("{} == {}", left, right),
Applicability::MaybeIncorrect);
} else {
err.help(msg);
}
if self.is_assign_to_bool(expr, expected_ty) {
// Error reported in `check_assign` so avoid emitting error again.
// FIXME(centril): Consider removing if/when `if` desugars to `match`.
err.delay_as_bug();
} else {
err.emit();
}
err.emit();
}
ty
}
Expand Down Expand Up @@ -3339,7 +3321,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_meets_expectation_or_error(cond_expr, ExpectIfCondition);
let cond_ty = self.check_expr_has_type_or_error(cond_expr, self.tcx.types.bool);
let cond_diverges = self.diverges.get();
self.diverges.set(Diverges::Maybe);

Expand Down Expand Up @@ -4424,34 +4406,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
tcx.types.never
}
ExprKind::Assign(ref lhs, ref rhs) => {
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);

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

match expected {
ExpectIfCondition => {
self.tcx.sess.delay_span_bug(lhs.span, "invalid lhs expression in if;\
expected error elsehwere");
}
_ => {
// Only check this if not in an `if` condition, as the
// mistyped comparison help is more appropriate.
if !lhs.is_place_expr() {
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() {
tcx.types.err
} else {
tcx.mk_unit()
}
self.check_assign(expr, expected, lhs, rhs)
}
ExprKind::If(ref cond, ref then_expr, ref opt_else_expr) => {
self.check_then_else(&cond, then_expr, opt_else_expr.as_ref().map(|e| &**e),
Expand Down Expand Up @@ -4752,6 +4707,51 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
}

/// Type check assignment expression `expr` of form `lhs = rhs`.
/// The expected type is `()` and is passsed to the function for the purposes of diagnostics.
fn check_assign(
&self,
expr: &'gcx hir::Expr,
expected: Expectation<'tcx>,
lhs: &'gcx hir::Expr,
rhs: &'gcx hir::Expr,
) -> Ty<'tcx> {
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty);

let expected_ty = expected.coercion_target_type(self, expr.span);
if expected_ty == self.tcx.types.bool {
// The expected type is `bool` but this will result in `()` so we can reasonably
// say that the user intended to write `lhs == rhs` instead of `lhs = rhs`.
// The likely cause of this is `if foo = bar { .. }`.
let actual_ty = self.tcx.mk_unit();
let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap();
let msg = "try comparing for equality";
let left = self.tcx.sess.source_map().span_to_snippet(lhs.span);
let right = self.tcx.sess.source_map().span_to_snippet(rhs.span);
if let (Ok(left), Ok(right)) = (left, right) {
let help = format!("{} == {}", left, right);
err.span_suggestion(expr.span, msg, help, Applicability::MaybeIncorrect);
} else {
err.help(msg);
}
err.emit();
} else if !lhs.is_place_expr() {
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() {
self.tcx.types.err
} else {
self.tcx.mk_unit()
}
}

// Finish resolving a path in a struct expression or pattern `S::A { .. }` if necessary.
// The newly resolved definition is written into `type_dependent_defs`.
fn finish_resolving_struct_path(&self,
Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/type/type-check/assignment-expected-bool.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// The purpose of this text is to ensure that we get good
// diagnostics when a `bool` is expected but that due to
// an assignment expression `x = y` the type is `()`.

fn main() {
let _: bool = 0 = 0; //~ ERROR mismatched types [E0308]

let _: bool = match 0 {
0 => 0 = 0, //~ ERROR mismatched types [E0308]
_ => 0 = 0, //~ ERROR mismatched types [E0308]
};

let _: bool = match true {
true => 0 = 0, //~ ERROR mismatched types [E0308]
_ => (),
};

if 0 = 0 {} //~ ERROR mismatched types [E0308]

let _: bool = if { 0 = 0 } { //~ ERROR mismatched types [E0308]
0 = 0 //~ ERROR mismatched types [E0308]
} else {
0 = 0 //~ ERROR mismatched types [E0308]
};

let _ = (0 = 0) //~ ERROR mismatched types [E0308]
&& { 0 = 0 } //~ ERROR mismatched types [E0308]
|| (0 = 0); //~ ERROR mismatched types [E0308]

// A test to check that not expecting `bool` behaves well:
let _: usize = 0 = 0;
//~^ ERROR mismatched types [E0308]
//~| ERROR invalid left-hand side expression [E0070]
}
Centril marked this conversation as resolved.
Show resolved Hide resolved
Loading