-
Notifications
You must be signed in to change notification settings - Fork 31
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
Mailbox should be the first argument in spawn
and spawn_link
#62
Comments
Yes, I stumbled upon the same problem, trying to pass cap1, cap2 etc. before mailbox. But why isn't it possible to define a macro that handles mailbox being last? And what about mixing captured variables, variables defined during invocation, and mailbox: in what order? |
If you try this example (with mailbox last): macro_rules! spawn {
( $( $captures:expr ),*, $mailbox:expr ) => {};
}
spawn!(1, 2, 3); It has an error:
But switching captures and mailbox around, it works just fine. |
Sorry, I completely forgot to come back to this. The reason why it's the last argument is to stay consistent with I'm not completely against changing this. Ideally, I would like |
Can we not do something similar to submillisecond, where we implement a trait for a function? impl<M, A, B, C> SpawnFn<M, (A, B, C)> for fn(Mailbox<M>, A, B, C)
where
M: Serialize,
A: Serialize,
B: Serialize,
C: Serialize,
{ ... } |
Not without forcing everyone to write |
The Mailbox prameter in the
spawn
andspawn_link
functions is expected to be the last parameter.I think it might make more sense for it to be first, since it's always required?
Similar to how typically optional/default arguments are placed last in many languages, similar to the type definition of
Mailbox<T, S = Bincode>
, with T being required, and S being optional.I think a side effect of this, would be that the
spawn!
andspawn_link!
macros could accept multiple captured variables. Currently they only support one, but it could be many:With mailbox being last, I don't think this is really possible.
It's a pretty minor change, and would be a breaking change sadly, but I thought I'd open an issue anyway.
The text was updated successfully, but these errors were encountered: