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.

Sign up for GitHub

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

Jump to bottom

Point at enclosing match when expecting () in arm #64825

Merged
merged 3 commits into from
Sep 29, 2019
Merged

Point at enclosing match when expecting () in arm #64825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a167ed
Point at enclosing match when expecting `()` in arm
estebank Sep 27, 2019
ae51953
review comments
estebank Sep 28, 2019
c861e24
clean up
estebank Sep 28, 2019
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
23 changes: 11 additions & 12 deletions src/librustc/hir/lowering/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1039,10 +1039,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 @@ -1088,21 +1087,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 @@ -1117,7 +1116,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 @@ -1154,14 +1153,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 @@ -1173,7 +1172,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
Open in desktop
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> {
estebank marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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(
estebank marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3858,6 +3858,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 @@ -3881,7 +3890,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
Open in desktop
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