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

How can we improve map_callback_return_to_option_ms? #639

Open
flosse opened this issue Nov 25, 2021 · 2 comments
Open

How can we improve map_callback_return_to_option_ms? #639

flosse opened this issue Nov 25, 2021 · 2 comments

Comments

@flosse
Copy link
Member

flosse commented Nov 25, 2021

It happens to me again and again that I stumble over the following runtime error:

Cmds can return only Msg, Option<Msg> or ()!

The cause is in the map_callback_return_to_option_ms! macro.

It would be great if we were able to catch this errors at compile time.
I know very little about macros, so can someone help to explain how that could be improved?

@MartinKavik
Copy link
Member

See the line pointing to #391. There are two links:

  1. https://gist.github.com/MartinKavik/36a96f28b9bad76d6cb7141946f5f716
  2. https://gist.github.com/MartinKavik/918ff345f1655eb32eb47090ecc73cd8

The first link points to the code used in the map_callback_return_to_option_ms! (precisely its POC without Option support). The macro itself was written lately while I was refactoring the code => the macro doesn't contain any macro magic, it's basically a simple template even though it looks like a monster due to the ugly code inside. The code itself "abuses" Any to provide a runtime workaround for conflicting trait implementations. To understand the motivation, we should look at the second link.
(Note: You can see an extra Option wrapper in the implementation because the Option has been "abused" as a Msg container while its casted to Any and back because casting through Any is possible only through a mutable reference, it isn't possible to do it with an owned value directly.)

So the second link shows a compile-time alternative to the whole map_callback_return_to_option_ms! macro. The goal is to support multiple return types of handlers. The problem is it works only on nightly. Once it works on stable, we can finally throw out that ugly macro and related code.
(Note: The code works with the current Rust and Edition 2021 with a feature renaming and enabling: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=77983f628bdc9d5275250beb48a72ed8)

The motivation for multiple return types was Hooks integration (returning ()) and conditionally sent Msg (returning Option<Msg>) so you don't have to create some dummy Msgs like Msg::Noop.

Note: The generic parameter MsU means "Msg or unit" (if I remember correctly) because the first implementation didn't have support for Option - only for Ms and ().

Hope it helps! I'm not very proud to have such code in the code base, I hope there'll be a better solution.

@fosskers
Copy link
Collaborator

fosskers commented Dec 8, 2021

I'll get the Edition 2021 PR merged, then let's go ahead with the proper fix for this.

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