Skip to content

Modify mismatched type error for functions with no return #57230

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.

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

Merged
merged 4 commits into from
Jan 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ impl<'hir> Map<'hir> {
Node::Item(_) |
Node::ForeignItem(_) |
Node::TraitItem(_) |
Node::Expr(Expr { node: ExprKind::Closure(..), ..}) |
Node::ImplItem(_) => true,
_ => false,
}
Expand All @@ -675,7 +676,7 @@ impl<'hir> Map<'hir> {
match *node {
Node::Expr(ref expr) => {
match expr.node {
ExprKind::While(..) | ExprKind::Loop(..) => true,
ExprKind::While(..) | ExprKind::Loop(..) | ExprKind::Ret(..) => true,
_ => false,
}
}
Expand Down
128 changes: 85 additions & 43 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4791,11 +4791,31 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// #41425 -- label the implicit `()` as being the
// "found type" here, rather than the "expected type".
if !self.diverges.get().always() {
coerce.coerce_forced_unit(self, &self.misc(blk.span), &mut |err| {
// #50009 -- Do not point at the entire fn block span, point at the return type
// span, as it is the cause of the requirement, and
// `consider_hint_about_removing_semicolon` will point at the last expression
// if it were a relevant part of the error. This improves usability in editors
// that highlight errors inline.
let mut sp = blk.span;
let mut fn_span = None;
if let Some((decl, ident)) = self.get_parent_fn_decl(blk.id) {
let ret_sp = decl.output.span();
if let Some(block_sp) = self.parent_item_span(blk.id) {
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
// output would otherwise be incorrect and even misleading. Make sure
// the span we're aiming at correspond to a `fn` body.
if block_sp == blk.span {
sp = ret_sp;
fn_span = Some(ident.span);
}
}
}
coerce.coerce_forced_unit(self, &self.misc(sp), &mut |err| {
if let Some(expected_ty) = expected.only_has_type(self) {
self.consider_hint_about_removing_semicolon(blk,
expected_ty,
err);
self.consider_hint_about_removing_semicolon(blk, expected_ty, err);
}
if let Some(fn_span) = fn_span {
err.span_label(fn_span, "this function's body doesn't return");
}
}, false);
}
Expand All @@ -4820,59 +4840,81 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
ty
}

/// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether a
/// suggestion can be made, `None` otherwise.
pub fn get_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, bool)> {
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
// `while` before reaching it, as block tail returns are not available in them.
if let Some(fn_id) = self.tcx.hir().get_return_block(blk_id) {
let parent = self.tcx.hir().get(fn_id);
fn parent_item_span(&self, id: ast::NodeId) -> Option<Span> {
let node = self.tcx.hir().get(self.tcx.hir().get_parent(id));
match node {
Node::Item(&hir::Item {
node: hir::ItemKind::Fn(_, _, _, body_id), ..
}) |
Node::ImplItem(&hir::ImplItem {
node: hir::ImplItemKind::Method(_, body_id), ..
}) => {
let body = self.tcx.hir().body(body_id);
if let ExprKind::Block(block, _) = &body.value.node {
return Some(block.span);
}
}
_ => {}
}
None
}

if let Node::Item(&hir::Item {
/// Given a function block's `NodeId`, return its `FnDecl` if it exists, or `None` otherwise.
fn get_parent_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, ast::Ident)> {
let parent = self.tcx.hir().get(self.tcx.hir().get_parent(blk_id));
self.get_node_fn_decl(parent).map(|(fn_decl, ident, _)| (fn_decl, ident))
}

/// Given a function `Node`, return its `FnDecl` if it exists, or `None` otherwise.
fn get_node_fn_decl(&self, node: Node) -> Option<(hir::FnDecl, ast::Ident, bool)> {
match node {
Node::Item(&hir::Item {
ident, node: hir::ItemKind::Fn(ref decl, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
// This is less than ideal, it will not suggest a return type span on any
// method called `main`, regardless of whether it is actually the entry point,
// but it will still present it as the reason for the expected type.
Some((decl, ident.name != Symbol::intern("main")))
})
} else if let Node::TraitItem(&hir::TraitItem {
node: hir::TraitItemKind::Method(hir::MethodSig {
}) => decl.clone().and_then(|decl| {
// This is less than ideal, it will not suggest a return type span on any
// method called `main`, regardless of whether it is actually the entry point,
// but it will still present it as the reason for the expected type.
Some((decl, ident, ident.name != Symbol::intern("main")))
}),
Node::TraitItem(&hir::TraitItem {
ident, node: hir::TraitItemKind::Method(hir::MethodSig {
ref decl, ..
}, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
Some((decl, true))
})
} else if let Node::ImplItem(&hir::ImplItem {
node: hir::ImplItemKind::Method(hir::MethodSig {
}) => decl.clone().and_then(|decl| Some((decl, ident, true))),
Node::ImplItem(&hir::ImplItem {
ident, node: hir::ImplItemKind::Method(hir::MethodSig {
ref decl, ..
}, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
Some((decl, false))
})
} else {
None
}
} else {
None
}) => decl.clone().and_then(|decl| Some((decl, ident, false))),
_ => None,
}
}

/// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether a
/// suggestion can be made, `None` otherwise.
pub fn get_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, bool)> {
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
// `while` before reaching it, as block tail returns are not available in them.
self.tcx.hir().get_return_block(blk_id).and_then(|blk_id| {
let parent = self.tcx.hir().get(blk_id);
self.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
})
}

/// On implicit return expressions with mismatched types, provide the following suggestions:
///
/// - Point out the method's return type as the reason for the expected type
/// - Possible missing semicolon
/// - Possible missing return type if the return type is the default, and not `fn main()`
pub fn suggest_mismatched_types_on_tail(&self,
err: &mut DiagnosticBuilder<'tcx>,
expression: &'gcx hir::Expr,
expected: Ty<'tcx>,
found: Ty<'tcx>,
cause_span: Span,
blk_id: ast::NodeId) {
pub fn suggest_mismatched_types_on_tail(
&self,
err: &mut DiagnosticBuilder<'tcx>,
expression: &'gcx hir::Expr,
expected: Ty<'tcx>,
found: Ty<'tcx>,
cause_span: Span,
blk_id: ast::NodeId,
) {
self.suggest_missing_semicolon(err, expression, expected, cause_span);
if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest);
Expand Down
32 changes: 16 additions & 16 deletions src/test/ui/block-result/consider-removing-last-semi.stderr
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
error[E0308]: mismatched types
--> $DIR/consider-removing-last-semi.rs:1:18
--> $DIR/consider-removing-last-semi.rs:1:11
|
LL | fn f() -> String { //~ ERROR mismatched types
| __________________^
LL | | 0u8;
LL | | "bla".to_string();
| | - help: consider removing this semicolon
LL | | }
| |_^ expected struct `std::string::String`, found ()
LL | fn f() -> String { //~ ERROR mismatched types
| - ^^^^^^ expected struct `std::string::String`, found ()
| |
| this function's body doesn't return
LL | 0u8;
LL | "bla".to_string();
| - help: consider removing this semicolon
|
= note: expected type `std::string::String`
found type `()`

error[E0308]: mismatched types
--> $DIR/consider-removing-last-semi.rs:6:18
--> $DIR/consider-removing-last-semi.rs:6:11
|
LL | fn g() -> String { //~ ERROR mismatched types
| __________________^
LL | | "this won't work".to_string();
LL | | "removeme".to_string();
| | - help: consider removing this semicolon
LL | | }
| |_^ expected struct `std::string::String`, found ()
LL | fn g() -> String { //~ ERROR mismatched types
| - ^^^^^^ expected struct `std::string::String`, found ()
| |
| this function's body doesn't return
LL | "this won't work".to_string();
LL | "removeme".to_string();
| - help: consider removing this semicolon
|
= note: expected type `std::string::String`
found type `()`
Expand Down
17 changes: 8 additions & 9 deletions src/test/ui/block-result/issue-11714.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
error[E0308]: mismatched types
--> $DIR/issue-11714.rs:1:18
--> $DIR/issue-11714.rs:1:14
|
LL | fn blah() -> i32 { //~ ERROR mismatched types
| __________________^
LL | | 1
LL | |
LL | | ;
| | - help: consider removing this semicolon
LL | | }
| |_^ expected i32, found ()
LL | fn blah() -> i32 { //~ ERROR mismatched types
| ---- ^^^ expected i32, found ()
| |
| this function's body doesn't return
...
LL | ;
| - help: consider removing this semicolon
|
= note: expected type `i32`
found type `()`
Expand Down
35 changes: 16 additions & 19 deletions src/test/ui/block-result/issue-13428.stderr
Original file line number Diff line number Diff line change
@@ -1,30 +1,27 @@
error[E0308]: mismatched types
--> $DIR/issue-13428.rs:3:20
--> $DIR/issue-13428.rs:3:13
|
LL | fn foo() -> String { //~ ERROR mismatched types
| ____________________^
LL | | format!("Hello {}",
LL | | "world")
LL | | // Put the trailing semicolon on its own line to test that the
LL | | // note message gets the offending semicolon exactly
LL | | ;
| | - help: consider removing this semicolon
LL | | }
| |_^ expected struct `std::string::String`, found ()
LL | fn foo() -> String { //~ ERROR mismatched types
| --- ^^^^^^ expected struct `std::string::String`, found ()
| |
| this function's body doesn't return
...
LL | ;
| - help: consider removing this semicolon
|
= note: expected type `std::string::String`
found type `()`

error[E0308]: mismatched types
--> $DIR/issue-13428.rs:11:20
--> $DIR/issue-13428.rs:11:13
|
LL | fn bar() -> String { //~ ERROR mismatched types
| ____________________^
LL | | "foobar".to_string()
LL | | ;
| | - help: consider removing this semicolon
LL | | }
| |_^ expected struct `std::string::String`, found ()
LL | fn bar() -> String { //~ ERROR mismatched types
| --- ^^^^^^ expected struct `std::string::String`, found ()
| |
| this function's body doesn't return
LL | "foobar".to_string()
LL | ;
| - help: consider removing this semicolon
|
= note: expected type `std::string::String`
found type `()`
Expand Down
28 changes: 14 additions & 14 deletions src/test/ui/coercion/coercion-missing-tail-expected-type.stderr
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
error[E0308]: mismatched types
--> $DIR/coercion-missing-tail-expected-type.rs:3:28
--> $DIR/coercion-missing-tail-expected-type.rs:3:24
|
LL | fn plus_one(x: i32) -> i32 { //~ ERROR mismatched types
| ____________________________^
LL | | x + 1;
| | - help: consider removing this semicolon
LL | | }
| |_^ expected i32, found ()
LL | fn plus_one(x: i32) -> i32 { //~ ERROR mismatched types
| -------- ^^^ expected i32, found ()
| |
| this function's body doesn't return
LL | x + 1;
| - help: consider removing this semicolon
|
= note: expected type `i32`
found type `()`

error[E0308]: mismatched types
--> $DIR/coercion-missing-tail-expected-type.rs:7:29
--> $DIR/coercion-missing-tail-expected-type.rs:7:13
|
LL | fn foo() -> Result<u8, u64> { //~ ERROR mismatched types
| _____________________________^
LL | | Ok(1);
| | - help: consider removing this semicolon
LL | | }
| |_^ expected enum `std::result::Result`, found ()
LL | fn foo() -> Result<u8, u64> { //~ ERROR mismatched types
| --- ^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found ()
| |
| this function's body doesn't return
LL | Ok(1);
| - help: consider removing this semicolon
|
= note: expected type `std::result::Result<u8, u64>`
found type `()`
Expand Down
6 changes: 4 additions & 2 deletions src/test/ui/issues/issue-32323.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0308]: mismatched types
--> $DIR/issue-32323.rs:5:49
--> $DIR/issue-32323.rs:5:30
|
LL | pub fn f<'a, T: Tr<'a>>() -> <T as Tr<'a>>::Out {}
| ^^ expected associated type, found ()
| - ^^^^^^^^^^^^^^^^^^ expected associated type, found ()
| |
| this function's body doesn't return
|
= note: expected type `<T as Tr<'a>>::Out`
found type `()`
Expand Down
16 changes: 8 additions & 8 deletions src/test/ui/issues/issue-43162.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ LL | break {}; //~ ERROR E0268
| ^^^^^^^^ cannot break outside of a loop

error[E0308]: mismatched types
--> $DIR/issue-43162.rs:1:18
--> $DIR/issue-43162.rs:1:13
|
LL | fn foo() -> bool {
| __________________^
LL | | //~^ ERROR E0308
LL | | break true; //~ ERROR E0268
| | - help: consider removing this semicolon
LL | | }
| |_^ expected bool, found ()
LL | fn foo() -> bool {
| --- ^^^^ expected bool, found ()
| |
| this function's body doesn't return
LL | //~^ ERROR E0308
LL | break true; //~ ERROR E0268
| - help: consider removing this semicolon
|
= note: expected type `bool`
found type `()`
Expand Down
10 changes: 5 additions & 5 deletions src/test/ui/issues/issue-44023.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0308]: mismatched types
--> $DIR/issue-44023.rs:5:42
--> $DIR/issue-44023.rs:5:36
|
LL | fn საჭმელად_გემრიელი_სადილი ( ) -> isize { //~ ERROR mismatched types
| __________________________________________^
LL | | }
| |_^ expected isize, found ()
LL | fn საჭმელად_გემრიელი_სადილი ( ) -> isize { //~ ERROR mismatched types
| ------------------------ ^^^^^ expected isize, found ()
| |
| this function's body doesn't return
|
= note: expected type `isize`
found type `()`
Expand Down
Loading