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

Improve diagnostics when closure doesn't meet trait bound #80635

Merged
merged 1 commit into from Jan 17, 2021

Conversation

roxelo
Copy link
Member

@roxelo roxelo commented Jan 3, 2021

Improves the diagnostics when closure doesn't meet trait bound by modifying TypeckResuts::closure_kind_origins such that hir::Place is used instead of Symbol. Using hir::Place to describe which capture influenced the decision of selecting a trait a closure satisfies to (Fn/FnMut/FnOnce, Copy) allows us to show precise path in the diagnostics when capture_disjoint_field feature is enabled.

Closes rust-lang/project-rfc-2229/issues/21

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 3, 2021
@roxelo roxelo changed the title Use hir::Place instead of Symbol in closure_kind_origin Improve diagnostics when closure doesn't meet trait bound Jan 12, 2021
for (i, proj) in place.projections.iter().enumerate() {
match proj.kind {
HirProjectionKind::Deref => {
curr_string = format!("*{}", curr_string);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you can use write! macro to append to strings more efficiently -- here is an example

@nikomatsakis
Copy link
Contributor

This basically looks good! I left one nit, but r=me. @roxelo, I'm delegating r+, so you should be able to write @bors r=nikomatsakis and approve the PR once you've addressed the nit.

@bors delegate+

@bors
Copy link
Contributor

bors commented Jan 16, 2021

📌 Commit b498870 has been approved by `nikomatsakis``

@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 Jan 16, 2021
@nikomatsakis
Copy link
Contributor

Oh, I guess bors saw my command. Well, I think it's fine as is actually so maybe I'll just leave it :)

@nikomatsakis
Copy link
Contributor

@bors delegate+

Regardless, I'll make sure bors sees the delegate request.

@bors
Copy link
Contributor

bors commented Jan 16, 2021

✌️ @roxelo can now approve this pull request

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 16, 2021
…mbol, r=nikomatsakis`

Improve diagnostics when closure doesn't meet trait bound

Improves the diagnostics when closure doesn't meet trait bound by modifying `TypeckResuts::closure_kind_origins` such that `hir::Place` is used instead of `Symbol`. Using `hir::Place` to describe which capture influenced the decision of selecting a trait a closure satisfies to (Fn/FnMut/FnOnce, Copy) allows us to show precise path in the diagnostics when `capture_disjoint_field` feature is enabled.

Closes rust-lang/project-rfc-2229/issues/21

r? `@nikomatsakis`
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 17, 2021
…mbol, r=nikomatsakis`

Improve diagnostics when closure doesn't meet trait bound

Improves the diagnostics when closure doesn't meet trait bound by modifying `TypeckResuts::closure_kind_origins` such that `hir::Place` is used instead of `Symbol`. Using `hir::Place` to describe which capture influenced the decision of selecting a trait a closure satisfies to (Fn/FnMut/FnOnce, Copy) allows us to show precise path in the diagnostics when `capture_disjoint_field` feature is enabled.

Closes rust-lang/project-rfc-2229/issues/21

r? ``@nikomatsakis``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2021
Rollup of 13 pull requests

Successful merges:

 - rust-lang#79298 (correctly deal with late-bound lifetimes in anon consts)
 - rust-lang#80031 (resolve: Reject ambiguity built-in attr vs different built-in attr)
 - rust-lang#80201 (Add benchmark and fast path for BufReader::read_exact)
 - rust-lang#80635 (Improve diagnostics when closure doesn't meet trait bound)
 - rust-lang#80765 (resolve: Simplify collection of traits in scope)
 - rust-lang#80932 (Allow downloading LLVM on Windows and MacOS)
 - rust-lang#80983 (Remove is_dllimport_foreign_item definition from cg_ssa)
 - rust-lang#81064 (Support non-stage0 check)
 - rust-lang#81080 (Force vec![] to expression position only)
 - rust-lang#81082 (BTreeMap: clean up a few more comments)
 - rust-lang#81084 (Use Option::map instead of open-coding it)
 - rust-lang#81095 (Use Option::unwrap_or instead of open-coding it)
 - rust-lang#81107 (Add NonZeroUn::is_power_of_two)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 19f9780 into rust-lang:master Jan 17, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 17, 2021
}
}

curr_string.to_string()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why convert String back to String?

Copy link
Member Author

@roxelo roxelo Jan 30, 2021

Choose a reason for hiding this comment

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

It's not needed, not sure why I did it 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update TypeckResuts::closure_kind_origin to use Place
6 participants