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.

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

Point at return type always when type mismatch against it #43484

Merged
merged 3 commits into from
Aug 9, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ impl<'tcx> TyS<'tcx> {
TypeVariants::TyFnPtr(..) |
TypeVariants::TyDynamic(..) |
TypeVariants::TyClosure(..) |
TypeVariants::TyInfer(..) |
TypeVariants::TyProjection(..) => false,
_ => true,
}
Expand Down
74 changes: 54 additions & 20 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ use rustc::middle::region::CodeExtent;
use rustc::ty::subst::{Kind, Subst, Substs};
use rustc::traits::{self, FulfillmentContext, ObligationCause, ObligationCauseCode};
use rustc::ty::{ParamTy, LvaluePreference, NoPreference, PreferMutLvalue};
use rustc::ty::{self, Ty, TyCtxt, Visibility};
use rustc::ty::{self, Ty, TyCtxt, Visibility, TypeVariants};
use rustc::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
use rustc::ty::fold::{BottomUpFolder, TypeFoldable};
use rustc::ty::maps::Providers;
Expand Down Expand Up @@ -4187,8 +4187,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 @@ -4199,14 +4199,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 @@ -4233,11 +4242,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 @@ -4293,20 +4299,48 @@ 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 {
let quote = if let TypeVariants::TyTuple(ref slice, _) = expected.sty {
if slice.len() == 0 { // don't use backtics for ()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this?

In normal error messages:

fn main() {
    let x: u32 = ();
}

The backticks appear when mentioning the type name, but not when mentioning the type "constructor":


error[E0308]: mismatched types
 --> src/main.rs:2:18
  |
2 |     let x: u32 = ();
  |                  ^^ expected u32, found ()
  |
  = note: expected type `u32`
             found type `()`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you are going to change that error message too to not have backticks, I'll want to remove this check.

""
} else {
"`"
}
} else {
"`"
};
err.span_label(sp, format!("expected {}{}{} because of return type",
quote,
expected,
quote));
}
}
}
}

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`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it looks like in the new note we put the empty tuple in backticks, but don't in the old text. This seems odd; any chance it's an easy fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted to the original output at @arielb1's request.

|
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