Skip to content

Commit

Permalink
Rollup merge of rust-lang#64825 - estebank:match-unit, r=Centril
Browse files Browse the repository at this point in the history 
Point at enclosing match when expecting `()` in arm

When encountering code like the following:

```rust
fn main() {
    match 3 {
        4 => 1,
        3 => {
            println!("Yep it maches.");
            2
        }
        _ => 2
    }
    println!("Bye!")
}
```

point at the enclosing `match` expression and suggest ignoring the
returned value:

```
error[E0308]: mismatched types
  --> $DIR/match-needing-semi.rs:8:13
   |
LL | /     match 3 {
LL | |         4 => 1,
LL | |         3 => {
LL | |             2
   | |             ^ expected (), found integer
LL | |         }
LL | |         _ => 2
LL | |     }
   | |     -- help: consider using a semicolon here
   | |_____|
   |       expected this to be `()`
   |
   = note: expected type `()`
              found type `{integer}
```

Fix rust-lang#40799.
  • Loading branch information
@Centril
Centril committed Sep 29, 2019
2 parents f34e2b1 + c861e24 commit 8109332
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 50 deletions.
23 changes: 11 additions & 12 deletions src/librustc/hir/lowering/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1037,10 +1037,9 @@ impl LoweringContext<'_> {
) -> hir::Expr {
// expand <head>
let mut head = self.lower_expr(head);
let head_sp = head.span;
let desugared_span = self.mark_span_with_reason(
DesugaringKind::ForLoop,
head_sp,
head.span,
None,
);
head.span = desugared_span;
Expand Down Expand Up @@ -1086,21 +1085,21 @@ impl LoweringContext<'_> {

// `match ::std::iter::Iterator::next(&mut iter) { ... }`
let match_expr = {
let iter = P(self.expr_ident(head_sp, iter, iter_pat_nid));
let ref_mut_iter = self.expr_mut_addr_of(head_sp, iter);
let iter = P(self.expr_ident(desugared_span, iter, iter_pat_nid));
let ref_mut_iter = self.expr_mut_addr_of(desugared_span, iter);
let next_path = &[sym::iter, sym::Iterator, sym::next];
let next_expr = P(self.expr_call_std_path(
head_sp,
desugared_span,
next_path,
hir_vec![ref_mut_iter],
));
let arms = hir_vec![pat_arm, break_arm];

self.expr_match(head_sp, next_expr, arms, hir::MatchSource::ForLoopDesugar)
self.expr_match(desugared_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
};
let match_stmt = self.stmt_expr(head_sp, match_expr);
let match_stmt = self.stmt_expr(desugared_span, match_expr);

let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat_hid));
let next_expr = P(self.expr_ident(desugared_span, next_ident, next_pat_hid));

// `let mut __next`
let next_let = self.stmt_let_pat(
Expand All @@ -1115,7 +1114,7 @@ impl LoweringContext<'_> {
let pat = self.lower_pat(pat);
let pat_let = self.stmt_let_pat(
ThinVec::new(),
head_sp,
desugared_span,
Some(next_expr),
pat,
hir::LocalSource::ForLoopDesugar,
Expand Down Expand Up @@ -1152,14 +1151,14 @@ impl LoweringContext<'_> {
let into_iter_path =
&[sym::iter, sym::IntoIterator, sym::into_iter];
P(self.expr_call_std_path(
head_sp,
desugared_span,
into_iter_path,
hir_vec![head],
))
};

let match_expr = P(self.expr_match(
head_sp,
desugared_span,
into_iter_expr,
hir_vec![iter_arm],
hir::MatchSource::ForLoopDesugar,
Expand All @@ -1171,7 +1170,7 @@ impl LoweringContext<'_> {
// surrounding scope of the `match` since the `match` is not a terminating scope.
//
// Also, add the attributes to the outer returned expr node.
self.expr_drop_temps(head_sp, match_expr, e.attrs.clone())
self.expr_drop_temps(desugared_span, match_expr, e.attrs.clone())
}

/// Desugar `ExprKind::Try` from: `<expr>?` into:
Expand Down
26 changes: 26 additions & 0 deletions src/librustc/hir/map/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,32 @@ impl<'hir> Map<'hir> {
CRATE_HIR_ID
}

/// When on a match arm tail expression or on a match arm, give back the enclosing `match`
/// expression.
///
/// Used by error reporting when there's a type error in a match arm caused by the `match`
/// expression needing to be unit.
pub fn get_match_if_cause(&self, hir_id: HirId) -> Option<&Expr> {
for (_, node) in ParentHirIterator::new(hir_id, &self) {
match node {
Node::Item(_) |
Node::ForeignItem(_) |
Node::TraitItem(_) |
Node::ImplItem(_) => break,
Node::Expr(expr) => match expr.kind {
ExprKind::Match(_, _, _) => return Some(expr),
_ => {}
},
Node::Stmt(stmt) => match stmt.kind {
StmtKind::Local(_) => break,
_ => {}
}
_ => {}
}
}
None
}

/// Returns the nearest enclosing scope. A scope is roughly an item or block.
pub fn get_enclosing_scope(&self, hir_id: HirId) -> Option<HirId> {
for (hir_id, node) in ParentHirIterator::new(hir_id, &self) {
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_typeck/check/_match.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 2. By expecting `bool` for `expr` we get nice diagnostics for e.g. `if x = y { .. }`.
//
// FIXME(60707): Consider removing hack with principled solution.
self.check_expr_has_type_or_error(discrim, self.tcx.types.bool)
self.check_expr_has_type_or_error(discrim, self.tcx.types.bool, |_| {})
} else {
self.demand_discriminant_type(arms, discrim)
};
Expand Down Expand Up @@ -106,7 +106,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some(g) = &arm.guard {
self.diverges.set(pats_diverge);
match g {
hir::Guard::If(e) => self.check_expr_has_type_or_error(e, tcx.types.bool),
hir::Guard::If(e) => {
self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {})
}
};
}

Expand Down Expand Up @@ -442,7 +444,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
kind: TypeVariableOriginKind::TypeInference,
span: discrim.span,
});
self.check_expr_has_type_or_error(discrim, discrim_ty);
self.check_expr_has_type_or_error(discrim, discrim_ty, |_| {});
discrim_ty
}
}
Expand Down
73 changes: 43 additions & 30 deletions src/librustc_typeck/check/coercion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1163,18 +1163,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
fcx.try_coerce(expression, expression_ty, self.expected_ty, AllowTwoPhase::No)
} else {
match self.expressions {
Expressions::Dynamic(ref exprs) =>
fcx.try_find_coercion_lub(cause,
exprs,
self.merged_ty(),
expression,
expression_ty),
Expressions::UpFront(ref coercion_sites) =>
fcx.try_find_coercion_lub(cause,
&coercion_sites[0..self.pushed],
self.merged_ty(),
expression,
expression_ty),
Expressions::Dynamic(ref exprs) => fcx.try_find_coercion_lub(
cause,
exprs,
self.merged_ty(),
expression,
expression_ty,
),
Expressions::UpFront(ref coercion_sites) => fcx.try_find_coercion_lub(
cause,
&coercion_sites[0..self.pushed],
self.merged_ty(),
expression,
expression_ty,
),
}
}
} else {
Expand Down Expand Up @@ -1216,7 +1218,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
self.pushed += 1;
}
}
Err(err) => {
Err(coercion_error) => {
let (expected, found) = if label_expression_as_expected {
// In the case where this is a "forced unit", like
// `break`, we want to call the `()` "expected"
Expand All @@ -1232,41 +1234,42 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
(self.final_ty.unwrap_or(self.expected_ty), expression_ty)
};

let mut db;
let mut err;
match cause.code {
ObligationCauseCode::ReturnNoExpression => {
db = struct_span_err!(
err = struct_span_err!(
fcx.tcx.sess, cause.span, E0069,
"`return;` in a function whose return type is not `()`");
db.span_label(cause.span, "return type is not `()`");
err.span_label(cause.span, "return type is not `()`");
}
ObligationCauseCode::BlockTailExpression(blk_id) => {
let parent_id = fcx.tcx.hir().get_parent_node(blk_id);
db = self.report_return_mismatched_types(
err = self.report_return_mismatched_types(
cause,
expected,
found,
err,
coercion_error,
fcx,
parent_id,
expression.map(|expr| (expr, blk_id)),
);
}
ObligationCauseCode::ReturnValue(id) => {
db = self.report_return_mismatched_types(
cause, expected, found, err, fcx, id, None);
err = self.report_return_mismatched_types(
cause, expected, found, coercion_error, fcx, id, None);
}
_ => {
db = fcx.report_mismatched_types(cause, expected, found, err);
err = fcx.report_mismatched_types(cause, expected, found, coercion_error);
}
}

if let Some(augment_error) = augment_error {
augment_error(&mut db);
augment_error(&mut err);
}

// Error possibly reported in `check_assign` so avoid emitting error again.
db.emit_unless(expression.filter(|e| fcx.is_assign_to_bool(e, expected)).is_some());
err.emit_unless(expression.filter(|e| fcx.is_assign_to_bool(e, expected))
.is_some());

self.final_ty = Some(fcx.tcx.types.err);
}
Expand All @@ -1278,12 +1281,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
cause: &ObligationCause<'tcx>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
err: TypeError<'tcx>,
ty_err: TypeError<'tcx>,
fcx: &FnCtxt<'a, 'tcx>,
id: hir::HirId,
expression: Option<(&'tcx hir::Expr, hir::HirId)>,
) -> DiagnosticBuilder<'a> {
let mut db = fcx.report_mismatched_types(cause, expected, found, err);
let mut err = fcx.report_mismatched_types(cause, expected, found, ty_err);

let mut pointing_at_return_type = false;
let mut return_sp = None;
Expand All @@ -1294,14 +1297,24 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
let parent_id = fcx.tcx.hir().get_parent_node(id);
let fn_decl = if let Some((expr, blk_id)) = expression {
pointing_at_return_type = fcx.suggest_mismatched_types_on_tail(
&mut db,
&mut err,
expr,
expected,
found,
cause.span,
blk_id,
);
let parent = fcx.tcx.hir().get(parent_id);
if let (Some(match_expr), true, false) = (
fcx.tcx.hir().get_match_if_cause(expr.hir_id),
expected.is_unit(),
pointing_at_return_type,
) {
if match_expr.span.desugaring_kind().is_none() {
err.span_label(match_expr.span, "expected this to be `()`");
fcx.suggest_semicolon_at_end(match_expr.span, &mut err);
}
}
fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
} else {
fcx.get_fn_decl(parent_id)
Expand All @@ -1310,20 +1323,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) {
if expression.is_none() {
pointing_at_return_type |= fcx.suggest_missing_return_type(
&mut db, &fn_decl, expected, found, can_suggest);
&mut err, &fn_decl, expected, found, can_suggest);
}
if !pointing_at_return_type {
return_sp = Some(fn_decl.output.span()); // `impl Trait` return type
}
}
if let (Some(sp), Some(return_sp)) = (fcx.ret_coercion_span.borrow().as_ref(), return_sp) {
db.span_label(return_sp, "expected because this return type...");
db.span_label( *sp, format!(
err.span_label(return_sp, "expected because this return type...");
err.span_label( *sp, format!(
"...is found to be `{}` here",
fcx.resolve_type_vars_with_obligations(expected),
));
}
db
err
}

pub fn complete<'a>(self, fcx: &FnCtxt<'a, 'tcx>) -> Ty<'tcx> {
Expand Down
9 changes: 6 additions & 3 deletions src/librustc_typeck/check/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
expr: &'tcx hir::Expr,
expected: Ty<'tcx>,
extend_err: impl Fn(&mut DiagnosticBuilder<'_>),
) -> Ty<'tcx> {
self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected))
self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected), extend_err)
}

fn check_expr_meets_expectation_or_error(
&self,
expr: &'tcx hir::Expr,
expected: Expectation<'tcx>,
extend_err: impl Fn(&mut DiagnosticBuilder<'_>),
) -> 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);
Expand Down Expand Up @@ -88,6 +90,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::DropTemps(expr) => expr,
_ => expr,
};
extend_err(&mut err);
// Error possibly reported in `check_assign` so avoid emitting error again.
err.emit_unless(self.is_assign_to_bool(expr, expected_ty));
}
Expand Down Expand Up @@ -971,7 +974,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
kind: TypeVariableOriginKind::MiscVariable,
span: element.span,
});
let element_ty = self.check_expr_has_type_or_error(&element, ty);
let element_ty = self.check_expr_has_type_or_error(&element, ty, |_| {});
(element_ty, ty)
}
};
Expand Down Expand Up @@ -1058,7 +1061,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// the fields with the base_expr. This could cause us to hit errors later
// when certain fields are assumed to exist that in fact do not.
if !error_happened {
self.check_expr_has_type_or_error(base_expr, adt_ty);
self.check_expr_has_type_or_error(base_expr, adt_ty, |_| {});
match adt_ty.kind {
ty::Adt(adt, substs) if adt.is_struct() => {
let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| {
Expand Down
14 changes: 13 additions & 1 deletion src/librustc_typeck/check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3879,6 +3879,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn suggest_semicolon_at_end(&self, span: Span, err: &mut DiagnosticBuilder<'_>) {
err.span_suggestion_short(
span.shrink_to_hi(),
"consider using a semicolon here",
";".to_string(),
Applicability::MachineApplicable,
);
}

pub fn check_stmt(&self, stmt: &'tcx hir::Stmt) {
// Don't do all the complex logic below for `DeclItem`.
match stmt.kind {
Expand All @@ -3902,7 +3911,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
hir::StmtKind::Item(_) => {}
hir::StmtKind::Expr(ref expr) => {
// Check with expected type of `()`.
self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit());

self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| {
self.suggest_semicolon_at_end(expr.span, err);
});
}
hir::StmtKind::Semi(ref expr) => {
self.check_expr(&expr);
Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/struct-literal-variant-in-if.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ error[E0308]: mismatched types
--> $DIR/struct-literal-variant-in-if.rs:10:20
|
LL | if x == E::V { field } {}
| ^^^^^ expected (), found bool
| ---------------^^^^^--- help: consider using a semicolon here
| | |
| | expected (), found bool
| expected this to be `()`
|
= note: expected type `()`
found type `bool`
Expand Down
Loading

0 comments on commit 8109332

Please sign in to comment.