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

estebank · 2017-07-26T05:08:34Z

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.

rust-highfive · 2017-07-26T05:08:38Z

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

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 rust-lang#42850, fixes rust-lang#25133, fixes rust-lang#41897.

estebank · 2017-07-26T05:13:47Z

r? @Mark-Simulacrum as this PR was prompted by this comment.

Mark-Simulacrum

Very happy to see this, thanks for working on it!

Mark-Simulacrum · 2017-07-26T12:07:58Z

src/test/ui/block-result/issue-13624.stderr

@@ -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`


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?

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

Mark-Simulacrum · 2017-07-26T12:09:34Z

src/test/ui/impl-trait/equality.stderr

@@ -1,6 +1,9 @@
 error[E0308]: mismatched types
  --> $DIR/equality.rs:25:5
   |
+21 | fn two(x: bool) -> impl Foo {
+   |                    -------- expected `_` because of return type


This note seems odd; presumably we expected i32? I'm not sure, but that seems to be the case here...

Removed all suggestions involving impl Trait.

aidanhs · 2017-08-03T18:38:42Z

@estebank have you had a chance to have a look at the review comments yet? Just want to make sure this doesn't get lost!

Mark-Simulacrum · 2017-08-03T18:41:30Z

Ah, this should also be r? @arielb1 or someone else, I'm not sufficiently familiar with these compiler internals.

estebank · 2017-08-04T20:34:15Z

I am worried about that the current approach isn't 100% accurate. Although this will point out the cause of the expected type most of the time, src/test/ui/impl-trait/equality.stderr shows a case where where type inference caused the expected type to be caused not by the fn's return type, but by the type inference's interaction with if branches of different types. I've come across other examples, but I'm failing to find them now.

I'm hesitant to merge this if it will ever be misleading, but making it work 100% of the time would mean start keeping track of the traceback for all inferred types, something that I've been thinking about doing (but has the potential to make the compiler use a bit more memory).

arielb1 · 2017-08-07T09:12:19Z

src/librustc/hir/map/mod.rs

@@ -664,7 +664,7 @@ impl<'hir> Map<'hir> {
            match *node {
                NodeExpr(ref expr) => {
                    match expr.node {
-                        ExprWhile(..) | ExprLoop(..) => true,
+                        ExprWhile(..) | ExprLoop(..) | ExprIf(..) => true,


This makes the comment above wrong. Why?

Reverted. I was considering removing all suggestions when involving if blocks due to the accuracy problem mentioned earlier. Given that now I only provide the suggestion if the types are the same, it can be safely reverted.

arielb1 · 2017-08-08T10:09:34Z

@estebank

About the expected _ because of return type case: the only time the return type can contain inference variables is because we have an impl Trait with intra-function conflicts. I think we should just disable the error message if the return type contains impl Trait, because that indicates a conflict (there are also cases such as the return type being Option<impl Trait> where we could try to separate conflicts from non-conflicts, so we could instead try to check whether our type can be coerced into the return type if we wanted. I'm fine with it either way).

arielb1 · 2017-08-09T09:36:47Z

src/librustc_typeck/check/mod.rs

+                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 ()


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 `()`

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

arielb1 · 2017-08-09T09:40:49Z

r=me with the backticks back.

estebank · 2017-08-09T19:12:25Z

@bors r=arielb1

bors · 2017-08-09T19:12:26Z

📌 Commit 9bd62a4 has been approved by arielb1

bors · 2017-08-09T19:50:09Z

⌛ Testing commit 9bd62a4 with merge f142499...

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.

bors · 2017-08-09T22:41:05Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing f142499 to master...

rust-highfive assigned arielb1 Jul 26, 2017

estebank force-pushed the point-to-return branch from d45dd95 to d96f9d4 Compare July 26, 2017 05:11

rust-highfive assigned Mark-Simulacrum and unassigned arielb1 Jul 26, 2017

Mark-Simulacrum reviewed Jul 26, 2017

View reviewed changes

alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 27, 2017

rust-highfive assigned arielb1 and unassigned Mark-Simulacrum Aug 3, 2017

arielb1 reviewed Aug 7, 2017

View reviewed changes

Only refer to return type when it matches

3fcdb8b

estebank force-pushed the point-to-return branch from bc5b98e to 3fcdb8b Compare August 9, 2017 06:00

arielb1 reviewed Aug 9, 2017

View reviewed changes

Readd backticks around ()

9bd62a4

estebank force-pushed the point-to-return branch from 48c681c to 9bd62a4 Compare August 9, 2017 17:48

bors merged commit 9bd62a4 into rust-lang:master Aug 9, 2017

estebank mentioned this pull request Sep 26, 2017

Incorrect "match arms have incompatible types" error message #24157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

estebank commented Jul 26, 2017

rust-highfive commented Jul 26, 2017

estebank commented Jul 26, 2017

Mark-Simulacrum left a comment

Mark-Simulacrum Jul 26, 2017

estebank Aug 9, 2017

estebank Aug 9, 2017

Mark-Simulacrum Jul 26, 2017

estebank Aug 9, 2017

aidanhs commented Aug 3, 2017

Mark-Simulacrum commented Aug 3, 2017

estebank commented Aug 4, 2017

arielb1 Aug 7, 2017 •

edited

Loading

estebank Aug 9, 2017

arielb1 commented Aug 8, 2017 •

edited

Loading

arielb1 Aug 9, 2017

arielb1 Aug 9, 2017

arielb1 commented Aug 9, 2017

estebank commented Aug 9, 2017

bors commented Aug 9, 2017

bors commented Aug 9, 2017

bors commented Aug 9, 2017

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

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

Conversation

estebank commented Jul 26, 2017

rust-highfive commented Jul 26, 2017

estebank commented Jul 26, 2017

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aidanhs commented Aug 3, 2017

Mark-Simulacrum commented Aug 3, 2017

estebank commented Aug 4, 2017

arielb1 Aug 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 commented Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 commented Aug 9, 2017

estebank commented Aug 9, 2017

bors commented Aug 9, 2017

bors commented Aug 9, 2017

bors commented Aug 9, 2017

arielb1 Aug 7, 2017 •

edited

Loading

arielb1 commented Aug 8, 2017 •

edited

Loading