-
Notifications
You must be signed in to change notification settings - Fork 259
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
#[callback_vec]
does not respect other callback parameters
#860
Comments
Another idea is to combine this with #861 and create two new annotations |
Also, if we change the semantics, we need to consider what happens when callback vec is before a specific callback. Does it pull all elements from promises len -2? What happens if callback_vec is in the middle? If there are two callback_vec, should we error at compile time? |
Yeah that is why I said that we should probably enforce for callback vec to always be the last callback parameter.
Probably, I don't see how two callback vec parameters could be useful |
Consider following function:
Intuitively it feels like
#[callback_vec]
parameter should act as a trailing variadic for all remaining promise callbacks. So one would expect that if you pass 4 promises tofoo
, thena
will resolve to the first callback,b
to the second andother
to the remaining two. The current implementation of#[callback_vec]
behaves differently though:other
will resolve to all four callbacks disregarding thata
andb
have already resolved the first two.I can see two paths forward:
#[callback_unwrap]
/#[callback_result]
and#[callback_vec]
together since this can be a footgun. This would be a fairly small breaking change for people who were mixing this pattern for some reason. They could just remove the regular callbacks and grab them from the vector manually.#[callback_vec]
to only unwrap promises that have not already been handled by other callback parameters. Also, forbid declaring#[callback_unwrap]
/#[callback_result]
parameters after the callback vec parameter (ensuring it is always the last one just to highlight this new semantics).The text was updated successfully, but these errors were encountered: