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.

Sign up for GitHub

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

Jump to bottom

When needing type annotations in local bindings, account for impl Trait and closures #63507

Merged
merged 7 commits into from
Aug 15, 2019
Merged

When needing type annotations in local bindings, account for impl Trait and closures #63507

merged 7 commits into from
Aug 15, 2019

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Aug 13, 2019 •
edited
Loading

Fix #46680, fix #63504, fix #63506, fix #40014, cc #63502.

@estebank
When needing type annotations in local bindings, account for impl Tra…
b8708e2 
…it and closures

Do not suggest nonsensical types when the type inference is failing on
`impl Trait` or anonymous closures.
@estebank
Resolve types when suggesting boxed closure
3b6d46c
@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2019
@estebank
Copy link
Contributor Author

r? @Centril

@rust-highfive rust-highfive assigned Centril and unassigned eddyb Aug 13, 2019
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@estebank
Fix tidy
1e6f753
@estebank
Suggest Box::new when appropriate
fb2511c 
When encountering a boxed value as expected and a stack allocated value
that could be boxed to fulfill the expectation, like in the following
snippet, suggest `Box::new` wrapping.
Centril
Centril reviewed Aug 13, 2019
View reviewed changes
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved
src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr Show resolved Hide resolved
src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Aug 13, 2019

Thanks! r=me with ^--- addressed :)

This was referenced Aug 13, 2019
Closed
Open
@estebank estebank mentioned this pull request Aug 13, 2019
Closed
@estebank
Copy link
Contributor Author

@bors r=Centril

@bors
Copy link
Contributor

bors commented Aug 13, 2019

📌 Commit 939c1cb has been approved by Centril

@bors 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 Aug 13, 2019
@estebank
Copy link
Contributor Author

estebank commented Aug 13, 2019 •
edited
Loading

@Centril I have an extra commit that could either go on this PR or a follow up, changing the suggestion for specific cases of closures, recommending setting a return type instead of a boxed closure. Should I include it in this PR and you do an extra round of reviews? It's to close #40014 too.

@Centril
Copy link
Contributor

Centril commented Aug 13, 2019

@estebank I'm fine with either... ;)

@estebank
Copy link
Contributor Author

@bors r-

@Centril incoming.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 13, 2019
@estebank 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 Aug 14, 2019
Centril
Centril reviewed Aug 14, 2019
View reviewed changes
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
src/librustc/infer/error_reporting/need_type_info.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Aug 14, 2019

The code makes sense but could use a bit of refactoring so the method isn't too overwhelming to read. I've suggested some ways to fix that ^--- r=me with those addressed.

Centril
Centril approved these changes Aug 14, 2019
View reviewed changes
Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

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

Thanks! r=me :)

@estebank
Copy link
Contributor Author

@bors r=Centril

@bors
Copy link
Contributor

bors commented Aug 14, 2019

📌 Commit 6c3a98e has been approved by Centril

@bors 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 Aug 14, 2019
Centril added a commit to Centril/rust that referenced this pull request Aug 14, 2019
@Centril
Rollup merge of rust-lang#63507 - estebank:type-inference-error, r=Ce…
d2d49d2 
…ntril

When needing type annotations in local bindings, account for impl Trait and closures

Fix rust-lang#46680, fix rust-lang#63504, fix rust-lang#63506, fix rust-lang#40014, cc rust-lang#63502.
@Centril Centril mentioned this pull request Aug 14, 2019
Merged
bors added a commit that referenced this pull request Aug 15, 2019
@bors
Auto merge of #63575 - Centril:rollup-anlv9g5, r=Centril
9e9a136 
Rollup of 11 pull requests

Successful merges:

 - #62984 (Add lint for excess trailing semicolons)
 - #63075 (Miri: Check that a ptr is aligned and inbounds already when evaluating `*`)
 - #63490 (libsyntax: cleanup and refactor `pat.rs`)
 - #63507 (When needing type annotations in local bindings, account for impl Trait and closures)
 - #63509 (Point at the right enclosing scope when using `await` in non-async fn)
 - #63528 (syntax: Remove `DummyResult::expr_only`)
 - #63537 (expand: Unimplement `MutVisitor` on `MacroExpander`)
 - #63542 (Add NodeId for Arm, Field and FieldPat)
 - #63543 (Merge Variant and Variant_)
 - #63560 (move test that shouldn't be in test/run-pass/)
 - #63570 (Adjust tracking issues for `MaybeUninit<T>` gates)

Failed merges:

r? @ghost
@bors bors merged commit 6c3a98e into rust-lang:master Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril approved these changes

Assignees

@Centril Centril

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
5 participants
@estebank @rust-highfive @Centril @bors @eddyb