Feature: Allow guards to return custom errors

[MathiasKoch]

I think it would be very beneficial to allow guards to return Result<(), Error> rather than bool, where false = Err(Error::GuardFailed).

I think most of it is fairly simple to implement, and i would love to come up with a PR for this, if this is something that you can see the value of?

The only place i come up a bit short is how we want to syntactically define these new error variants?

[Yatekii]

I think this would be very valuable :) I wanted this since the beginning but I didn't have enough time/energy yet :)

[korken89]

This would indeed be an improvement

[MathiasKoch]

@Yatekii @korken89

I would love to add a PR for this, but do you have any suggestions on where/how we should define error variants? Should they be part of DSL generation and if so, with what syntax?

[Yatekii]

Good question, they have to be known in advance :/

Or wait, maybe they don't even. Isn't it enough to just define the guard and it all works when you propagate the error in the statemachine with ? pretty much?

[MathiasKoch]

Yeah, that should be enough, but i think they need to be part of the generated Error enum?

[MathiasKoch]

Is the current DSL a standard from somewhere, or is it completely made up within the bounds of this repo?

[Yatekii]

I think the DSL is inferred from boost: https://boost-ext.github.io/sml/overview.html

[MathiasKoch]

Could it be a solution to allow defining error variants in the beginning of the statemachine! macro along the lines of:

statemachine!{
    #[Variant1, Variant2, Variant3], // # denotes error variants 
    State1 + Event1 = State2,
    // ...
}

Which would then code-gen to:

pub enum Error {
    /// When an event is processed which should not come in the current state.            
    InvalidEvent,
    /// When an event is processed whose guard did not return `true`.
    GuardFailed,
    Variant1,
    Variant2,
    Variant3,
}

And then change the signature of guards to return Result<(), Error>?

Alternative code-gen:

pub enum GuardError {
    Variant1,
    Variant2,
    Variant3,
}

pub enum Error {
    /// When an event is processed which should not come in the current state.            
    InvalidEvent,
    /// When an event is processed whose guard did not return `true`.
    GuardFailed(GuardError),
}

With a guard signature to return Result<(), GuardError>?

[korken89]

Ah right, now I remember why I did not do this from the start. I have been toying with the idea to make the proc macro more powerful, and this would be in the same direction. There is an old discussion in which extension syntax was talked about, but I can't find it right now.

Overall though I am open to extending and making the macro more powerful!

[MathiasKoch]

@korken89 Great!

Overall though I am open to extending and making the macro more powerful!

Do you have even larger changes in mind, than the ones i suggested here? And also, do you have an opinion on the suggestions made? Maybe even a better syntax for it, as i am less interested in the actual syntax for this, and more in the feature output of it.

[korken89]

I can't find the original discussion, but it was something along the lines of:

statemachine!{
    guard_error: GuardError, // Guard error type
    by_value: true, // Or something to indicate when one wants the statemachine to use moves instead of references
    transitions: {
        // The old transition syntax
    },
};

The main idea was to use names so one can easily extend the macro more without breaking changes hopefully.

[MathiasKoch]

Aha, that looks great! It's a slightly bigger change to the code-gen, but i htink it would be worth it for the added flexibility in the long run.

I can try to give it a shot, when i get the time.

[korken89]

That would be awesome! Ping me if you have questions. I'm not so active at the moment in all the open-source work as we are having a baby any day now. :)

[MathiasKoch]

Wauw, Congratz!

[MathiasKoch]

@korken89 @Yatekii I have done most of the work for this new named syntax with:

statemachine!{
    guard_error: GuardError, // Guard error type
    temporary_context: &mut u16,
    transitions: {
        // The old transition syntax
    },
};

And everything works as i would expect it to, such that the above would generate guards with signature: fn guard1(&mut self, _event_data: &MyEventData) -> Result<(), GuardError>

The only thing we need take a decision on, is whether the guard signature should be A or B in case guard_error is unspecified (default behaviour)

• A. fn guard1(&mut self, _event_data: &MyEventData) -> bool
• B. fn guard1(&mut self, _event_data: &MyEventData) -> Result<(), ()>

I can see pros and cons of both and personally i would lean towards B for consistency.

Main disadvantage of B, is that it would no longer be possible to do the super simple comparison guards.

fn guard(&mut self, event_data: &MyEventData) -> bool {
    event_data == &MyEventData(42)
}

My take on this is to leave the by_value: true, // Or something to indicate when one wants the statemachine to use moves instead of references for another PR.

EDIT: You can see the current PR state here: #15