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

Improved diagnostic on failure to meet send bound on future in a foreign crate #94493

Merged
merged 1 commit into from
Apr 19, 2022
Merged

Improved diagnostic on failure to meet send bound on future in a foreign crate #94493

merged 1 commit into from
Apr 19, 2022

Conversation

oribenshir
Copy link
Contributor

@oribenshir oribenshir commented Mar 1, 2022 •
edited
Loading

Provide a better diagnostic on failure to meet send bound on futures in a foreign crate.

fixes #78543

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 1, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 1, 2022
@oribenshir oribenshir changed the title issue 78543 - async fn in foreign crate diag Improved diagnostic on failure to meet send bound on future in a foreign crate Mar 1, 2022
@oribenshir
Copy link
Contributor Author

This is my first PR for the rustc project, will need a rigorous review. And sorry in advance if I missed anything from the contribution guidelines.

@oribenshir oribenshir mentioned this pull request Mar 1, 2022
Closed
@bors
Copy link
Contributor

bors commented Mar 2, 2022

☔ The latest upstream changes (presumably #87402) made this pull request unmergeable. Please resolve the merge conflicts.

@lcnr
Copy link
Contributor

lcnr commented Mar 2, 2022

r? @estebank maybe, don't have the capacity for this rn

@rust-highfive rust-highfive assigned estebank and unassigned lcnr Mar 2, 2022
@estebank
Copy link
Contributor

estebank commented Mar 2, 2022 •
edited
Loading

Thanks for this! You'll have to rebase against a more recent master. Can you also make sure you run ./x.py fmt? It'd be also a good idea to squash the commits, so far I'm only seeing a single unit of work so multiple commits aren't needed.

I'll take a more detailed view later this week, but so far it looks good.

@oribenshir oribenshir force-pushed the feature/ISSUE-78543_async_fn_in_foreign_crate_diag_2 branch 2 times, most recently from 4dd9b89 to 726bdab Compare March 5, 2022 10:08
@estebank
Copy link
Contributor

I haven't forgotten about this, sorry for the delay in reviewing!

davidtwco
davidtwco requested changes Apr 3, 2022
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

Apologies for the delay in having this reviewed, this looks pretty good to me, I've left a few comments.

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/context.rs Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/context.rs Outdated Show resolved Hide resolved
compiler/rustc_metadata/src/rmeta/encoder.rs Outdated Show resolved Hide resolved
@davidtwco
Copy link
Member

r? @davidtwco

@rust-highfive rust-highfive assigned davidtwco and unassigned estebank Apr 3, 2022
@davidtwco davidtwco 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 Apr 3, 2022
@bors
Copy link
Contributor

bors commented Apr 13, 2022

☔ The latest upstream changes (presumably #95656) made this pull request unmergeable. Please resolve the merge conflicts.

@oribenshir
Provide a better diagnostic on failure to meet send bound on futures …
ebe3c56 
…in a foreign crate

Adding diagnostic data on generators to the crate metadata and using it to provide
a better diagnostic on failure to meet send bound on futures originated from a foreign crate
@oribenshir oribenshir force-pushed the feature/ISSUE-78543_async_fn_in_foreign_crate_diag_2 branch from 726bdab to ebe3c56 Compare April 16, 2022 11:13
@oribenshir
Copy link
Contributor Author

Sorry for the late response (recovered from COVID)
Thanks for the review, I handled all the requests and the merge conflict

@davidtwco
Copy link
Member

Thanks!

@bors r+

@davidtwco
Copy link
Member

bors? you there?

@bors r+

@bors
Copy link
Contributor

bors commented Apr 19, 2022

📌 Commit ebe3c56 has been approved by davidtwco

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 19, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#94493 - oribenshir:feature/ISSUE-78543_asyn…
a857380 
…c_fn_in_foreign_crate_diag_2, r=davidtwco

Improved diagnostic on failure to meet send bound on future in a foreign crate

Provide a better diagnostic on failure to meet send bound on futures in a foreign crate.

fixes rust-lang#78543
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#94493 - oribenshir:feature/ISSUE-78543_asyn…
15cb9eb 
…c_fn_in_foreign_crate_diag_2, r=davidtwco

Improved diagnostic on failure to meet send bound on future in a foreign crate

Provide a better diagnostic on failure to meet send bound on futures in a foreign crate.

fixes rust-lang#78543
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Closed
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2022
@bors
Auto merge of rust-lang#96214 - Dylan-DPC:rollup-a5b4fow, r=Dylan-DPC
4ca19e0 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#94493 (Improved diagnostic on failure to meet send bound on future in a foreign crate)
 - rust-lang#95809 (Fix typo in bootstrap.py)
 - rust-lang#96086 (Remove `--extern-location` and all associated code)
 - rust-lang#96089 (`alloc`: make `vec!` unavailable under `no_global_oom_handling`)
 - rust-lang#96122 (Fix an invalid error for a suggestion to add a slice in pattern-matching)
 - rust-lang#96142 (Stop using CRATE_DEF_INDEX outside of metadata encoding.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ab59516 into rust-lang:master Apr 19, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.

Bad diagnostic for Send bound on Future coming from async fn in foreign crate
7 participants
@oribenshir @rust-highfive @bors @lcnr @estebank @davidtwco @rustbot