Skip to content

Commit

Permalink
Auto merge of #43484 - estebank:point-to-return, r=arielb1
Browse files Browse the repository at this point in the history
Point at return type always when type mismatch against it

Before this, the diagnostic errors would only point at the return type
when changing it would be a possible solution to a type error. Add a
label to the return type without a suggestion to change in order to make
the source of the expected type obvious.

Follow up to #42850, fixes #25133, fixes #41897.
  • Loading branch information
bors committed Aug 9, 2017
2 parents 33d7194 + 9bd62a4 commit f142499
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ impl<'tcx> TyS<'tcx> {
TypeVariants::TyFnPtr(..) |
TypeVariants::TyDynamic(..) |
TypeVariants::TyClosure(..) |
TypeVariants::TyInfer(..) |
TypeVariants::TyProjection(..) => false,
_ => true,
}
Expand Down
61 changes: 42 additions & 19 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4229,8 +4229,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
ty
}

/// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether it is
/// `fn main` if it is a method, `None` otherwise.
/// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether a
/// suggetion 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.
Expand All @@ -4241,14 +4241,23 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
name, node: hir::ItemFn(ref decl, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
// This is less than ideal, it will not present the return type span on any
// method called `main`, regardless of whether it is actually the entry point.
Some((decl, name == Symbol::intern("main")))
// 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, name != Symbol::intern("main")))
})
} else if let Node::NodeTraitItem(&hir::TraitItem {
node: hir::TraitItemKind::Method(hir::MethodSig {
ref decl, ..
}, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
Some((decl, true))
})
} else if let Node::NodeImplItem(&hir::ImplItem {
node: hir::ImplItemKind::Method(hir::MethodSig {
ref decl, ..
}, ..), ..
}) = parent {
decl.clone().and_then(|decl| {
Some((decl, false))
Expand All @@ -4275,11 +4284,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
blk_id: ast::NodeId) {
self.suggest_missing_semicolon(err, expression, expected, cause_span);

if let Some((fn_decl, is_main)) = self.get_fn_decl(blk_id) {
// `fn main()` must return `()`, do not suggest changing return type
if !is_main {
self.suggest_missing_return_type(err, &fn_decl, found);
}
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 Expand Up @@ -4335,20 +4341,37 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
fn suggest_missing_return_type(&self,
err: &mut DiagnosticBuilder<'tcx>,
fn_decl: &hir::FnDecl,
ty: Ty<'tcx>) {

// Only recommend changing the return type for methods that
expected: Ty<'tcx>,
found: Ty<'tcx>,
can_suggest: bool) {
// Only suggest changing the return type for methods that
// haven't set a return type at all (and aren't `fn main()` or an impl).
if let &hir::FnDecl {
output: hir::FunctionRetTy::DefaultReturn(span), ..
} = fn_decl {
if ty.is_suggestable() {
match (&fn_decl.output, found.is_suggestable(), can_suggest) {
(&hir::FunctionRetTy::DefaultReturn(span), true, true) => {
err.span_suggestion(span,
"try adding a return type",
format!("-> {} ", ty));
} else {
format!("-> {} ", found));
}
(&hir::FunctionRetTy::DefaultReturn(span), false, true) => {
err.span_label(span, "possibly return type missing here?");
}
(&hir::FunctionRetTy::DefaultReturn(span), _, _) => {
// `fn main()` must return `()`, do not suggest changing return type
err.span_label(span, "expected `()` because of default return type");
}
(&hir::FunctionRetTy::Return(ref ty), _, _) => {
// Only point to return type if the expected type is the return type, as if they
// are not, the expectation must have been caused by something else.
debug!("suggest_missing_return_type: return type {:?} node {:?}", ty, ty.node);
let sp = ty.span;
let ty = AstConv::ast_ty_to_ty(self, ty);
debug!("suggest_missing_return_type: return type sty {:?}", ty.sty);
debug!("suggest_missing_return_type: expected type sty {:?}", ty.sty);
if ty.sty == expected.sty {
err.span_label(sp, format!("expected `{}` because of return type",
expected));
}
}
}
}

Expand Down
12 changes: 5 additions & 7 deletions src/test/compile-fail/struct-path-self-type-mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,15 @@ impl Bar for Foo<i32> {
}

impl<T> Foo<T> {
fn new<U>(u: U) -> Foo<U> {
fn new<U>(u: U) -> Foo<U> { //~ NOTE expected `Foo<U>` because of return type
Self {
//~^ ERROR mismatched types
//~| expected type parameter, found a different type parameter
//~| expected type `Foo<U>`
//~| found type `Foo<T>`
//~| NOTE expected type parameter, found a different type parameter
//~| NOTE expected type `Foo<U>`
inner: u
//~^ ERROR mismatched types
//~| expected type parameter, found a different type parameter
//~| expected type `T`
//~| found type `U`
//~| NOTE expected type parameter, found a different type parameter
//~| NOTE expected type `T`
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0308]: mismatched types
--> $DIR/block-must-not-have-result-res.rs:15:9
|
14 | fn drop(&mut self) {
| - expected `()` because of default return type
15 | true //~ ERROR mismatched types
| ^^^^ expected (), found bool
|
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/block-result/issue-13624.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0308]: mismatched types
--> $DIR/issue-13624.rs:17:5
|
16 | pub fn get_enum_struct_variant() -> () {
| -- expected `()` because of return type
17 | Enum::EnumStructVariant { x: 1, y: 2, z: 3 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found enum `a::Enum`
|
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/block-result/issue-22645.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ error[E0277]: the trait bound `{integer}: Scalar` is not satisfied
error[E0308]: mismatched types
--> $DIR/issue-22645.rs:25:3
|
23 | fn main() {
| - expected `()` because of default return type
24 | let b = Bob + 3.5;
25 | b + 3 //~ ERROR E0277
| ^^^^^ expected (), found struct `Bob`
|
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/block-result/issue-5500.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0308]: mismatched types
--> $DIR/issue-5500.rs:12:5
|
11 | fn main() {
| - expected `()` because of default return type
12 | &panic!()
| ^^^^^^^^^ expected (), found reference
|
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/mismatched_types/abridged.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0308]: mismatched types
--> $DIR/abridged.rs:26:5
|
25 | fn a() -> Foo {
| --- expected `Foo` because of return type
26 | Some(Foo { bar: 1 })
| ^^^^^^^^^^^^^^^^^^^^ expected struct `Foo`, found enum `std::option::Option`
|
Expand All @@ -10,6 +12,8 @@ error[E0308]: mismatched types
error[E0308]: mismatched types
--> $DIR/abridged.rs:30:5
|
29 | fn a2() -> Foo {
| --- expected `Foo` because of return type
30 | Ok(Foo { bar: 1})
| ^^^^^^^^^^^^^^^^^ expected struct `Foo`, found enum `std::result::Result`
|
Expand All @@ -19,6 +23,8 @@ error[E0308]: mismatched types
error[E0308]: mismatched types
--> $DIR/abridged.rs:34:5
|
33 | fn b() -> Option<Foo> {
| ----------- expected `std::option::Option<Foo>` because of return type
34 | Foo { bar: 1 }
| ^^^^^^^^^^^^^^ expected enum `std::option::Option`, found struct `Foo`
|
Expand All @@ -28,6 +34,8 @@ error[E0308]: mismatched types
error[E0308]: mismatched types
--> $DIR/abridged.rs:38:5
|
37 | fn c() -> Result<Foo, Bar> {
| ---------------- expected `std::result::Result<Foo, Bar>` because of return type
38 | Foo { bar: 1 }
| ^^^^^^^^^^^^^^ expected enum `std::result::Result`, found struct `Foo`
|
Expand All @@ -37,6 +45,9 @@ error[E0308]: mismatched types
error[E0308]: mismatched types
--> $DIR/abridged.rs:49:5
|
41 | fn d() -> X<X<String, String>, String> {
| ---------------------------- expected `X<X<std::string::String, std::string::String>, std::string::String>` because of return type
...
49 | x
| ^ expected struct `std::string::String`, found integral variable
|
Expand All @@ -46,6 +57,9 @@ error[E0308]: mismatched types
error[E0308]: mismatched types
--> $DIR/abridged.rs:60:5
|
52 | fn e() -> X<X<String, String>, String> {
| ---------------------------- expected `X<X<std::string::String, std::string::String>, std::string::String>` because of return type
...
60 | x
| ^ expected struct `std::string::String`, found integral variable
|
Expand Down

0 comments on commit f142499

Please sign in to comment.