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

Attempt to remove YokeTraitHack #1090

Closed
wants to merge 3 commits into from
Closed

Conversation

sffc
Copy link
Member

@sffc sffc commented Sep 23, 2021

I get a lot further than before, but there's still an error on StructProvider.

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • provider/core/src/data_provider.rs is different
  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • provider/core/src/struct_provider.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@sffc
Copy link
Member Author

sffc commented Oct 1, 2021

Blocked on rust-lang/rust#89196

@sffc sffc added the blocked A dependency must be resolved before this is actionable label Oct 1, 2021
@sffc
Copy link
Member Author

sffc commented Oct 1, 2021

Based on @Manishearth's respose rust-lang/rust#89196 (comment), it seems unlikely that we will be able to get rid of YokeTraitHack, unless it turns out that the inference bug is solvable.

To be clear, the issue impacts virtually every call site of DataProvider::load_payload. The cost of fully qualifying every call site of DataPayload::load_payload is much higher than the cost of keeping YokeTraitHack.

Shall I close this PR and open a new bug to track the new Rust bug in the chance that it is fixed at some point?

@Manishearth
Copy link
Member

@sffc sounds good. We can still remove the _with_capture and other hacks we had to add.

I was a bit worried about this when introducing DataMarker (because of the kind of indirection it introduces) but on balance I think having to use the trait hack is better than not having DataMarker's ergonomics

@sffc
Copy link
Member Author

sffc commented Oct 1, 2021

This PR is superseded by #1134

#1061 is the issue to track removing the workarounds for Rust bugs.

I am attempting to remove _with_capture in #1133 but I hit another issue, which is explained over there.

@sffc sffc closed this Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked A dependency must be resolved before this is actionable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants