Skip to content

Commit 8109332

Browse files
CentrilCentril
authored
Rollup merge of #64825 - estebank:match-unit, r=Centril
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 #40799.
2 parents f34e2b1 + c861e24 commit 8109332

File tree

10 files changed

+180
-50
lines changed

10 files changed

+180
-50
lines changed

‎src/librustc/hir/lowering/expr.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,10 +1037,9 @@ impl LoweringContext<'_> {
10371037
) -> hir::Expr {
10381038
// expand <head>
10391039
let mut head = self.lower_expr(head);
1040-
let head_sp = head.span;
10411040
let desugared_span = self.mark_span_with_reason(
10421041
DesugaringKind::ForLoop,
1043-
head_sp,
1042+
head.span,
10441043
None,
10451044
);
10461045
head.span = desugared_span;
@@ -1086,21 +1085,21 @@ impl LoweringContext<'_> {
10861085

10871086
// `match ::std::iter::Iterator::next(&mut iter) { ... }`
10881087
let match_expr = {
1089-
let iter = P(self.expr_ident(head_sp, iter, iter_pat_nid));
1090-
let ref_mut_iter = self.expr_mut_addr_of(head_sp, iter);
1088+
let iter = P(self.expr_ident(desugared_span, iter, iter_pat_nid));
1089+
let ref_mut_iter = self.expr_mut_addr_of(desugared_span, iter);
10911090
let next_path = &[sym::iter, sym::Iterator, sym::next];
10921091
let next_expr = P(self.expr_call_std_path(
1093-
head_sp,
1092+
desugared_span,
10941093
next_path,
10951094
hir_vec![ref_mut_iter],
10961095
));
10971096
let arms = hir_vec![pat_arm, break_arm];
10981097

1099-
self.expr_match(head_sp, next_expr, arms, hir::MatchSource::ForLoopDesugar)
1098+
self.expr_match(desugared_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
11001099
};
1101-
let match_stmt = self.stmt_expr(head_sp, match_expr);
1100+
let match_stmt = self.stmt_expr(desugared_span, match_expr);
11021101

1103-
let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat_hid));
1102+
let next_expr = P(self.expr_ident(desugared_span, next_ident, next_pat_hid));
11041103

11051104
// `let mut __next`
11061105
let next_let = self.stmt_let_pat(
@@ -1115,7 +1114,7 @@ impl LoweringContext<'_> {
11151114
let pat = self.lower_pat(pat);
11161115
let pat_let = self.stmt_let_pat(
11171116
ThinVec::new(),
1118-
head_sp,
1117+
desugared_span,
11191118
Some(next_expr),
11201119
pat,
11211120
hir::LocalSource::ForLoopDesugar,
@@ -1152,14 +1151,14 @@ impl LoweringContext<'_> {
11521151
let into_iter_path =
11531152
&[sym::iter, sym::IntoIterator, sym::into_iter];
11541153
P(self.expr_call_std_path(
1155-
head_sp,
1154+
desugared_span,
11561155
into_iter_path,
11571156
hir_vec![head],
11581157
))
11591158
};
11601159

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

11771176
/// Desugar `ExprKind::Try` from: `<expr>?` into:

‎src/librustc/hir/map/mod.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,32 @@ impl<'hir> Map<'hir> {
818818
CRATE_HIR_ID
819819
}
820820

821+
/// When on a match arm tail expression or on a match arm, give back the enclosing `match`
822+
/// expression.
823+
///
824+
/// Used by error reporting when there's a type error in a match arm caused by the `match`
825+
/// expression needing to be unit.
826+
pub fn get_match_if_cause(&self, hir_id: HirId) -> Option<&Expr> {
827+
for (_, node) in ParentHirIterator::new(hir_id, &self) {
828+
match node {
829+
Node::Item(_) |
830+
Node::ForeignItem(_) |
831+
Node::TraitItem(_) |
832+
Node::ImplItem(_) => break,
833+
Node::Expr(expr) => match expr.kind {
834+
ExprKind::Match(_, _, _) => return Some(expr),
835+
_ => {}
836+
},
837+
Node::Stmt(stmt) => match stmt.kind {
838+
StmtKind::Local(_) => break,
839+
_ => {}
840+
}
841+
_ => {}
842+
}
843+
}
844+
None
845+
}
846+
821847
/// Returns the nearest enclosing scope. A scope is roughly an item or block.
822848
pub fn get_enclosing_scope(&self, hir_id: HirId) -> Option<HirId> {
823849
for (hir_id, node) in ParentHirIterator::new(hir_id, &self) {

‎src/librustc_typeck/check/_match.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
3636
// 2. By expecting `bool` for `expr` we get nice diagnostics for e.g. `if x = y { .. }`.
3737
//
3838
// FIXME(60707): Consider removing hack with principled solution.
39-
self.check_expr_has_type_or_error(discrim, self.tcx.types.bool)
39+
self.check_expr_has_type_or_error(discrim, self.tcx.types.bool, |_| {})
4040
} else {
4141
self.demand_discriminant_type(arms, discrim)
4242
};
@@ -106,7 +106,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
106106
if let Some(g) = &arm.guard {
107107
self.diverges.set(pats_diverge);
108108
match g {
109-
hir::Guard::If(e) => self.check_expr_has_type_or_error(e, tcx.types.bool),
109+
hir::Guard::If(e) => {
110+
self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {})
111+
}
110112
};
111113
}
112114

@@ -442,7 +444,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
442444
kind: TypeVariableOriginKind::TypeInference,
443445
span: discrim.span,
444446
});
445-
self.check_expr_has_type_or_error(discrim, discrim_ty);
447+
self.check_expr_has_type_or_error(discrim, discrim_ty, |_| {});
446448
discrim_ty
447449
}
448450
}

‎src/librustc_typeck/check/coercion.rs

Lines changed: 43 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,18 +1163,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
11631163
fcx.try_coerce(expression, expression_ty, self.expected_ty, AllowTwoPhase::No)
11641164
} else {
11651165
match self.expressions {
1166-
Expressions::Dynamic(ref exprs) =>
1167-
fcx.try_find_coercion_lub(cause,
1168-
exprs,
1169-
self.merged_ty(),
1170-
expression,
1171-
expression_ty),
1172-
Expressions::UpFront(ref coercion_sites) =>
1173-
fcx.try_find_coercion_lub(cause,
1174-
&coercion_sites[0..self.pushed],
1175-
self.merged_ty(),
1176-
expression,
1177-
expression_ty),
1166+
Expressions::Dynamic(ref exprs) => fcx.try_find_coercion_lub(
1167+
cause,
1168+
exprs,
1169+
self.merged_ty(),
1170+
expression,
1171+
expression_ty,
1172+
),
1173+
Expressions::UpFront(ref coercion_sites) => fcx.try_find_coercion_lub(
1174+
cause,
1175+
&coercion_sites[0..self.pushed],
1176+
self.merged_ty(),
1177+
expression,
1178+
expression_ty,
1179+
),
11781180
}
11791181
}
11801182
} else {
@@ -1216,7 +1218,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
12161218
self.pushed += 1;
12171219
}
12181220
}
1219-
Err(err) => {
1221+
Err(coercion_error) => {
12201222
let (expected, found) = if label_expression_as_expected {
12211223
// In the case where this is a "forced unit", like
12221224
// `break`, we want to call the `()` "expected"
@@ -1232,41 +1234,42 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
12321234
(self.final_ty.unwrap_or(self.expected_ty), expression_ty)
12331235
};
12341236

1235-
let mut db;
1237+
let mut err;
12361238
match cause.code {
12371239
ObligationCauseCode::ReturnNoExpression => {
1238-
db = struct_span_err!(
1240+
err = struct_span_err!(
12391241
fcx.tcx.sess, cause.span, E0069,
12401242
"`return;` in a function whose return type is not `()`");
1241-
db.span_label(cause.span, "return type is not `()`");
1243+
err.span_label(cause.span, "return type is not `()`");
12421244
}
12431245
ObligationCauseCode::BlockTailExpression(blk_id) => {
12441246
let parent_id = fcx.tcx.hir().get_parent_node(blk_id);
1245-
db = self.report_return_mismatched_types(
1247+
err = self.report_return_mismatched_types(
12461248
cause,
12471249
expected,
12481250
found,
1249-
err,
1251+
coercion_error,
12501252
fcx,
12511253
parent_id,
12521254
expression.map(|expr| (expr, blk_id)),
12531255
);
12541256
}
12551257
ObligationCauseCode::ReturnValue(id) => {
1256-
db = self.report_return_mismatched_types(
1257-
cause, expected, found, err, fcx, id, None);
1258+
err = self.report_return_mismatched_types(
1259+
cause, expected, found, coercion_error, fcx, id, None);
12581260
}
12591261
_ => {
1260-
db = fcx.report_mismatched_types(cause, expected, found, err);
1262+
err = fcx.report_mismatched_types(cause, expected, found, coercion_error);
12611263
}
12621264
}
12631265

12641266
if let Some(augment_error) = augment_error {
1265-
augment_error(&mut db);
1267+
augment_error(&mut err);
12661268
}
12671269

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

12711274
self.final_ty = Some(fcx.tcx.types.err);
12721275
}
@@ -1278,12 +1281,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
12781281
cause: &ObligationCause<'tcx>,
12791282
expected: Ty<'tcx>,
12801283
found: Ty<'tcx>,
1281-
err: TypeError<'tcx>,
1284+
ty_err: TypeError<'tcx>,
12821285
fcx: &FnCtxt<'a, 'tcx>,
12831286
id: hir::HirId,
12841287
expression: Option<(&'tcx hir::Expr, hir::HirId)>,
12851288
) -> DiagnosticBuilder<'a> {
1286-
let mut db = fcx.report_mismatched_types(cause, expected, found, err);
1289+
let mut err = fcx.report_mismatched_types(cause, expected, found, ty_err);
12871290

12881291
let mut pointing_at_return_type = false;
12891292
let mut return_sp = None;
@@ -1294,14 +1297,24 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
12941297
let parent_id = fcx.tcx.hir().get_parent_node(id);
12951298
let fn_decl = if let Some((expr, blk_id)) = expression {
12961299
pointing_at_return_type = fcx.suggest_mismatched_types_on_tail(
1297-
&mut db,
1300+
&mut err,
12981301
expr,
12991302
expected,
13001303
found,
13011304
cause.span,
13021305
blk_id,
13031306
);
13041307
let parent = fcx.tcx.hir().get(parent_id);
1308+
if let (Some(match_expr), true, false) = (
1309+
fcx.tcx.hir().get_match_if_cause(expr.hir_id),
1310+
expected.is_unit(),
1311+
pointing_at_return_type,
1312+
) {
1313+
if match_expr.span.desugaring_kind().is_none() {
1314+
err.span_label(match_expr.span, "expected this to be `()`");
1315+
fcx.suggest_semicolon_at_end(match_expr.span, &mut err);
1316+
}
1317+
}
13051318
fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
13061319
} else {
13071320
fcx.get_fn_decl(parent_id)
@@ -1310,20 +1323,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
13101323
if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) {
13111324
if expression.is_none() {
13121325
pointing_at_return_type |= fcx.suggest_missing_return_type(
1313-
&mut db, &fn_decl, expected, found, can_suggest);
1326+
&mut err, &fn_decl, expected, found, can_suggest);
13141327
}
13151328
if !pointing_at_return_type {
13161329
return_sp = Some(fn_decl.output.span()); // `impl Trait` return type
13171330
}
13181331
}
13191332
if let (Some(sp), Some(return_sp)) = (fcx.ret_coercion_span.borrow().as_ref(), return_sp) {
1320-
db.span_label(return_sp, "expected because this return type...");
1321-
db.span_label( *sp, format!(
1333+
err.span_label(return_sp, "expected because this return type...");
1334+
err.span_label( *sp, format!(
13221335
"...is found to be `{}` here",
13231336
fcx.resolve_type_vars_with_obligations(expected),
13241337
));
13251338
}
1326-
db
1339+
err
13271340
}
13281341

13291342
pub fn complete<'a>(self, fcx: &FnCtxt<'a, 'tcx>) -> Ty<'tcx> {

‎src/librustc_typeck/check/expr.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
5353
&self,
5454
expr: &'tcx hir::Expr,
5555
expected: Ty<'tcx>,
56+
extend_err: impl Fn(&mut DiagnosticBuilder<'_>),
5657
) -> Ty<'tcx> {
57-
self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected))
58+
self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected), extend_err)
5859
}
5960

6061
fn check_expr_meets_expectation_or_error(
6162
&self,
6263
expr: &'tcx hir::Expr,
6364
expected: Expectation<'tcx>,
65+
extend_err: impl Fn(&mut DiagnosticBuilder<'_>),
6466
) -> Ty<'tcx> {
6567
let expected_ty = expected.to_option(&self).unwrap_or(self.tcx.types.bool);
6668
let mut ty = self.check_expr_with_expectation(expr, expected);
@@ -88,6 +90,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
8890
ExprKind::DropTemps(expr) => expr,
8991
_ => expr,
9092
};
93+
extend_err(&mut err);
9194
// Error possibly reported in `check_assign` so avoid emitting error again.
9295
err.emit_unless(self.is_assign_to_bool(expr, expected_ty));
9396
}
@@ -971,7 +974,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
971974
kind: TypeVariableOriginKind::MiscVariable,
972975
span: element.span,
973976
});
974-
let element_ty = self.check_expr_has_type_or_error(&element, ty);
977+
let element_ty = self.check_expr_has_type_or_error(&element, ty, |_| {});
975978
(element_ty, ty)
976979
}
977980
};
@@ -1058,7 +1061,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10581061
// the fields with the base_expr. This could cause us to hit errors later
10591062
// when certain fields are assumed to exist that in fact do not.
10601063
if !error_happened {
1061-
self.check_expr_has_type_or_error(base_expr, adt_ty);
1064+
self.check_expr_has_type_or_error(base_expr, adt_ty, |_| {});
10621065
match adt_ty.kind {
10631066
ty::Adt(adt, substs) if adt.is_struct() => {
10641067
let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| {

‎src/librustc_typeck/check/mod.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3879,6 +3879,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
38793879
}
38803880
}
38813881

3882+
fn suggest_semicolon_at_end(&self, span: Span, err: &mut DiagnosticBuilder<'_>) {
3883+
err.span_suggestion_short(
3884+
span.shrink_to_hi(),
3885+
"consider using a semicolon here",
3886+
";".to_string(),
3887+
Applicability::MachineApplicable,
3888+
);
3889+
}
3890+
38823891
pub fn check_stmt(&self, stmt: &'tcx hir::Stmt) {
38833892
// Don't do all the complex logic below for `DeclItem`.
38843893
match stmt.kind {
@@ -3902,7 +3911,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
39023911
hir::StmtKind::Item(_) => {}
39033912
hir::StmtKind::Expr(ref expr) => {
39043913
// Check with expected type of `()`.
3905-
self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit());
3914+
3915+
self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| {
3916+
self.suggest_semicolon_at_end(expr.span, err);
3917+
});
39063918
}
39073919
hir::StmtKind::Semi(ref expr) => {
39083920
self.check_expr(&expr);

‎src/test/ui/struct-literal-variant-in-if.stderr

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ error[E0308]: mismatched types
5050
--> $DIR/struct-literal-variant-in-if.rs:10:20
5151
|
5252
LL | if x == E::V { field } {}
53-
| ^^^^^ expected (), found bool
53+
| ---------------^^^^^--- help: consider using a semicolon here
54+
| | |
55+
| | expected (), found bool
56+
| expected this to be `()`
5457
|
5558
= note: expected type `()`
5659
found type `bool`

0 commit comments

Comments
 (0)