Skip to content

Name consistency of async function attributes #918

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

Closed
raviqqe opened this issue Mar 29, 2018 · 10 comments
Closed

Name consistency of async function attributes #918

raviqqe opened this issue Mar 29, 2018 · 10 comments

Comments

@raviqqe
Copy link
Contributor

raviqqe commented Mar 29, 2018

Attributes of async and async_stream to box them which are currently named pinned and pinned_send should be the same as async_move and async_stream_move like boxed and boxed_send from users' aspects. That is because async or async_stream functions are pinned anyway whether they are boxed or not.

@withoutboats
Copy link

The reason for the difference is that they put them in a PinBox whereas the move ones put them in a Box. But I'm also fine with using the same name for both, I didn't think too hard about this decision.

@Nemo157
Copy link
Member

Nemo157 commented Mar 29, 2018

I was recently wondering why we have separate async and async_move attributes instead of an async(move) attribute argument (although, I think with the std nomenclature it should be async(unpin) now)? It seems to me most consistent to pass all these modifiers via arguments and try and reduce the set of macros to the minimum possible.

@raviqqe
Copy link
Contributor Author

raviqqe commented Mar 29, 2018

I mean that it doesn't make sense for users to put pinned attribute to async or async_stream functions as normal async or async_stream functions which are not in PinBox are also pinned. But I'm not sure how other users feel about that because I know their implementation and am not a "pure" user.

@withoutboats
Copy link

The whole API was just whatever made sense as I was working on it, and it could be a good idea to change it.

We could have three attribute arguments:

  • move
  • boxed
  • send

And the combination of these produces the outcomes we have today.

@raviqqe
Copy link
Contributor Author

raviqqe commented Mar 29, 2018

Another option is:

  • move
  • boxed
  • boxed_send

as the send attribute is always used with boxed but cannot without it.

@withoutboats
Copy link

Its true that send without boxed wouldn't mean anything right now. I wonder if it has to, though? Couldn't we generate impl Future + Send as a way of checking that your future stays Send?

@raviqqe
Copy link
Contributor Author

raviqqe commented Mar 29, 2018

Is there any use case where impl Future + Send is necessary? I guess the Send annotation is guessed automatically by Rust compiler because impl Trait indicates the least requirement unlike trait objects.

@withoutboats
Copy link

You're right, its not necessary. But if you wanted to get an error if you accidentally made your future !Send, you could use this to get that (basically as a check on yourself).

@Nemo157
Copy link
Member

Nemo157 commented Apr 5, 2018

An extension to this would be to integrate the async_stream macro in as well via something like #[async(stream(item = T)). This would reduce the attribute macros exported by futures down to just the single #[async] macro with 4 possible arguments which all compose together.

@raviqqe
Copy link
Contributor Author

raviqqe commented Apr 12, 2018

I think @withoutboats's idea about putting Send manually to prevent accidental !Send asynchronous functions is reasonable as I struggled with that kind of stuff a lot before making a bunch of async functions which refer to each other in a complex way.

Nemo157 added a commit to Nemo157/futures-rs that referenced this issue Apr 12, 2018
Nemo157 added a commit to Nemo157/futures-rs that referenced this issue Apr 12, 2018
Nemo157 added a commit to Nemo157/futures-rs that referenced this issue Apr 16, 2018
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

No branches or pull requests

3 participants