Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 5d90e89

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#81769 - estebank:tail-expr-as-potential-return, r=lcnr
Suggest `return`ing tail expressions that match return type Some newcomers are confused by the behavior of tail expressions, interpreting that "leaving out the `;` makes it the return value". To help them go in the right direction, suggest using `return` instead when applicable.
2 parents 8e51bd4 + fc6c19e commit 5d90e89

22 files changed

+206
-48
lines changed

‎compiler/rustc_hir/src/hir.rs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// ignore-tidy-filelength
2-
use crate::def::{DefKind, Namespace, Res};
2+
use crate::def::{CtorKind, DefKind, Namespace, Res};
33
use crate::def_id::DefId;
44
crate use crate::hir_id::HirId;
55
use crate::{itemlikevisit, LangItem};
@@ -1576,6 +1576,63 @@ impl Expr<'_> {
15761576
}
15771577
expr
15781578
}
1579+
1580+
pub fn can_have_side_effects(&self) -> bool {
1581+
match self.peel_drop_temps().kind {
1582+
ExprKind::Path(_) | ExprKind::Lit(_) => false,
1583+
ExprKind::Type(base, _)
1584+
| ExprKind::Unary(_, base)
1585+
| ExprKind::Field(base, _)
1586+
| ExprKind::Index(base, _)
1587+
| ExprKind::AddrOf(.., base)
1588+
| ExprKind::Cast(base, _) => {
1589+
// This isn't exactly true for `Index` and all `Unnary`, but we are using this
1590+
// method exclusively for diagnostics and there's a *cultural* pressure against
1591+
// them being used only for its side-effects.
1592+
base.can_have_side_effects()
1593+
}
1594+
ExprKind::Struct(_, fields, init) => fields
1595+
.iter()
1596+
.map(|field| field.expr)
1597+
.chain(init.into_iter())
1598+
.all(|e| e.can_have_side_effects()),
1599+
1600+
ExprKind::Array(args)
1601+
| ExprKind::Tup(args)
1602+
| ExprKind::Call(
1603+
Expr {
1604+
kind:
1605+
ExprKind::Path(QPath::Resolved(
1606+
None,
1607+
Path { res: Res::Def(DefKind::Ctor(_, CtorKind::Fn), _), .. },
1608+
)),
1609+
..
1610+
},
1611+
args,
1612+
) => args.iter().all(|arg| arg.can_have_side_effects()),
1613+
ExprKind::If(..)
1614+
| ExprKind::Match(..)
1615+
| ExprKind::MethodCall(..)
1616+
| ExprKind::Call(..)
1617+
| ExprKind::Closure(..)
1618+
| ExprKind::Block(..)
1619+
| ExprKind::Repeat(..)
1620+
| ExprKind::Break(..)
1621+
| ExprKind::Continue(..)
1622+
| ExprKind::Ret(..)
1623+
| ExprKind::Loop(..)
1624+
| ExprKind::Assign(..)
1625+
| ExprKind::InlineAsm(..)
1626+
| ExprKind::LlvmInlineAsm(..)
1627+
| ExprKind::AssignOp(..)
1628+
| ExprKind::ConstBlock(..)
1629+
| ExprKind::Box(..)
1630+
| ExprKind::Binary(..)
1631+
| ExprKind::Yield(..)
1632+
| ExprKind::DropTemps(..)
1633+
| ExprKind::Err => true,
1634+
}
1635+
}
15791636
}
15801637

15811638
/// Checks if the specified expression is a built-in range literal.

‎compiler/rustc_typeck/src/check/callee.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
346346
if call_is_multiline {
347347
err.span_suggestion(
348348
callee.span.shrink_to_hi(),
349-
"try adding a semicolon",
349+
"consider using a semicolon here",
350350
";".to_owned(),
351351
Applicability::MaybeIncorrect,
352352
);

‎compiler/rustc_typeck/src/check/coercion.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,15 +1450,17 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
14501450
) {
14511451
if cond_expr.span.desugaring_kind().is_none() {
14521452
err.span_label(cond_expr.span, "expected this to be `()`");
1453-
fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
1453+
if expr.can_have_side_effects() {
1454+
fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
1455+
}
14541456
}
14551457
}
14561458
fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
14571459
} else {
14581460
fcx.get_fn_decl(parent_id)
14591461
};
14601462

1461-
if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) {
1463+
if let Some((fn_decl, can_suggest)) = fn_decl {
14621464
if expression.is_none() {
14631465
pointing_at_return_type |= fcx.suggest_missing_return_type(
14641466
&mut err,
@@ -1472,6 +1474,16 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
14721474
fn_output = Some(&fn_decl.output); // `impl Trait` return type
14731475
}
14741476
}
1477+
1478+
let parent_id = fcx.tcx.hir().get_parent_item(id);
1479+
let parent_item = fcx.tcx.hir().get(parent_id);
1480+
1481+
if let (Some((expr, _)), Some((fn_decl, _, _))) =
1482+
(expression, fcx.get_node_fn_decl(parent_item))
1483+
{
1484+
fcx.suggest_missing_return_expr(&mut err, expr, fn_decl, expected, found);
1485+
}
1486+
14751487
if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.get(), fn_output) {
14761488
self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output);
14771489
}

‎compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
561561
hir::StmtKind::Expr(ref expr) => {
562562
// Check with expected type of `()`.
563563
self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| {
564-
self.suggest_semicolon_at_end(expr.span, err);
564+
if expr.can_have_side_effects() {
565+
self.suggest_semicolon_at_end(expr.span, err);
566+
}
565567
});
566568
}
567569
hir::StmtKind::Semi(ref expr) => {

‎compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
4444
blk_id: hir::HirId,
4545
) -> bool {
4646
let expr = expr.peel_drop_temps();
47-
self.suggest_missing_semicolon(err, expr, expected, cause_span);
47+
if expr.can_have_side_effects() {
48+
self.suggest_missing_semicolon(err, expr, expected, cause_span);
49+
}
4850
let mut pointing_at_return_type = false;
4951
if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
5052
pointing_at_return_type =
5153
self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest);
54+
self.suggest_missing_return_expr(err, expr, &fn_decl, expected, found);
5255
}
5356
pointing_at_return_type
5457
}
@@ -392,10 +395,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
392395
| ExprKind::Loop(..)
393396
| ExprKind::If(..)
394397
| ExprKind::Match(..)
395-
| ExprKind::Block(..) => {
398+
| ExprKind::Block(..)
399+
if expression.can_have_side_effects() =>
400+
{
396401
err.span_suggestion(
397402
cause_span.shrink_to_hi(),
398-
"try adding a semicolon",
403+
"consider using a semicolon here",
399404
";".to_string(),
400405
Applicability::MachineApplicable,
401406
);
@@ -464,6 +469,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
464469
}
465470
}
466471

472+
pub(in super::super) fn suggest_missing_return_expr(
473+
&self,
474+
err: &mut DiagnosticBuilder<'_>,
475+
expr: &'tcx hir::Expr<'tcx>,
476+
fn_decl: &hir::FnDecl<'_>,
477+
expected: Ty<'tcx>,
478+
found: Ty<'tcx>,
479+
) {
480+
if !expected.is_unit() {
481+
return;
482+
}
483+
let found = self.resolve_vars_with_obligations(found);
484+
if let hir::FnRetTy::Return(ty) = fn_decl.output {
485+
let ty = AstConv::ast_ty_to_ty(self, ty);
486+
let ty = self.normalize_associated_types_in(expr.span, ty);
487+
if self.can_coerce(found, ty) {
488+
err.multipart_suggestion(
489+
"you might have meant to return this value",
490+
vec![
491+
(expr.span.shrink_to_lo(), "return ".to_string()),
492+
(expr.span.shrink_to_hi(), ";".to_string()),
493+
],
494+
Applicability::MaybeIncorrect,
495+
);
496+
}
497+
}
498+
}
499+
467500
pub(in super::super) fn suggest_missing_parentheses(
468501
&self,
469502
err: &mut DiagnosticBuilder<'_>,

‎src/test/ui/async-await/suggest-missing-await.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ async fn dummy() {}
2121
async fn suggest_await_in_async_fn_return() {
2222
dummy()
2323
//~^ ERROR mismatched types [E0308]
24-
//~| HELP try adding a semicolon
24+
//~| HELP consider using a semicolon here
2525
//~| HELP consider `await`ing on the `Future`
2626
//~| SUGGESTION .await
2727
}

‎src/test/ui/async-await/suggest-missing-await.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ help: consider `await`ing on the `Future`
2929
|
3030
LL | dummy().await
3131
| ^^^^^^
32-
help: try adding a semicolon
32+
help: consider using a semicolon here
3333
|
3434
LL | dummy();
3535
| ^

‎src/test/ui/block-result/block-must-not-have-result-while.stderr

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ LL | | true
1414
| | ^^^^ expected `()`, found `bool`
1515
LL | |
1616
LL | | }
17-
| | -- help: consider using a semicolon here
18-
| |_____|
19-
| expected this to be `()`
17+
| |_____- expected this to be `()`
2018

2119
error: aborting due to previous error; 1 warning emitted
2220

‎src/test/ui/block-result/unexpected-return-on-unit.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error[E0308]: mismatched types
44
LL | foo()
55
| ^^^^^ expected `()`, found `usize`
66
|
7-
help: try adding a semicolon
7+
help: consider using a semicolon here
88
|
99
LL | foo();
1010
| ^

‎src/test/ui/macros/empty-trailing-stmt.stderr

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ error[E0308]: mismatched types
33
|
44
LL | { true }
55
| ^^^^ expected `()`, found `bool`
6+
|
7+
help: you might have meant to return this value
8+
|
9+
LL | { return true; }
10+
| ^^^^^^ ^
611

712
error[E0308]: mismatched types
813
--> $DIR/empty-trailing-stmt.rs:5:13

‎src/test/ui/parser/expr-as-stmt-2.stderr

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,29 @@ error[E0308]: mismatched types
22
--> $DIR/expr-as-stmt-2.rs:3:26
33
|
44
LL | if let Some(x) = a { true } else { false }
5-
| ---------------------^^^^------------------ help: consider using a semicolon here
5+
| ---------------------^^^^-----------------
66
| | |
77
| | expected `()`, found `bool`
88
| expected this to be `()`
9+
|
10+
help: you might have meant to return this value
11+
|
12+
LL | if let Some(x) = a { return true; } else { false }
13+
| ^^^^^^ ^
914

1015
error[E0308]: mismatched types
1116
--> $DIR/expr-as-stmt-2.rs:3:40
1217
|
1318
LL | if let Some(x) = a { true } else { false }
14-
| -----------------------------------^^^^^--- help: consider using a semicolon here
19+
| -----------------------------------^^^^^--
1520
| | |
1621
| | expected `()`, found `bool`
1722
| expected this to be `()`
23+
|
24+
help: you might have meant to return this value
25+
|
26+
LL | if let Some(x) = a { true } else { return false; }
27+
| ^^^^^^ ^
1828

1929
error[E0308]: mismatched types
2030
--> $DIR/expr-as-stmt-2.rs:6:5

‎src/test/ui/parser/expr-as-stmt.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,24 +40,44 @@ error[E0308]: mismatched types
4040
|
4141
LL | {2} + {2}
4242
| ^ expected `()`, found integer
43+
|
44+
help: you might have meant to return this value
45+
|
46+
LL | {return 2;} + {2}
47+
| ^^^^^^ ^
4348

4449
error[E0308]: mismatched types
4550
--> $DIR/expr-as-stmt.rs:12:6
4651
|
4752
LL | {2} + 2
4853
| ^ expected `()`, found integer
54+
|
55+
help: you might have meant to return this value
56+
|
57+
LL | {return 2;} + 2
58+
| ^^^^^^ ^
4959

5060
error[E0308]: mismatched types
5161
--> $DIR/expr-as-stmt.rs:18:7
5262
|
5363
LL | { 42 } + foo;
5464
| ^^ expected `()`, found integer
65+
|
66+
help: you might have meant to return this value
67+
|
68+
LL | { return 42; } + foo;
69+
| ^^^^^^ ^
5570

5671
error[E0308]: mismatched types
5772
--> $DIR/expr-as-stmt.rs:24:7
5873
|
5974
LL | { 3 } * 3
6075
| ^ expected `()`, found integer
76+
|
77+
help: you might have meant to return this value
78+
|
79+
LL | { return 3; } * 3
80+
| ^^^^^^ ^
6181

6282
error[E0614]: type `{integer}` cannot be dereferenced
6383
--> $DIR/expr-as-stmt.rs:24:11

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ error[E0308]: mismatched types
5757
--> $DIR/struct-literal-variant-in-if.rs:10:20
5858
|
5959
LL | if x == E::V { field } {}
60-
| ---------------^^^^^--- help: consider using a semicolon here
60+
| ---------------^^^^^--
6161
| | |
6262
| | expected `()`, found `bool`
6363
| expected this to be `()`

‎src/test/ui/proc-macro/issue-37788.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | fn main() {
55
| - expected `()` because of default return type
66
LL | // Test that constructing the `visible_parent_map` (in `cstore_impl.rs`) does not ICE.
77
LL | std::cell::Cell::new(0)
8-
| ^^^^^^^^^^^^^^^^^^^^^^^- help: try adding a semicolon: `;`
8+
| ^^^^^^^^^^^^^^^^^^^^^^^- help: consider using a semicolon here: `;`
99
| |
1010
| expected `()`, found struct `Cell`
1111
|

‎src/test/ui/return/return-type.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | foo(4 as usize)
66
|
77
= note: expected unit type `()`
88
found struct `S<usize>`
9-
help: try adding a semicolon
9+
help: consider using a semicolon here
1010
|
1111
LL | foo(4 as usize);
1212
| ^

‎src/test/ui/return/tail-expr-as-potential-return.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
fn main() {
2+
let _ = foo(true);
3+
}
4+
5+
fn foo(x: bool) -> Result<f64, i32> {
6+
if x {
7+
Err(42) //~ ERROR mismatched types
8+
}
9+
Ok(42.0)
10+
}

‎src/test/ui/return/tail-expr-as-potential-return.stderr

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/tail-expr-as-potential-return.rs:7:9
3+
|
4+
LL | / if x {
5+
LL | | Err(42)
6+
| | ^^^^^^^ expected `()`, found enum `Result`
7+
LL | | }
8+
| |_____- expected this to be `()`
9+
|
10+
= note: expected unit type `()`
11+
found enum `Result<_, {integer}>`
12+
help: you might have meant to return this value
13+
|
14+
LL | return Err(42);
15+
| ^^^^^^ ^
16+
17+
error: aborting due to previous error
18+
19+
For more information about this error, try `rustc --explain E0308`.

‎src/test/ui/specialization/specialization-default-projection.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ LL | fn monomorphic() -> () {
2929
| -- expected `()` because of return type
3030
...
3131
LL | generic::<()>()
32-
| ^^^^^^^^^^^^^^^- help: try adding a semicolon: `;`
32+
| ^^^^^^^^^^^^^^^- help: consider using a semicolon here: `;`
3333
| |
3434
| expected `()`, found associated type
3535
|

‎src/test/ui/suggestions/issue-51055-missing-semicolon-between-call-and-tuple.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | fn vindictive() -> bool { true }
55
| ----------------------- `vindictive` defined here returns `bool`
66
...
77
LL | vindictive()
8-
| -^^^^^^^^^^^- help: try adding a semicolon: `;`
8+
| -^^^^^^^^^^^- help: consider using a semicolon here: `;`
99
| _____|
1010
| |
1111
LL | | (1, 2)

0 commit comments

Comments
 (0)