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

the msg! macro #214

Closed
o0Ignition0o opened this issue Jun 19, 2020 · 3 comments · Fixed by #309
Closed

the msg! macro #214

o0Ignition0o opened this issue Jun 19, 2020 · 3 comments · Fixed by #309
Labels
A-children Area: Children related issues D-general Diagnostics: General enhancement New feature or request research We need to take a look if this thing is possible

Comments

@o0Ignition0o
Copy link
Contributor

One of the first pain points I had when I started working on bastion was the msg! macro.

There's a lot going on there, and it might be refactored leveraging Type Level Programming or something.

Let's share our thoughts on that here, and maybe we can figure out a nice trait or API to setup children and message handlers :)

For reference, here's what our current handlers look like:

// This is the heavylifting.
// A child will wait for a message, and try to process it.
async fn fib_child_task(ctx: BastionContext) -> Result<(), ()> {
    loop {
        // This macro is weird.
        // Bear with me as I tell you more
        // about the variants in the match statements below.
        // so the first line means "wait for the next message", and then lets match against it
        msg! { ctx.recv().await?,
            // =!> refer to messages that can be replied to.
            // In order to reply to a message, we use the answer! macro
            // and pass ctx as parameter, so that bastion knows where to send the reply
            request: String =!> {
                let (command, number) = deserialize_into_fib_command(request);
                if command == "fib" {
                    answer!(ctx, format!("{}", fib(number))).expect("couldn't reply :(");
                } else {
                    answer!(ctx, "I'm sorry I didn't understand the task I was supposed to do").expect("couldn't reply :(");
                }
            };
            // ref <name> are broadcasts.
            // each child in a children group receives a broadcast and can process it.
            ref broadcast: String => {
                info!("received broadcast: {:?}", *broadcast);
            };
            // <name> (without the ref keyword) are messages that have a unique recipient.
            message: String => {
                info!("someone told me something: {}", message);
            };
            // <name> that have the `_` type are catch alls
            unknown:_ => {
                error!("uh oh, I received a message I didn't understand\n {:?}", unknown);
            };
        }
    }
}
@Relrin
Copy link
Member

Relrin commented Jul 17, 2020

Probably #236 issue is a part of the ticket

@vertexclique vertexclique added A-callbacks Area: Lifecycle callbacks A-children Area: Children related issues D-general Diagnostics: General enhancement New feature or request research We need to take a look if this thing is possible and removed A-callbacks Area: Lifecycle callbacks labels Jul 19, 2020
@resolritter
Copy link

This week was my first working with this project. The msg! macro was nothing but unpleasant work with at first sight (I also think it looks ugly, but I digress).

One big issue is that rustfmt doesn't work properly on code inside of macros. This one hosts whole function bodies inside of it which get no benefit from the formatting from rustfmt.

Another bad thing is that, since it's a syntax of its own, it added cumbersomeness and cognitive burden to starting out with the library. It's a pain in the ass to balance out { and } when you're refactoring since it's not "normal" Rust code and, being a macro, the syntax errors you get are awful. You don't get that "hack away" feeling when the whole thing is just a big macro instead of being a big function.

Any approach which messes with interpolating normal code inside macros sounds like a bad idea to me. msg! even obfuscates SignedMessage and Envelope, both of which are relevant for understanding how the library works past any non-trivial example while, at the same time, also being familiar idiom for folks coming from actor systems in other languages.

In summary, this approach can't provide good ergonomics for usability. I'd rather try to spend UX time on the library's primitives or brainstorming constructs which play naturally well with Rust's syntax rather than making this "embedded DSL" thing work.

@scrabsha
Copy link
Contributor

I recently got interested in Bastion, and I may have a solution that can replace the entire msg macro.

The thing is to use method chaining instead of match arms. For instance, the fibonnaci example could become:

MsgBuilder::from(ctx.recv.await?)
    .with_question(|request: String| { /* ... */ })
    .with_broadcast(|broadcast: String| { /* ... */ })
    .with_message(|message: String| { /* ... */ })
    .with_catchall(|unknown| { /* ... */ })
    .run();

The MsgBuilder would store the different closures and call the appropriate one. A proof of concept that this is possible can be found here. It currently only handles Fn, but I think it can be refactored to accept FnMut too, if needed.

What are your thoughts on it? If this solution is accepted, then I would be very happy to implement it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-children Area: Children related issues D-general Diagnostics: General enhancement New feature or request research We need to take a look if this thing is possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants