From c3abbb2c7b98c9978f705dc5592a4b8d4c10bbed Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Sun, 30 Jun 2024 16:16:25 +0200 Subject: [PATCH 1/7] Fixing the internal state management when guards fail --- macros/src/codegen.rs | 29 +++++++++++++++++++++------- tests/test.rs | 44 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/macros/src/codegen.rs b/macros/src/codegen.rs index 6664a8b..32589be 100644 --- a/macros/src/codegen.rs +++ b/macros/src/codegen.rs @@ -487,14 +487,29 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { quote! { #guard_result self.context.log_guard(stringify!(#guard_expression), &guard_result); - if guard_result.map_err(#error_type_name::GuardFailed)? { - #action_code - let out_state = #states_type_name::#out_state; - self.context.log_state_change(&out_state); - #entry_exit_states - self.state = Some(out_state); - return self.state() + + // If the guard fails, we need to fall back to our original + // state. + match guard_result { + Err(err) => { + self.state = Some(#states_type_name::#in_state); + return Err(#error_type_name::GuardFailed(err)); + } + + // If the guard passed, we transition immediately. + // Otherwise, there may be a later transition that passes, + // so we'll defer to that. + Ok(true) => { + #action_code + let out_state = #states_type_name::#out_state; + self.context.log_state_change(&out_state); + #entry_exit_states + self.state = Some(out_state); + return self.state(); + } + _ => {}, } + } } else { // Unguarded transition quote!{ diff --git a/tests/test.rs b/tests/test.rs index fa0b68d..f2d4826 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -270,3 +270,47 @@ fn guarded_transition_before_unguarded() { sm.process_event(Events::Event1).unwrap(); assert!(matches!(sm.state(), Ok(&States::Fault))); } + +#[test] +fn guard_errors() { + use smlang::statemachine; + statemachine! { + transitions: { + *Init + Event1 [guard] = Done, + } + } + + struct Context { + pub guard_passable: bool, + pub guard_errors: bool, + } + impl StateMachineContext for Context { + fn guard(&mut self) -> Result { + if self.guard_errors { + Err(()) + } else { + Ok(self.guard_passable) + } + } + } + + let mut sm = StateMachine::new(Context { guard_passable: false, guard_errors: true }); + + // Test attempting to transition when the guard fails. + sm.context_mut().guard_errors = true; + assert!(sm.process_event(Events::Event1).is_err()); + assert!(matches!(sm.state(), Ok(&States::Init))); + + // Test attempting to transition when the guard is not passable. + sm.context_mut().guard_errors = false; + assert!(sm.process_event(Events::Event1).is_err()); + assert!(matches!(sm.state(), Ok(&States::Init))); + + assert!(sm.process_event(Events::Event1).is_err()); + assert!(matches!(sm.state(), Ok(&States::Init))); + + sm.context_mut().guard_passable = true; + sm.process_event(Events::Event1).unwrap(); + assert!(matches!(sm.state(), Ok(&States::Done))); + +} From e48140e23d5640becc486cc0e357e03c9cababf6 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Sun, 30 Jun 2024 16:19:08 +0200 Subject: [PATCH 2/7] Fixing format --- tests/test.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test.rs b/tests/test.rs index f2d4826..c139f9f 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -294,7 +294,10 @@ fn guard_errors() { } } - let mut sm = StateMachine::new(Context { guard_passable: false, guard_errors: true }); + let mut sm = StateMachine::new(Context { + guard_passable: false, + guard_errors: true, + }); // Test attempting to transition when the guard fails. sm.context_mut().guard_errors = true; @@ -312,5 +315,4 @@ fn guard_errors() { sm.context_mut().guard_passable = true; sm.process_event(Events::Event1).unwrap(); assert!(matches!(sm.state(), Ok(&States::Done))); - } From f0715d5ba7cfd8079eccb56e917d6158e0083269 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 1 Jul 2024 10:44:22 +0200 Subject: [PATCH 3/7] Reverting actions taking ownership of state data --- CHANGELOG.md | 2 - examples/async.rs | 6 +- examples/dominos.rs | 8 +- examples/ex1.rs | 6 +- examples/ex2.rs | 4 +- examples/ex3.rs | 4 +- examples/guard_action_syntax.rs | 2 +- ...rd_action_syntax_with_temporary_context.rs | 2 +- examples/guard_custom_error.rs | 2 +- examples/input_state_pattern_match.rs | 14 +- examples/named_async.rs | 6 +- examples/named_dominos.rs | 8 +- examples/named_ex1.rs | 6 +- examples/named_ex2.rs | 4 +- examples/named_ex3.rs | 4 +- examples/named_input_state_pattern_match.rs | 14 +- examples/on_entry_on_exit_generic.rs | 10 +- examples/reuse_action.rs | 2 +- examples/state_machine_logger.rs | 14 +- macros/src/codegen.rs | 153 ++++++------------ macros/src/lib.rs | 19 ++- tests/test.rs | 46 +++--- 22 files changed, 145 insertions(+), 191 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cc360f..cbeb0b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,8 +25,6 @@ implementation. ### Changed -- [breaking] Actions now take owned values -- [breaking] `state()` now returns a `Result` - `StateMachine::new` and `StateMachine::new_with_state` are now const functions - Fixed clippy warnings - [breaking] Changed guard functions return type from Result<(),_> to Result diff --git a/examples/async.rs b/examples/async.rs index 8acb68b..7e7e357 100644 --- a/examples/async.rs +++ b/examples/async.rs @@ -64,7 +64,7 @@ fn main() { lock: smol::lock::RwLock::new(false), done: false, }); - assert!(matches!(sm.state(), Ok(&States::State1))); + assert!(matches!(sm.state(), &States::State1)); let r = sm.process_event(Events::Event1).await; assert!(matches!(r, Ok(&States::State2))); @@ -78,11 +78,11 @@ fn main() { // Now all events will not give any change of state let r = sm.process_event(Events::Event1).await; assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::State4(_)))); + assert!(matches!(sm.state(), &States::State4(_))); let r = sm.process_event(Events::Event2).await; assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::State4(_)))); + assert!(matches!(sm.state(), &States::State4(_))); }); // ... diff --git a/examples/dominos.rs b/examples/dominos.rs index 2876400..e25b138 100644 --- a/examples/dominos.rs +++ b/examples/dominos.rs @@ -22,15 +22,15 @@ impl StateMachineContext for Context { Some(Events::ToD2) } - fn to_d3(&mut self, _state_data: Option) -> Option { + fn to_d3(&mut self, _state_data: &Option) -> Option { Some(Events::ToD3) } - fn to_d4(&mut self, _state_data: Option) -> Option { + fn to_d4(&mut self, _state_data: &Option) -> Option { Some(Events::ToD4) } - fn to_d5(&mut self, _state_data: Option) -> Option { + fn to_d5(&mut self, _state_data: &Option) -> Option { Some(Events::ToD5) } } @@ -67,5 +67,5 @@ fn main() { } // All the dominos fell! - assert!(matches!(sm.state(), Ok(&States::D5))); + assert!(matches!(sm.state(), &States::D5)); } diff --git a/examples/ex1.rs b/examples/ex1.rs index 79df62e..3320b69 100644 --- a/examples/ex1.rs +++ b/examples/ex1.rs @@ -21,7 +21,7 @@ impl StateMachineContext for Context {} fn main() { let mut sm = StateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&States::State1))); + assert!(matches!(sm.state(), &States::State1)); let r = sm.process_event(Events::Event1); assert!(matches!(r, Ok(&States::State2))); @@ -32,9 +32,9 @@ fn main() { // Now all events will not give any change of state let r = sm.process_event(Events::Event1); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::State3))); + assert!(matches!(sm.state(), &States::State3)); let r = sm.process_event(Events::Event2); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::State3))); + assert!(matches!(sm.state(), &States::State3)); } diff --git a/examples/ex2.rs b/examples/ex2.rs index d413175..cd15e7f 100644 --- a/examples/ex2.rs +++ b/examples/ex2.rs @@ -22,7 +22,7 @@ impl StateMachineContext for Context {} fn main() { let mut sm = StateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&States::State1))); + assert!(matches!(sm.state(), &States::State1)); let r = sm.process_event(Events::Event1); assert!(matches!(r, Ok(&States::State2))); @@ -43,5 +43,5 @@ fn main() { // Now we cannot use Event1 again, as it is outside the state machine loop let r = sm.process_event(Events::Event1); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::State2))); + assert!(matches!(sm.state(), &States::State2)); } diff --git a/examples/ex3.rs b/examples/ex3.rs index 2048d11..0879ff3 100644 --- a/examples/ex3.rs +++ b/examples/ex3.rs @@ -39,7 +39,7 @@ impl StateMachineContext for Context { fn main() { let mut sm = StateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&States::State1))); + assert!(matches!(sm.state(), &States::State1)); println!("Before action 1"); @@ -58,5 +58,5 @@ fn main() { println!("After action 2"); // Now we are stuck due to the guard never returning true - assert!(matches!(sm.state(), Ok(&States::State2))); + assert!(matches!(sm.state(), &States::State2)); } diff --git a/examples/guard_action_syntax.rs b/examples/guard_action_syntax.rs index 653dc8e..f3b953f 100644 --- a/examples/guard_action_syntax.rs +++ b/examples/guard_action_syntax.rs @@ -42,7 +42,7 @@ impl StateMachineContext for Context { } // Action2 has access to the data from State2 - fn action2(&mut self, _state_data: MyStateData) { + fn action2(&mut self, _state_data: &MyStateData) { todo!() } } diff --git a/examples/guard_action_syntax_with_temporary_context.rs b/examples/guard_action_syntax_with_temporary_context.rs index 628156e..69b4be2 100644 --- a/examples/guard_action_syntax_with_temporary_context.rs +++ b/examples/guard_action_syntax_with_temporary_context.rs @@ -49,7 +49,7 @@ impl StateMachineContext for Context { } // Action2 has access to the data from State2 - fn action2(&mut self, temp_context: &mut u16, _state_data: MyStateData) { + fn action2(&mut self, temp_context: &mut u16, _state_data: &MyStateData) { *temp_context += 1; } } diff --git a/examples/guard_custom_error.rs b/examples/guard_custom_error.rs index fa307bf..e7437ca 100644 --- a/examples/guard_custom_error.rs +++ b/examples/guard_custom_error.rs @@ -50,7 +50,7 @@ impl StateMachineContext for Context { } // Action2 has access to the data from State2 - fn action2(&mut self, _state_data: MyStateData) { + fn action2(&mut self, _state_data: &MyStateData) { todo!() } } diff --git a/examples/input_state_pattern_match.rs b/examples/input_state_pattern_match.rs index bf0b123..006dfeb 100644 --- a/examples/input_state_pattern_match.rs +++ b/examples/input_state_pattern_match.rs @@ -45,7 +45,7 @@ impl StateMachineContext for Context {} fn main() { let mut sm = StateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&States::Idle))); + assert!(matches!(sm.state(), &States::Idle)); let r = sm.process_event(Events::Charge); assert!(matches!(r, Ok(&States::Charging))); @@ -61,7 +61,7 @@ fn main() { let r = sm.process_event(Events::Charge); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::Charged))); + assert!(matches!(sm.state(), &States::Charged)); let r = sm.process_event(Events::Discharge); assert!(matches!(r, Ok(&States::Discharging))); @@ -71,7 +71,7 @@ fn main() { let r = sm.process_event(Events::Discharge); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::Discharged))); + assert!(matches!(sm.state(), &States::Discharged)); sm = StateMachine::new_with_state(Context, States::Idle); let r = sm.process_event(Events::FaultDetected); @@ -95,17 +95,17 @@ fn main() { let r = sm.process_event(Events::Charge); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); let r = sm.process_event(Events::Discharge); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); let r = sm.process_event(Events::ChargeComplete); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); let r = sm.process_event(Events::DischargeComplete); assert!(matches!(r, Err(Error::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); } diff --git a/examples/named_async.rs b/examples/named_async.rs index 97053d5..e34f201 100644 --- a/examples/named_async.rs +++ b/examples/named_async.rs @@ -60,7 +60,7 @@ fn main() { lock: smol::lock::RwLock::new(false), done: false, }); - assert!(matches!(sm.state(), Ok(&AsyncSimpleStates::State1))); + assert!(matches!(sm.state(), &AsyncSimpleStates::State1)); let r = sm.process_event(AsyncSimpleEvents::Event1).await; assert!(matches!(r, Ok(&AsyncSimpleStates::State2))); @@ -74,11 +74,11 @@ fn main() { // Now all events will not give any change of state let r = sm.process_event(AsyncSimpleEvents::Event1).await; assert!(matches!(r, Err(AsyncSimpleError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&AsyncSimpleStates::State4(_)))); + assert!(matches!(sm.state(), &AsyncSimpleStates::State4(_))); let r = sm.process_event(AsyncSimpleEvents::Event2).await; assert!(matches!(r, Err(AsyncSimpleError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&AsyncSimpleStates::State4(_)))); + assert!(matches!(sm.state(), &AsyncSimpleStates::State4(_))); }); // ... diff --git a/examples/named_dominos.rs b/examples/named_dominos.rs index 02f6f08..8b258d1 100644 --- a/examples/named_dominos.rs +++ b/examples/named_dominos.rs @@ -23,15 +23,15 @@ impl DominosStateMachineContext for Context { Some(DominosEvents::ToD2) } - fn to_d3(&mut self, _state_data: Option) -> Option { + fn to_d3(&mut self, _state_data: &Option) -> Option { Some(DominosEvents::ToD3) } - fn to_d4(&mut self, _state_data: Option) -> Option { + fn to_d4(&mut self, _state_data: &Option) -> Option { Some(DominosEvents::ToD4) } - fn to_d5(&mut self, _state_data: Option) -> Option { + fn to_d5(&mut self, _state_data: &Option) -> Option { Some(DominosEvents::ToD5) } } @@ -68,5 +68,5 @@ fn main() { } // All the dominos fell! - assert!(matches!(sm.state(), Ok(&DominosStates::D5))); + assert!(matches!(sm.state(), &DominosStates::D5)); } diff --git a/examples/named_ex1.rs b/examples/named_ex1.rs index 175a4be..5ba3c17 100644 --- a/examples/named_ex1.rs +++ b/examples/named_ex1.rs @@ -23,7 +23,7 @@ impl LinearStateMachineContext for Context {} fn main() { let mut sm = LinearStateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&LinearStates::State1))); + assert!(matches!(sm.state(), &LinearStates::State1)); let r = sm.process_event(LinearEvents::Event1); assert!(matches!(r, Ok(&LinearStates::State2))); @@ -34,9 +34,9 @@ fn main() { // Now all events will not give any change of state let r = sm.process_event(LinearEvents::Event1); assert!(matches!(r, Err(LinearError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&LinearStates::State3))); + assert!(matches!(sm.state(), &LinearStates::State3)); let r = sm.process_event(LinearEvents::Event2); assert!(matches!(r, Err(LinearError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&LinearStates::State3))); + assert!(matches!(sm.state(), &LinearStates::State3)); } diff --git a/examples/named_ex2.rs b/examples/named_ex2.rs index cbcfafe..b73df3f 100644 --- a/examples/named_ex2.rs +++ b/examples/named_ex2.rs @@ -23,7 +23,7 @@ impl LoopingStateMachineContext for Context {} fn main() { let mut sm = LoopingStateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&LoopingStates::State1))); + assert!(matches!(sm.state(), &LoopingStates::State1)); let r = sm.process_event(LoopingEvents::Event1); assert!(matches!(r, Ok(&LoopingStates::State2))); @@ -44,5 +44,5 @@ fn main() { // Now we cannot use Event1 again, as it is outside the state machine loop let r = sm.process_event(LoopingEvents::Event1); assert!(matches!(r, Err(LoopingError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&LoopingStates::State2))); + assert!(matches!(sm.state(), &LoopingStates::State2)); } diff --git a/examples/named_ex3.rs b/examples/named_ex3.rs index 4cacb2b..7ae0ce7 100644 --- a/examples/named_ex3.rs +++ b/examples/named_ex3.rs @@ -40,7 +40,7 @@ impl LoopingWithGuardsStateMachineContext for Context { fn main() { let mut sm = LoopingWithGuardsStateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&LoopingWithGuardsStates::State1))); + assert!(matches!(sm.state(), &LoopingWithGuardsStates::State1)); println!("Before action 1"); @@ -59,5 +59,5 @@ fn main() { println!("After action 2"); // Now we are stuck due to the guard never returning true - assert!(matches!(sm.state(), Ok(&LoopingWithGuardsStates::State2))); + assert!(matches!(sm.state(), &LoopingWithGuardsStates::State2)); } diff --git a/examples/named_input_state_pattern_match.rs b/examples/named_input_state_pattern_match.rs index cb5aead..edccc87 100644 --- a/examples/named_input_state_pattern_match.rs +++ b/examples/named_input_state_pattern_match.rs @@ -46,7 +46,7 @@ impl BatteryStateMachineContext for Context {} fn main() { let mut sm = BatteryStateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&BatteryStates::Idle))); + assert!(matches!(sm.state(), &BatteryStates::Idle)); let r = sm.process_event(BatteryEvents::Charge); assert!(matches!(r, Ok(&BatteryStates::Charging))); @@ -62,7 +62,7 @@ fn main() { let r = sm.process_event(BatteryEvents::Charge); assert!(matches!(r, Err(BatteryError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&BatteryStates::Charged))); + assert!(matches!(sm.state(), &BatteryStates::Charged)); let r = sm.process_event(BatteryEvents::Discharge); assert!(matches!(r, Ok(&BatteryStates::Discharging))); @@ -72,7 +72,7 @@ fn main() { let r = sm.process_event(BatteryEvents::Discharge); assert!(matches!(r, Err(BatteryError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&BatteryStates::Discharged))); + assert!(matches!(sm.state(), &BatteryStates::Discharged)); sm = BatteryStateMachine::new_with_state(Context, BatteryStates::Idle); let r = sm.process_event(BatteryEvents::FaultDetected); @@ -96,17 +96,17 @@ fn main() { let r = sm.process_event(BatteryEvents::Charge); assert!(matches!(r, Err(BatteryError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&BatteryStates::Fault))); + assert!(matches!(sm.state(), &BatteryStates::Fault)); let r = sm.process_event(BatteryEvents::Discharge); assert!(matches!(r, Err(BatteryError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&BatteryStates::Fault))); + assert!(matches!(sm.state(), &BatteryStates::Fault)); let r = sm.process_event(BatteryEvents::ChargeComplete); assert!(matches!(r, Err(BatteryError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&BatteryStates::Fault))); + assert!(matches!(sm.state(), &BatteryStates::Fault)); let r = sm.process_event(BatteryEvents::DischargeComplete); assert!(matches!(r, Err(BatteryError::InvalidEvent))); - assert!(matches!(sm.state(), Ok(&BatteryStates::Fault))); + assert!(matches!(sm.state(), &BatteryStates::Fault)); } diff --git a/examples/on_entry_on_exit_generic.rs b/examples/on_entry_on_exit_generic.rs index 608869b..b06f090 100644 --- a/examples/on_entry_on_exit_generic.rs +++ b/examples/on_entry_on_exit_generic.rs @@ -39,31 +39,31 @@ fn main() { // first event starts the dominos let _ = sm.process_event(OnEntryExampleEvents::ToD1).unwrap(); - assert!(matches!(sm.state(), Ok(&OnEntryExampleStates::D1))); + assert!(matches!(sm.state(), &OnEntryExampleStates::D1)); assert_eq!(sm.context().exited_d0, 1); assert_eq!(sm.context().entered_d1, 1); let _ = sm.process_event(OnEntryExampleEvents::ToD2).unwrap(); - assert!(matches!(sm.state(), Ok(&OnEntryExampleStates::D2))); + assert!(matches!(sm.state(), &OnEntryExampleStates::D2)); assert_eq!(sm.context().exited_d0, 1); assert_eq!(sm.context().entered_d1, 1); let _ = sm.process_event(OnEntryExampleEvents::ToD1).unwrap(); - assert!(matches!(sm.state(), Ok(&OnEntryExampleStates::D1))); + assert!(matches!(sm.state(), &OnEntryExampleStates::D1)); assert_eq!(sm.context().exited_d0, 1); assert_eq!(sm.context().entered_d1, 2); let _ = sm.process_event(OnEntryExampleEvents::ToD0).unwrap(); - assert!(matches!(sm.state(), Ok(&OnEntryExampleStates::D0))); + assert!(matches!(sm.state(), &OnEntryExampleStates::D0)); assert_eq!(sm.context().exited_d0, 1); assert_eq!(sm.context().entered_d1, 2); let _ = sm.process_event(OnEntryExampleEvents::ToD3).unwrap(); - assert!(matches!(sm.state(), Ok(&OnEntryExampleStates::D3))); + assert!(matches!(sm.state(), &OnEntryExampleStates::D3)); assert_eq!(sm.context().exited_d0, 2); assert_eq!(sm.context().entered_d1, 2); } diff --git a/examples/reuse_action.rs b/examples/reuse_action.rs index 8959374..46c401a 100644 --- a/examples/reuse_action.rs +++ b/examples/reuse_action.rs @@ -25,7 +25,7 @@ impl StateMachineContext for Context { fn main() { let mut sm = StateMachine::new(Context(0)); - assert!(matches!(sm.state(), Ok(&States::State1))); + assert!(matches!(sm.state(), &States::State1)); assert!(sm.context.0 == 0); // triggers action diff --git a/examples/state_machine_logger.rs b/examples/state_machine_logger.rs index 3441d67..3374627 100644 --- a/examples/state_machine_logger.rs +++ b/examples/state_machine_logger.rs @@ -46,7 +46,7 @@ impl StateMachineContext for Context { } // Action2 has access to the data from State2 - fn action2(&mut self, state_data: MyStateData) { + fn action2(&mut self, state_data: &MyStateData) { println!("Printing state data {:?}", state_data); } @@ -57,15 +57,11 @@ impl StateMachineContext for Context { ); } - fn log_guard(&self, guard: &'static str, result: &Result) { - if let Ok(result) = *result { - if result { - println!("[StateMachineLogger]\tEnabled `{}`", guard); - } else { - println!("[StateMachineLogger]\tDisabled `{}`", guard); - } + fn log_guard(&self, guard: &'static str, result: bool) { + if result { + println!("[StateMachineLogger]\tEnabled `{}`", guard); } else { - println!("[StateMachineLogger]\tFailed `{}`", guard); + println!("[StateMachineLogger]\tDisabled `{}`", guard); } } diff --git a/macros/src/codegen.rs b/macros/src/codegen.rs index 32589be..24cefa6 100644 --- a/macros/src/codegen.rs +++ b/macros/src/codegen.rs @@ -79,7 +79,7 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { } Some(_) => { quote! { - #state_name(state_data) + #state_name(ref state_data) } } } @@ -144,7 +144,7 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { }) .collect(); - let guard_action_parameters: Vec> = transitions + let action_parameters: Vec> = transitions .iter() .map(|(name, value)| { let state_name = &sm.states.get(name).unwrap().to_string(); @@ -153,7 +153,8 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { .iter() .map(|(name, _)| { let state_data = match sm.state_data.data_types.get(state_name) { - Some(_) => quote! { state_data }, + Some(Type::Reference(_)) => quote! { state_data }, + Some(_) => quote! { &state_data }, None => quote! {}, }; @@ -172,7 +173,7 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { }) .collect(); - let guard_action_ref_parameters: Vec> = transitions + let guard_parameters: Vec> = transitions .iter() .map(|(name, value)| { let state_name = &sm.states.get(name).unwrap().to_string(); @@ -360,14 +361,6 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { }) }; - let state_data = match sm.state_data.data_types.get(state) { - Some(st) => { - quote! { state_data: #st, } - } - None => { - quote! {} - } - }; let event_data = match sm.event_data.data_types.get(event) { Some(et) => { quote! { event_data: #et } @@ -408,18 +401,18 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { .zip( actions .iter() - .zip(in_states.iter().zip(out_states.iter().zip(guard_action_parameters.iter().zip(guard_action_ref_parameters.iter())))), + .zip(in_states.iter().zip(out_states.iter().zip(action_parameters.iter().zip(guard_parameters.iter())))), ) .map( - |(guards, (actions, (in_state, (out_states, (guard_action_parameters, guard_action_ref_parameters)))))| { + |(guards, (actions, (in_state, (out_states, (action_parameters, guard_parameters)))))| { guards .iter() .zip( actions .iter() - .zip(out_states.iter().zip(guard_action_parameters.iter().zip(guard_action_ref_parameters.iter()))), + .zip(out_states.iter().zip(action_parameters.iter().zip(guard_parameters.iter()))), ) - .map(|(guard, (action, (out_state, (g_a_param, g_a_ref_param))))| { + .map(|(guard, (action, (out_state, (action_params, guard_params))))| { let streams: Vec = guard.iter() .zip(action.iter().zip(out_state)).map(|(guard, (action,out_state))| { @@ -434,40 +427,30 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { let entry_exit_states = quote! { - self.context_mut().#exit_ident(); - self.context_mut().#entry_ident(); + self.context.#exit_ident(); + self.context.#entry_ident(); }; - let (is_async_action,action_code) = generate_action(action, &temporary_context_call, g_a_param); + let (is_async_action, action_code) = generate_action(action, &temporary_context_call, action_params); is_async_state_machine |= is_async_action; + if let Some(expr) = guard { // Guarded transition - let mut is_async_expression = false; let guard_expression= expr.to_token_stream(&mut |async_ident: &AsyncIdent| { let guard_ident = &async_ident.ident; - is_async_expression |= async_ident.is_async; - if async_ident.is_async { - quote! { - self.context.#guard_ident(#temporary_context_call #g_a_ref_param).await? - } + let guard_await = if async_ident.is_async { + is_async_state_machine = true; + quote! { .await } } else { - quote! { - self.context.#guard_ident(#temporary_context_call #g_a_ref_param)? - } - } - }); - is_async_state_machine |= is_async_expression; - let guard_result = if is_async_expression { - // This #guard_expression contains a boolean expression of async guard functions. - // Each guard function has Result return type. - // For example, [ async f && !async g ] will expand into - // self.context.f().await? && !self.context.g().await? - // We need to catch the error, if any of the functions returns it. - // Therefore, we use an async block to catch these .await? operator results - // and convert the result of the expression back to Result + quote! {} + }; quote! { - #[allow(clippy::needless_question_mark)] - let guard_result = (async{Ok(#guard_expression)}).await; + self.context.#guard_ident(#temporary_context_call #guard_params) #guard_await .map_err(#error_type_name::GuardFailed)? } - } else { + }); + + // After we assembled the result of the expression into 'guard_result' variable + // we now either check the guard Ok value to enable or disable the transition + // or repackage the Err value and propagate to the caller of process_event() + quote! { // This #guard_expression contains a boolean expression of guard functions // Each guard function has Result return type. // For example, [ f && !g ] will expand into @@ -476,40 +459,20 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { // Therefore, we wrap it with a closure and immediately call the closure // to catch the question mark operator results // and convert the result of the expression back to to Result - quote! { - #[allow(clippy::needless_question_mark)] - let guard_result = (||Ok(#guard_expression))(); + let guard_passed = #guard_expression; + self.context.log_guard(stringify!(#guard_expression), guard_passed); + + // If the guard passed, we transition immediately. + // Otherwise, there may be a later transition that passes, + // so we'll defer to that. + if guard_passed { + #action_code + let out_state = #states_type_name::#out_state; + self.context.log_state_change(&out_state); + #entry_exit_states + self.state = out_state; + return Ok(&self.state); } - }; - // After we assembled the result of the expression into 'guard_result' variable - // we now either check the guard Ok value to enable or disable the transition - // or repackage the Err value and propagate to the caller of process_event() - quote! { - #guard_result - self.context.log_guard(stringify!(#guard_expression), &guard_result); - - // If the guard fails, we need to fall back to our original - // state. - match guard_result { - Err(err) => { - self.state = Some(#states_type_name::#in_state); - return Err(#error_type_name::GuardFailed(err)); - } - - // If the guard passed, we transition immediately. - // Otherwise, there may be a later transition that passes, - // so we'll defer to that. - Ok(true) => { - #action_code - let out_state = #states_type_name::#out_state; - self.context.log_state_change(&out_state); - #entry_exit_states - self.state = Some(out_state); - return self.state(); - } - _ => {}, - } - } } else { // Unguarded transition quote!{ @@ -517,8 +480,8 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { let out_state = #states_type_name::#out_state; self.context.log_state_change(&out_state); #entry_exit_states - self.state = Some(out_state); - return self.state(); + self.state = out_state; + return Ok(&self.state); } } } @@ -541,7 +504,7 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { Some(st) => quote! { pub const fn new(context: T, state_data: #st ) -> Self { #state_machine_type_name { - state: Some(#states_type_name::#starting_state (state_data)), + state: #states_type_name::#starting_state (state_data), context } } @@ -549,7 +512,7 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { None => quote! { pub const fn new(context: T ) -> Self { #state_machine_type_name { - state: Some(#states_type_name::#starting_state), + state: #states_type_name::#starting_state, context } } @@ -571,12 +534,6 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { quote! {} }; - let guard_error_type = if sm.custom_guard_error { - quote! { Self::GuardError } - } else { - quote! { () } - }; - let (is_async, is_async_trait) = if is_async_state_machine { (quote! { async }, quote! { #[smlang::async_trait] }) } else { @@ -613,7 +570,7 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { /// Called after executing a guard during `process_event()`. No-op by /// default but can be overridden in implementations of a state machine's /// `StateMachineContext` trait. - fn log_guard(&self, guard: &'static str, result: &Result) {} + fn log_guard(&self, guard: &'static str, result: bool) {} /// Called after executing an action during `process_event()`. No-op by /// default but can be overridden in implementations of a state machine's @@ -661,16 +618,11 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { TransitionsFailed, /// When guard is failed. GuardFailed(T), - /// When the state has an unexpected value. - /// - /// This can happen if there is a bug in the code generated by smlang, - /// or if a guard or action gets panicked. - Poisoned, } /// State machine structure definition. pub struct #state_machine_type_name<#state_lifetimes T: #state_machine_context_type_name> { - state: Option<#states_type_name <#state_lifetimes>>, + state: #states_type_name <#state_lifetimes>, context: T } @@ -683,15 +635,15 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { #[inline(always)] pub const fn new_with_state(context: T, initial_state: #states_type_name <#state_lifetimes>) -> Self { #state_machine_type_name { - state: Some(initial_state), + state: initial_state, context } } /// Returns the current state. #[inline(always)] - pub fn state(&self) -> Result<&#states_type_name <#state_lifetimes>, #error_type> { - self.state.as_ref().ok_or(#error_type_name ::Poisoned) + pub fn state(&self) -> &#states_type_name <#state_lifetimes> { + &self.state } /// Returns the current context. @@ -715,8 +667,8 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { #temporary_context event: #events_type_name <#event_lifetimes> ) -> Result<&#states_type_name <#state_lifetimes>, #error_type> { - self.context.log_process_event(self.state()?, &event); - match self.state.take().ok_or(#error_type_name ::Poisoned)? { + self.context.log_process_event(self.state(), &event); + match self.state { #( #[allow(clippy::match_single_binding)] #states_type_name::#in_states => match event { @@ -726,16 +678,11 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { #[allow(unreachable_code)] { // none of the guarded or non-guarded transitions occurred, - // therefore return an error, and keep the same state - self.state = Some(#states_type_name::#in_states); Err(#error_type_name ::TransitionsFailed) } }),* #[allow(unreachable_patterns)] - _ => { - self.state = Some(#states_type_name::#in_states); - Err(#error_type_name ::InvalidEvent) - } + _ => Err(#error_type_name ::InvalidEvent), }),* } } diff --git a/macros/src/lib.rs b/macros/src/lib.rs index cbffd84..c85dba1 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -8,6 +8,7 @@ mod diagramgen; mod parser; mod validation; +use std::io::Write; use syn::parse_macro_input; // dot -Tsvg statemachine.gv -o statemachine.svg @@ -28,7 +29,7 @@ pub fn statemachine(input: proc_macro::TokenStream) -> proc_macro::TokenStream { // Generate dot syntax for the statemachine. let diagram = diagramgen::generate_diagram(&sm); - let diagram_name = if let Some(name) = &sm.name { + let name = if let Some(name) = &sm.name { name.to_string() } else { let mut diagram_hasher = std::collections::hash_map::DefaultHasher::new(); @@ -38,7 +39,7 @@ pub fn statemachine(input: proc_macro::TokenStream) -> proc_macro::TokenStream { // Start the 'dot' process. let mut process = std::process::Command::new("dot") - .args(["-Tsvg", "-o", &format!("statemachine_{diagram_name}.svg")]) + .args(["-Tsvg", "-o", &format!("statemachine_{name}.svg")]) .stdin(std::process::Stdio::piped()) .spawn() .expect("Failed to execute 'dot'. Are you sure graphviz is installed?"); @@ -65,7 +66,19 @@ pub fn statemachine(input: proc_macro::TokenStream) -> proc_macro::TokenStream { return e.to_compile_error().into(); } - codegen::generate_code(&sm).into() + let tokens = codegen::generate_code(&sm).into(); + // Write expanded state machine to a file for reference. + let name = if let Some(name) = &sm.name { + name.to_string() + } else { + "default".to_string() + }; + let mut expansion_file = + std::fs::File::create(format!("target/smlang-expansion-{name}.rs")).unwrap(); + expansion_file + .write_all(format!("{tokens}").as_bytes()) + .unwrap(); + tokens } Err(error) => error.to_compile_error().into(), } diff --git a/tests/test.rs b/tests/test.rs index c139f9f..7fe80b3 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -24,10 +24,10 @@ fn wildcard_after_input_state() { let mut sm = StateMachine::new(Context); sm.process_event(Events::Event1).unwrap(); - assert!(matches!(sm.state(), Ok(&States::State2))); + assert!(matches!(sm.state(), &States::State2)); sm.process_event(Events::Event1).unwrap(); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); } #[test] @@ -92,13 +92,13 @@ fn derive_display_events_states() { impl StateMachineContext for Context {} let mut sm = StateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); let event = Events::Event; assert_eq!(format!("{}", event), "Event"); sm.process_event(event).unwrap(); - assert!(matches!(sm.state(), Ok(&States::End))); + assert!(matches!(sm.state(), &States::End)); } #[test] @@ -116,13 +116,13 @@ fn named_derive_display_events_states() { impl SMStateMachineContext for Context {} let mut sm = SMStateMachine::new(Context); - assert!(matches!(sm.state(), Ok(&SMStates::Init))); + assert!(matches!(sm.state(), &SMStates::Init)); let event = SMEvents::Event; assert_eq!(format!("{}", event), "Event"); sm.process_event(event).unwrap(); - assert!(matches!(sm.state(), Ok(&SMStates::End))); + assert!(matches!(sm.state(), &SMStates::End)); } #[test] @@ -152,10 +152,10 @@ fn async_guards_and_actions() { let mut sm = StateMachine::new(Context); sm.process_event(Events::Event1).await.unwrap(); - assert!(matches!(sm.state(), Ok(&States::State2))); + assert!(matches!(sm.state(), &States::State2)); sm.process_event(Events::Event1).await.unwrap(); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); }); } @@ -198,37 +198,37 @@ fn guard_expressions() { password: 42, attempts: 0, }); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); let bad_entry = Entry(10); let good_entry = Entry(42); let _ = sm.process_event(Events::Login(&bad_entry)); assert_eq!(sm.context().attempts, 1); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); let _ = sm.process_event(Events::Login(&bad_entry)); assert_eq!(sm.context().attempts, 2); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); let _ = sm.process_event(Events::Login(&good_entry)); assert_eq!(sm.context().attempts, 3); - assert!(matches!(sm.state(), Ok(&States::LoggedIn))); + assert!(matches!(sm.state(), &States::LoggedIn)); let _ = sm.process_event(Events::Logout); assert_eq!(sm.context().attempts, 0); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); let _ = sm.process_event(Events::Login(&bad_entry)); let _ = sm.process_event(Events::Login(&bad_entry)); let _ = sm.process_event(Events::Login(&bad_entry)); assert_eq!(sm.context().attempts, 3); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); // exhausted attempts let _ = sm.process_event(Events::Login(&bad_entry)); assert_eq!(sm.context().attempts, 4); - assert!(matches!(sm.state(), Ok(&States::LoginDenied))); + assert!(matches!(sm.state(), &States::LoginDenied)); // Invalid event, as we are in a final state assert_eq!( @@ -236,7 +236,7 @@ fn guard_expressions() { Err(Error::InvalidEvent) ); assert_eq!(sm.context().attempts, 4); - assert!(matches!(sm.state(), Ok(&States::LoginDenied))); + assert!(matches!(sm.state(), &States::LoginDenied)); } #[test] fn guarded_transition_before_unguarded() { @@ -262,13 +262,13 @@ fn guarded_transition_before_unguarded() { } let mut sm = StateMachine::new(Context { enabled: true }); sm.process_event(Events::Event1).unwrap(); - assert!(matches!(sm.state(), Ok(&States::State2))); + assert!(matches!(sm.state(), &States::State2)); sm.process_event(Events::Event1).unwrap(); - assert!(matches!(sm.state(), Ok(&States::State1))); + assert!(matches!(sm.state(), &States::State1)); sm.process_event(Events::Event1).unwrap(); - assert!(matches!(sm.state(), Ok(&States::Fault))); + assert!(matches!(sm.state(), &States::Fault)); } #[test] @@ -302,17 +302,17 @@ fn guard_errors() { // Test attempting to transition when the guard fails. sm.context_mut().guard_errors = true; assert!(sm.process_event(Events::Event1).is_err()); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); // Test attempting to transition when the guard is not passable. sm.context_mut().guard_errors = false; assert!(sm.process_event(Events::Event1).is_err()); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); assert!(sm.process_event(Events::Event1).is_err()); - assert!(matches!(sm.state(), Ok(&States::Init))); + assert!(matches!(sm.state(), &States::Init)); sm.context_mut().guard_passable = true; sm.process_event(Events::Event1).unwrap(); - assert!(matches!(sm.state(), Ok(&States::Done))); + assert!(matches!(sm.state(), &States::Done)); } From cc6ff09f578c1283d2d481ff6b75db04aff25702 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 1 Jul 2024 10:48:36 +0200 Subject: [PATCH 4/7] cleaning up docs --- CHANGELOG.md | 2 ++ macros/src/codegen.rs | 7 ------- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbeb0b7..faf5582 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - There are now `on_entry_` and `on_entry_` functions defined to allow handling entry and exit from all state machine states. These have a default empty implementation. +* The expanded code is now written to `target/smlang-expansion-.rs` during the build +process. ### Fixed diff --git a/macros/src/codegen.rs b/macros/src/codegen.rs index 24cefa6..2b02d96 100644 --- a/macros/src/codegen.rs +++ b/macros/src/codegen.rs @@ -447,18 +447,11 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { } }); - // After we assembled the result of the expression into 'guard_result' variable - // we now either check the guard Ok value to enable or disable the transition - // or repackage the Err value and propagate to the caller of process_event() quote! { // This #guard_expression contains a boolean expression of guard functions // Each guard function has Result return type. // For example, [ f && !g ] will expand into // self.context.f()? && !self.context.g()? - // We need to catch the error, if any of the functions returns it. - // Therefore, we wrap it with a closure and immediately call the closure - // to catch the question mark operator results - // and convert the result of the expression back to to Result let guard_passed = #guard_expression; self.context.log_guard(stringify!(#guard_expression), guard_passed); From 8be589e237e894a7a343ee5ab1af97bab8c56aa7 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 1 Jul 2024 10:51:58 +0200 Subject: [PATCH 5/7] Removing expansion --- macros/src/lib.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index c85dba1..55fbfdb 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -66,19 +66,7 @@ pub fn statemachine(input: proc_macro::TokenStream) -> proc_macro::TokenStream { return e.to_compile_error().into(); } - let tokens = codegen::generate_code(&sm).into(); - // Write expanded state machine to a file for reference. - let name = if let Some(name) = &sm.name { - name.to_string() - } else { - "default".to_string() - }; - let mut expansion_file = - std::fs::File::create(format!("target/smlang-expansion-{name}.rs")).unwrap(); - expansion_file - .write_all(format!("{tokens}").as_bytes()) - .unwrap(); - tokens + codegen::generate_code(&sm).into() } Err(error) => error.to_compile_error().into(), } From cec769f8030ef8af798a8bdb33a0ddf183a58341 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 1 Jul 2024 10:53:04 +0200 Subject: [PATCH 6/7] fixing clippy --- macros/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 55fbfdb..33af6ff 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -8,7 +8,6 @@ mod diagramgen; mod parser; mod validation; -use std::io::Write; use syn::parse_macro_input; // dot -Tsvg statemachine.gv -o statemachine.svg From f1a4534dd0a0e2ca00ca571de07c786b15f264a8 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 1 Jul 2024 10:54:06 +0200 Subject: [PATCH 7/7] Reverting macros lib file --- macros/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 33af6ff..cbffd84 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -28,7 +28,7 @@ pub fn statemachine(input: proc_macro::TokenStream) -> proc_macro::TokenStream { // Generate dot syntax for the statemachine. let diagram = diagramgen::generate_diagram(&sm); - let name = if let Some(name) = &sm.name { + let diagram_name = if let Some(name) = &sm.name { name.to_string() } else { let mut diagram_hasher = std::collections::hash_map::DefaultHasher::new(); @@ -38,7 +38,7 @@ pub fn statemachine(input: proc_macro::TokenStream) -> proc_macro::TokenStream { // Start the 'dot' process. let mut process = std::process::Command::new("dot") - .args(["-Tsvg", "-o", &format!("statemachine_{name}.svg")]) + .args(["-Tsvg", "-o", &format!("statemachine_{diagram_name}.svg")]) .stdin(std::process::Stdio::piped()) .spawn() .expect("Failed to execute 'dot'. Are you sure graphviz is installed?");