Skip to content

Commit

Permalink
generalize diagnostic for x = y where type bool is expected.
Browse files Browse the repository at this point in the history
  • Loading branch information
Centril committed Mar 27, 2019
1 parent 267fb90 commit 7945eff
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 85 deletions.
7 changes: 6 additions & 1 deletion src/librustc_typeck/check/coercion.rs
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
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
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

0 comments on commit 7945eff

Please sign in to comment.