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

feat: remove not required Send bound #32

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

dignifiedquire
Copy link
Contributor

The async traits don't actually require the use of the Send bound. This is required for us to make use of them in a wasm context, where we explicitly have !Send readers and writers.

The async traits don't actually require the use of the `Send` bound. This is required for us to make use of them in a wasm context, where we explicitly have `!Send` readers and writers.
@dermesser dermesser merged commit f8a2339 into dermesser:master Jun 30, 2023
@dignifiedquire dignifiedquire deleted the feat-no-send-bound branch June 30, 2023 08:44
dermesser added a commit that referenced this pull request Jun 30, 2023
@Stebalien
Copy link

FYI, this broke our car implementation. The fact that Send is no longer required means that the returned future is no longer Send, and there's no way to convince rust to abstract over it.

@dermesser
Copy link
Owner

@Stebalien would you mind trying the branch in #33 with your code? I am not quite sure how it interacts with the async_trait crate. In that case you can enable (by default) the feature sendable_io_traits (or disable it, in case of wasm targets).

@Stebalien
Copy link

Unfortunately I get the same error. But I have no idea why.

Here's a reproducer: https://github.com/filecoin-project/ref-fvm/tree/integer-encoding-send-error

Note: I found that we were actually using two varint crates (yours and unsigned-varint) so I ended up just switching over to the other one, so this isn't really an issue for us anymore.

@dermesser
Copy link
Owner

Unfortunately I get the same error. But I have no idea why.

Here's a reproducer: https://github.com/filecoin-project/ref-fvm/tree/integer-encoding-send-error

Note: I found that we were actually using two varint crates (yours and unsigned-varint) so I ended up just switching over to the other one, so this isn't really an issue for us anymore.

thank you, also for the reproduction case! I'm glad it is solved for you, I will look into whether it is still worth solving in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants