Point at method call when type annotations are needed #65951

estebank · 2019-10-30T00:47:11Z

Point at method call instead of whole expression when type annotations are needed.
Suggest use of turbofish on function and methods.

Fix #49391, fix #46333, fix #48089. CC #58517, #63502, #63082.

Fixes #40015

r? @nikomatsakis

src/test/ui/issues/issue-58022.stderr

src/test/ui/associated-types/associated-types-unconstrained.stderr

nikomatsakis

WDYT @estebank?

src/test/ui/issues/issue-65611.stderr

src/test/ui/question-mark-type-infer.stderr

src/librustc/infer/error_reporting/need_type_info.rs

src/test/ui/type/type-annotation-needed.stderr

nikomatsakis · 2019-12-05T22:07:08Z

@estebank are you blocked here looking for advice?

src/test/ui/issues/issue-24424.stderr

src/test/ui/issues/issue-21974.stderr

src/librustc/infer/error_reporting/need_type_info.rs

src/librustc_typeck/check/expr.rs

bors · 2019-12-11T07:14:05Z

☀️ Try build successful - checks-azure
Build commit: f6cbf0379bcde7c56241c313eafb438da272e069 (f6cbf0379bcde7c56241c313eafb438da272e069)

rust-timer · 2019-12-11T07:14:07Z

Queued f6cbf0379bcde7c56241c313eafb438da272e069 with parent 7dbfb0a, future comparison URL.

rust-timer · 2019-12-11T17:27:47Z

Finished benchmarking try commit f6cbf0379bcde7c56241c313eafb438da272e069, comparison URL.

estebank · 2019-12-11T17:58:27Z

I don't know how noisy the Max-RSS metrics are, but the worst impact was ~16%, but it seems inside of the noise bands (given the ~12% improvement elsewhere).

nikomatsakis

OK, this is looking pretty good, I don't think we need the new table though. I left some ideas for improvements but I don't think they're really necessary. I'd be game to r+ if we can remove the extra table especially.

src/librustc_typeck/check/expr.rs

nikomatsakis · 2019-12-11T21:47:14Z

src/test/ui/question-mark-type-infer.stderr

+   |                     ^^^^^^^
+   |                     |
+   |                     cannot infer type
+   |                     help: consider specifying the type argument in the method call: `collect::<B>`


I'm a bit unsure about introducing the letter B here. It could be that the name happens to shadow a name that's already in scope for the user, which might be confusing. I wonder if we could write something like collect::<_> or is that potentially more confusing? (Especially as it would not be especially helpful if they were to write that exact thing, though the same applies to B.)

I think the only way to improve on this is not to use _ but rather to attempt to give a reasonable type that could fit there, but that would be out of scope for this PR. My main objective here is to 1) bring attention to the fact that this method has a type argument and 2) what the turbofish looks like.

nikomatsakis · 2019-12-11T21:47:59Z

src/test/ui/type-inference/sort_by_key.stderr

+LL |     lst.sort_by_key(|&(v, _)| v.iter().sum());
+   |         ^^^^^^^^^^^                    --- help: consider specifying the type argument in the method call: `sum::<S>`
+   |         |
+   |         cannot infer type for `K`


This is pre-existing, but I wonder if we could say

LL | lst.sort_by_key(|&(v, _)| v.iter().sum()); | ^^^^^^^^^^^ --- help: consider specifying the type argument in the method call: `sum::<S>` | | | cannot infer type for type parameter `K` declared on `sort_by_key`

or something like that?

I know, it's kind of tricky to do it right at the moment, but the slight rewording could be tackled (mentioning sort_by_key might be much harder)

nikomatsakis · 2019-12-11T21:50:38Z

src/librustc/infer/error_reporting/need_type_info.rs

+            let method_defs = borrow.node_method_def_id();
+            if let Some(did) = method_defs.get(e.hir_id) {
+                let generics = self.tcx.generics_of(*did);
+                if !generics.params.is_empty() {


So I was debating if this condition is sufficient. I decided it probably is, though we could also choose to be more selective.

One thing is that there won't always be a direct connection between the type parameters and the return type. e.g. it might be

fn foo<T: Iterator>() -> T::Item

but, then again, it's still true that specifying T would help to figure out the Item type.

Another is that there could be many parameters.

I could imagine searching through the substitutions for the call to detect cases (like collect) where the return type is directly linked to a parameter and limiting to that case, or highlighting which parameter specifies the return type in the case that there are many (e.g., maybe writing "try writing out the return type X, as in foo.bar::<_, _, X>()"

But it's probably "good enough" the way it is, tbh, those seem like "potential improvements". (And the best phrasing is sort of unclear anyway.)

One thing I did wonder is if we want to be careful around "defaults", though they are currently being phased out on functions:

fn foo<T, U = u64>()...

you could imagine not showing U in our suggestions. But nobody quite knows what the semantics of said defaults should be so it's probably "ok as is".

error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions. --> file12.rs:1:11 | 1 | fn foo<T, U = u64>() -> T { | ^ | = note: `#[deny(invalid_type_param_default)]` on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #36887 <https://github.com/rust-lang/rust/issues/36887>

with #[allow(invalid_type_param_default)] we do something more reasonable already IMO:

error[E0282]: type annotations needed --> file12.rs:8:13 | 8 | let _ = foo(); | - ^^^ cannot infer type for `T` | | | consider giving this pattern a type

For the Iterator::Item case, the output is not ideal, but not any worse than it already is:

error[E0282]: type annotations needed --> file12.rs:8:13 | 8 | let _ = foo(); | ^^^ cannot infer type for `T`

error[E0284]: type annotations needed --> file12.rs:13:15 | 13 | let _ = S.foo(); | ^^^ cannot infer type for `T` | = note: cannot resolve `<_ as std::iter::Iterator>::Item == _`

I'm inclined at leaving things as they are for now.

The last case is because we have a visitor looking at the method calls for something that resolves to T, but what we should be looking is a method call that resolves to <T as std::iter::Iterator>::Item instead (for this case).

Another interesting case:

trait T { type A; fn foo(&self) -> Self::A { panic!() } } struct S<X>(std::marker::PhantomData<X>); impl<X> T for S<X> { type A = X; } fn main() { S(std::marker::PhantomData).foo(); }

error[E0282]: type annotations needed --> file12.rs:14:5 | 2 | type A; | ------- `<Self as T>::A` defined here ... 14 | S(std::marker::PhantomData).foo(); | ^-------------------------------- | | | this method call resolves to `<Self as T>::A` | cannot infer type for type parameter `X`

estebank · 2019-12-12T01:37:48Z

@nikomatsakis I would call this ready to be merged.

nikomatsakis · 2019-12-12T15:20:06Z

src/test/ui/async-await/unresolved_type_param.rs

@@ -8,7 +8,7 @@ async fn bar<T>() -> () {}
 async fn foo() {
    bar().await;
    //~^ ERROR type inside `async fn` body must be known in this context
-    //~| NOTE cannot infer type for `T`
+    //~| NOTE cannot infer type for type parameter `T`


@estebank do you think it's possible/desirable for us to say "type parameter T declared on bar" or something like that?

I think it is desirable, but not trivial to accomplish at the moment. The only way I can see of relating a type param and where it came from is with judicious use of a visitor, which I believe will require enough tweaking that would push this PR to not land in the next beta. I can certainly follow up on that. Note that neither this change or that one will address the cases where we say "cannot infer type 🤷" and nothing else.

nikomatsakis

I'm not going to block this PR over this, because I think it's an orthogonal issue, but I think this is how we would make the change I am talking about.

nikomatsakis · 2019-12-13T13:26:50Z

src/librustc/infer/error_reporting/need_type_info.rs

 impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
    pub fn extract_type_name(
        &self,
        ty: Ty<'tcx>,
        highlight: Option<ty::print::RegionHighlightMode>,
-    ) -> (String, Option<Span>) {
+    ) -> (String, Option<Span>, Cow<'static, str>) {
        if let ty::Infer(ty::TyVar(ty_vid)) = ty.kind {
            let ty_vars = self.type_variables.borrow();
            let var_origin = ty_vars.var_origin(ty_vid);
            if let TypeVariableOriginKind::TypeParameterDefinition(name) = var_origin.kind {


I think @estebank if we wanted to say more about where the type parameter came from, we would augment this enum with the DefId of the parameter instead of its name (I imagine we can extract the name -- as well as the context -- from that DefId)

Filed #67277 to track this

nikomatsakis · 2019-12-13T13:27:51Z

@bors r+

bors · 2019-12-13T13:27:54Z

📌 Commit da023c0 has been approved by nikomatsakis

bors · 2019-12-13T22:56:33Z

⌛ Testing commit da023c0 with merge 8843b28...

@nikomatsakis

Point at method call when type annotations are needed - Point at method call instead of whole expression when type annotations are needed. - Suggest use of turbofish on function and methods. Fix #49391, fix #46333, fix #48089. CC #58517, #63502, #63082. Fixes #40015 r? @nikomatsakis

bors · 2019-12-14T02:08:25Z

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing 8843b28 to master...

@estebank

…type-parameter, r=estebank Indicate origin of where type parameter for uninferred types Based on rust-lang#65951 (which is not merge yet), fixes rust-lang#67277. This PR improves a little the diagnostic for code like: ``` async fn foo() { bar().await; } async fn bar<T>() -> () {} ``` by showing: ``` error[E0698]: type inside `async fn` body must be known in this context --> unresolved_type_param.rs:9:5 | 9 | bar().await; | ^^^ cannot infer type for type parameter `T` declared on the function `bar` | ... ``` (The ``` declared on the function `bar` ``` part is new) A small side note: `Vec` and `slice` seem to resist this change, because querying `item_name()` panics, and `get_opt_name()` returns `None`. r? @estebank

@estebank

…type-parameter, r=estebank Indicate origin of where type parameter for uninferred types Based on rust-lang#65951 (which is not merge yet), fixes rust-lang#67277. This PR improves a little the diagnostic for code like: ``` async fn foo() { bar().await; } async fn bar<T>() -> () {} ``` by showing: ``` error[E0698]: type inside `async fn` body must be known in this context --> unresolved_type_param.rs:9:5 | 9 | bar().await; | ^^^ cannot infer type for type parameter `T` declared on the function `bar` | ... ``` (The ``` declared on the function `bar` ``` part is new) A small side note: `Vec` and `slice` seem to resist this change, because querying `item_name()` panics, and `get_opt_name()` returns `None`. r? @estebank

@estebank

…type-parameter, r=estebank Indicate origin of where type parameter for uninferred types Based on rust-lang#65951 (which is not merge yet), fixes rust-lang#67277. This PR improves a little the diagnostic for code like: ``` async fn foo() { bar().await; } async fn bar<T>() -> () {} ``` by showing: ``` error[E0698]: type inside `async fn` body must be known in this context --> unresolved_type_param.rs:9:5 | 9 | bar().await; | ^^^ cannot infer type for type parameter `T` declared on the function `bar` | ... ``` (The ``` declared on the function `bar` ``` part is new) A small side note: `Vec` and `slice` seem to resist this change, because querying `item_name()` panics, and `get_opt_name()` returns `None`. r? @estebank

@estebank

…type-parameter, r=estebank Indicate origin of where type parameter for uninferred types Based on rust-lang#65951 (which is not merge yet), fixes rust-lang#67277. This PR improves a little the diagnostic for code like: ``` async fn foo() { bar().await; } async fn bar<T>() -> () {} ``` by showing: ``` error[E0698]: type inside `async fn` body must be known in this context --> unresolved_type_param.rs:9:5 | 9 | bar().await; | ^^^ cannot infer type for type parameter `T` declared on the function `bar` | ... ``` (The ``` declared on the function `bar` ``` part is new) A small side note: `Vec` and `slice` seem to resist this change, because querying `item_name()` panics, and `get_opt_name()` returns `None`. r? @estebank

rust-highfive assigned nikomatsakis Oct 30, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 30, 2019

estebank commented Oct 30, 2019

View reviewed changes

src/test/ui/issues/issue-58022.stderr Show resolved Hide resolved

estebank commented Oct 30, 2019

View reviewed changes

src/test/ui/associated-types/associated-types-unconstrained.stderr Show resolved Hide resolved

This comment has been minimized.

Sign in to view

estebank force-pushed the type-inference-error branch from abb0f56 to 11b772d Compare October 31, 2019 17:19

nikomatsakis requested changes Nov 8, 2019

View reviewed changes

estebank commented Nov 9, 2019

View reviewed changes

src/test/ui/type/type-annotation-needed.stderr Show resolved Hide resolved

estebank force-pushed the type-inference-error branch from 11b772d to f468ef3 Compare November 16, 2019 20:39

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 24, 2019

This comment has been minimized.

Sign in to view

estebank commented Dec 6, 2019

View reviewed changes

src/test/ui/issues/issue-24424.stderr Show resolved Hide resolved

estebank commented Dec 6, 2019

View reviewed changes

src/test/ui/issues/issue-21974.stderr Show resolved Hide resolved

estebank commented Dec 6, 2019

View reviewed changes

src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

estebank force-pushed the type-inference-error branch from f468ef3 to 9d6ad88 Compare December 10, 2019 17:01

estebank commented Dec 10, 2019

View reviewed changes

src/librustc/infer/error_reporting/need_type_info.rs Show resolved Hide resolved

estebank commented Dec 10, 2019

View reviewed changes

src/librustc_typeck/check/expr.rs Outdated Show resolved Hide resolved

estebank mentioned this pull request Dec 10, 2019

Type inference fails even though all types are known #58517

Open

This comment has been minimized.

Sign in to view

estebank mentioned this pull request Dec 10, 2019

Diagnostics doesn't provide proper explaination as to where type annotations should be added #48089

Closed

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 10, 2019

This comment has been minimized.

Sign in to view

estebank force-pushed the type-inference-error branch from 9d6ad88 to d8528d0 Compare December 10, 2019 18:22

nikomatsakis reviewed Dec 11, 2019

View reviewed changes

estebank added 2 commits December 11, 2019 15:33

Reuse existing HirId → DefId table

45550ef

Add more context for type parameters

da023c0

nikomatsakis reviewed Dec 12, 2019

View reviewed changes

nikomatsakis approved these changes Dec 13, 2019

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2019

nikomatsakis mentioned this pull request Dec 13, 2019

indicate origin of where type parameter for uninferred types #67277

Closed

ohadravid mentioned this pull request Dec 13, 2019

Indicate origin of where type parameter for uninferred types #67285

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 14, 2019

bors merged commit da023c0 into rust-lang:master Dec 14, 2019

This was referenced Dec 14, 2019

Allocate HIR on an arena 1/4 #66931

Merged

Allocate HIR on an arena 2/4 -- Expr & Pat #66936

Merged

Allocate HIR on an arena 3/4 -- Ty #66942

Merged

estebank deleted the type-inference-error branch November 9, 2023 05:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Point at method call when type annotations are needed #65951

Point at method call when type annotations are needed #65951

estebank commented Oct 30, 2019 •

edited by nikomatsakis

Loading

This comment has been minimized.

This comment has been minimized.

nikomatsakis left a comment

This comment has been minimized.

This comment has been minimized.

nikomatsakis commented Dec 5, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Dec 11, 2019

rust-timer commented Dec 11, 2019

rust-timer commented Dec 11, 2019

estebank commented Dec 11, 2019 •

edited

Loading

nikomatsakis left a comment

nikomatsakis Dec 11, 2019

estebank Dec 11, 2019

nikomatsakis Dec 11, 2019

estebank Dec 11, 2019

nikomatsakis Dec 11, 2019

nikomatsakis Dec 11, 2019

estebank Dec 11, 2019 •

edited

Loading

estebank Dec 11, 2019 •

edited

Loading

estebank commented Dec 12, 2019

nikomatsakis Dec 12, 2019

estebank Dec 12, 2019

nikomatsakis left a comment •

edited

Loading

nikomatsakis Dec 13, 2019

nikomatsakis Dec 13, 2019

nikomatsakis commented Dec 13, 2019

bors commented Dec 13, 2019

bors commented Dec 13, 2019

bors commented Dec 14, 2019

Point at method call when type annotations are needed #65951

Point at method call when type annotations are needed #65951

Conversation

estebank commented Oct 30, 2019 • edited by nikomatsakis Loading

This comment has been minimized.

This comment has been minimized.

nikomatsakis left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

nikomatsakis commented Dec 5, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Dec 11, 2019

rust-timer commented Dec 11, 2019

rust-timer commented Dec 11, 2019

estebank commented Dec 11, 2019 • edited Loading

nikomatsakis 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

Choose a reason for hiding this comment

estebank Dec 11, 2019 • edited Loading

Choose a reason for hiding this comment

estebank Dec 11, 2019 • edited Loading

Choose a reason for hiding this comment

estebank commented Dec 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Dec 13, 2019

bors commented Dec 13, 2019

bors commented Dec 13, 2019

bors commented Dec 14, 2019

estebank commented Oct 30, 2019 •

edited by nikomatsakis

Loading

estebank commented Dec 11, 2019 •

edited

Loading

estebank Dec 11, 2019 •

edited

Loading

estebank Dec 11, 2019 •

edited

Loading

nikomatsakis left a comment •

edited

Loading