rustc_typeck: improve diagnostics for -> _ fn return type by lundibundi · Pull Request #62694 · rust-lang/rust

lundibundi · 2019-07-15T16:41:53Z

This should implement IIUC the mentioned issue.

~~I'm not sure if there is a better way than get_infer_ret_ty to get/check the return type without code duplication.~~

~~Also, is this unwrap be okay ty::Binder::bind(*tables.liberated_fn_sigs().get(hir_id).unwrap())?~~

r? @eddyb
Closes: #56132

eddyb · 2019-07-16T13:19:01Z

Heh, you could also implement it for const/static with type _.

We could also add parser recovery for when no type is provided syntactically, for example: const FOO = 42;. I think more people would attempt that. But this could be done in a follow up.

Help for const FOO = 42; looks nice, that's how I usually forget about it in const's, though I'm not sure what'd be the correct parser recovery, replacing the type with Infer and then failing here with proper help?

Yes, basically delegate to the AST representation of const $name: _ = $expr; and make sure to error in the parser.

@Centril But would we have the correct type to suggest in the parser? (I assume you implied the type of self.parse_expr() on the right side of const declaration)

The correct type in the parser would be Infer.

But then the suggestion for const FOO = 42 would be the same as for const FOO: _ = 42 - to replace the _ even though the user hasn't written it, could we perhaps denote some sort of type-absence to handle that properly? Or am I misunderstanding something?

We can skip this for now, tbh.

Yeah, let's follow up later.
@lundibundi We should take care not to suggest replacing _ with something since the user did not write that. Instead we should indicate that the type is missing and that we think the type is so and so.

eddyb

This looks great! I only had minor nits.

eddyb · 2019-07-16T13:21:59Z

Hmm, you could also get the return type from here, instead of from the body. Not sure which is better, cc @matthewjasper @nikomatsakis

I think it would be at least more 'correct' since we are suggesting the return type of this function and not the 'body' and since we already have it might as well use it.
Edit: Pushed as a separate commit.

Closes: rust-lang#56132

lundibundi · 2019-07-17T14:02:09Z

Merged fixups and rebased on master as there were quite many fixups! and GitHub displayed the commits in the wrong order anyway.

eddyb · 2019-07-18T16:19:43Z

@bors r+

bors · 2019-07-18T16:19:44Z

📌 Commit f8681f0 has been approved by eddyb

bors · 2019-07-19T04:04:24Z

⌛ Testing commit f8681f0 with merge f9477a7...

@eddyb

rustc_typeck: improve diagnostics for -> _ fn return type This should implement IIUC the mentioned issue. ~~I'm not sure if there is a better way than `get_infer_ret_ty` to get/check the return type without code duplication.~~ ~~Also, is this unwrap be okay `ty::Binder::bind(*tables.liberated_fn_sigs().get(hir_id).unwrap())`?~~ r? @eddybCloses: #56132

bors · 2019-07-19T07:49:05Z

☀️ Test successful - checks-azure
Approved by: eddyb
Pushing f9477a7 to master...

@eddyb

… r=eddyb rustc_typeck: improve diagnostics for _ const/static declarations This continues rust-lang#62694 and adds type suggestions to const/static declarations with `_` type. r? @eddyb

@eddyb

… r=eddyb rustc_typeck: improve diagnostics for _ const/static declarations This continues rust-lang#62694 and adds type suggestions to const/static declarations with `_` type. r? @eddyb

@eddyb

… r=eddyb rustc_typeck: improve diagnostics for _ const/static declarations This continues rust-lang#62694 and adds type suggestions to const/static declarations with `_` type. r? @eddyb

rust-highfive assigned eddyb Jul 15, 2019

This comment has been minimized.
Sign in to view

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