bugfix: don't do things with side-effects in assertins

Author: cmnrd

include/reactor-cpp/assert.hh

@@ -33,17 +33,8 @@ constexpr bool runtime_assertion = true;
 #include <unistd.h>
 #endif
 
-// assert macro that avoids unused variable warnings
-#ifdef NDEBUG
-// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
-#define reactor_assert(x)                                                                                              \
-  do {                                                                                                                 \
-    (void)sizeof(x);                                                                                                   \
-  } while (0)
-#else
 // NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define reactor_assert(x) assert(x)
-#endif
 
 namespace reactor {
 using EnvPhase = Environment::Phase;

include/reactor-cpp/connection.hh

@@ -48,7 +48,10 @@ public:
     port->register_set_callback(upstream_set_callback());
   }
 
-  virtual void bind_downstream_port(Port<T>* port) { reactor_assert(this->downstream_ports_.insert(port).second); };
+  virtual void bind_downstream_port(Port<T>* port) {
+    [[maybe_unused]] bool result = this->downstream_ports_.insert(port).second;
+    reactor_assert(result);
+  };
 };
 
 template <class T> class BaseDelayedConnection : public Connection<T> {

lib/action.cc

@@ -22,7 +22,8 @@ void BaseAction::register_trigger(Reaction* reaction) {
   validate(this->container() == reaction->container(),
            "Action triggers must belong to the same reactor as the triggered "
            "reaction");
-  reactor_assert(triggers_.insert(reaction).second);
+  [[maybe_unused]] bool result = triggers_.insert(reaction).second;
+  reactor_assert(result);
 }
 
 void BaseAction::register_scheduler(Reaction* reaction) {
@@ -32,7 +33,8 @@ void BaseAction::register_scheduler(Reaction* reaction) {
   // the reaction must belong to the same reactor as this action
   validate(this->container() == reaction->container(), "Scheduable actions must belong to the same reactor as the "
                                                        "triggered reaction");
-  reactor_assert(schedulers_.insert(reaction).second);
+  [[maybe_unused]] bool result = schedulers_.insert(reaction).second;
+  reactor_assert(result);
 }
 
 void Timer::startup() {

lib/environment.cc

@@ -40,20 +40,23 @@ Environment::Environment(const std::string& name, Environment* containing_enviro
     , top_environment_(containing_environment_->top_environment_)
     , scheduler_(this)
     , timeout_(containing_environment->timeout()) {
-  reactor_assert(containing_environment->contained_environments_.insert(this).second);
+  [[maybe_unused]] bool result = containing_environment->contained_environments_.insert(this).second;
+  reactor_assert(result);
 }
 
 void Environment::register_reactor(Reactor* reactor) {
   reactor_assert(reactor != nullptr);
   validate(this->phase() == Phase::Construction, "Reactors may only be registered during construction phase!");
   validate(reactor->is_top_level(), "The environment may only contain top level reactors!");
-  reactor_assert(top_level_reactors_.insert(reactor).second);
+  [[maybe_unused]] bool result = top_level_reactors_.insert(reactor).second;
+  reactor_assert(result);
 }
 
 void Environment::register_input_action(BaseAction* action) {
   reactor_assert(action != nullptr);
   validate(this->phase() == Phase::Construction, "Input actions may only be registered during construction phase!");
-  reactor_assert(input_actions_.insert(action).second);
+  [[maybe_unused]] bool result = input_actions_.insert(action).second;
+  reactor_assert(result);
   run_forever_ = true;
 }
 

lib/port.cc

@@ -16,7 +16,7 @@ namespace reactor {
 
 void BasePort::base_bind_to(BasePort* port) {
   reactor_assert(port != nullptr);
-  reactor_assert(this->environment() == port->environment()); // NOLINT
+  reactor_assert(this->environment() == port->environment());
   validate(!port->has_inward_binding(), "Ports may only be connected once");
   validate(!port->has_anti_dependencies(), "Ports with anti dependencies may not be connected to other ports");
   assert_phase(this, Environment::Phase::Assembly);
@@ -41,12 +41,13 @@ void BasePort::base_bind_to(BasePort* port) {
   }
 
   port->inward_binding_ = this;
-  reactor_assert(this->outward_bindings_.insert(port).second);
+  [[maybe_unused]] bool result = this->outward_bindings_.insert(port).second;
+  reactor_assert(result);
 }
 
 void BasePort::register_dependency(Reaction* reaction, bool is_trigger) noexcept {
   reactor_assert(reaction != nullptr);
-  reactor_assert(this->environment() == reaction->environment()); // NOLINT
+  reactor_assert(this->environment() == reaction->environment());
   validate(!this->has_outward_bindings(), "Dependencies may no be declared on ports with an outward binding!");
   assert_phase(this, Environment::Phase::Assembly);
 
@@ -58,15 +59,17 @@ void BasePort::register_dependency(Reaction* reaction, bool is_trigger) noexcept
              "Dependent output ports must belong to a contained reactor");
   }
 
-  reactor_assert(dependencies_.insert(reaction).second);
+  [[maybe_unused]] bool result = dependencies_.insert(reaction).second;
+  reactor_assert(result);
   if (is_trigger) {
-    reactor_assert(triggers_.insert(reaction).second);
+    result = triggers_.insert(reaction).second;
+    reactor_assert(result);
   }
 }
 
 void BasePort::register_antidependency(Reaction* reaction) noexcept {
   reactor_assert(reaction != nullptr);
-  reactor_assert(this->environment() == reaction->environment()); // NOLINT
+  reactor_assert(this->environment() == reaction->environment());
   validate(!this->has_inward_binding(), "Antidependencies may no be declared on ports with an inward binding!");
   assert_phase(this, Environment::Phase::Assembly);
 
@@ -79,7 +82,8 @@ void BasePort::register_antidependency(Reaction* reaction) noexcept {
              "Antidependent input ports must belong to a contained reactor");
   }
 
-  reactor_assert(anti_dependencies_.insert(reaction).second);
+  [[maybe_unused]] bool result = anti_dependencies_.insert(reaction).second;
+  reactor_assert(result);
 }
 
 [[maybe_unused]] auto Port<void>::typed_outward_bindings() const noexcept -> const std::set<Port<void>*>& {

lib/reaction.cc

@@ -31,7 +31,8 @@ void Reaction::declare_trigger(BaseAction* action) {
   validate(this->container() == action->container(), "Action triggers must belong to the same reactor as the triggered "
                                                      "reaction");
 
-  reactor_assert(action_triggers_.insert(action).second);
+  [[maybe_unused]] bool result = action_triggers_.insert(action).second;
+  reactor_assert(result);
   action->register_trigger(this);
 }
 
@@ -42,7 +43,8 @@ void Reaction::declare_schedulable_action(BaseAction* action) {
   validate(this->container() == action->container(), "Scheduable actions must belong to the same reactor as the "
                                                      "triggered reaction");
 
-  reactor_assert(scheduable_actions_.insert(action).second);
+  [[maybe_unused]] bool result = scheduable_actions_.insert(action).second;
+  reactor_assert(result);
   action->register_scheduler(this);
 }
 
@@ -61,8 +63,10 @@ void Reaction::declare_trigger(BasePort* port) {
              "Output port triggers must belong to a contained reactor");
   }
 
-  reactor_assert(port_trigger_.insert(port).second);
-  reactor_assert(dependencies_.insert(port).second);
+  [[maybe_unused]] bool result = port_trigger_.insert(port).second;
+  reactor_assert(result);
+  result = dependencies_.insert(port).second;
+  reactor_assert(result);
   port->register_dependency(this, true);
 }
 
@@ -79,7 +83,8 @@ void Reaction::declare_dependency(BasePort* port) {
              "Dependent output ports must belong to a contained reactor");
   }
 
-  reactor_assert(dependencies_.insert(port).second);
+  [[maybe_unused]] bool result = dependencies_.insert(port).second;
+  reactor_assert(result);
   port->register_dependency(this, false);
 }
 
@@ -96,7 +101,8 @@ void Reaction::declare_antidependency(BasePort* port) {
              "Antidependent input ports must belong to a contained reactor");
   }
 
-  reactor_assert(antidependencies_.insert(port).second);
+  [[maybe_unused]] bool result = antidependencies_.insert(port).second;
+  reactor_assert(result);
   port->register_antidependency(this);
 }
 

lib/reactor.cc

@@ -80,36 +80,41 @@ void Reactor::register_action([[maybe_unused]] BaseAction* action) {
   reactor::validate(this->environment()->phase() == Environment::Phase::Construction ||
                         this->environment()->phase() == Environment::Phase::Assembly,
                     "Actions can only be registered during construction phase!");
-  reactor_assert(actions_.insert(action).second);
+  [[maybe_unused]] bool result = actions_.insert(action).second;
+  reactor_assert(result);
 }
 
 void Reactor::register_input(BasePort* port) {
   reactor_assert(port != nullptr);
   reactor::validate(this->environment()->phase() == Environment::Phase::Construction,
                     "Ports can only be registered during construction phase!");
-  reactor_assert(inputs_.insert(port).second);
+  [[maybe_unused]] bool result = inputs_.insert(port).second;
+  reactor_assert(result);
 }
 
 void Reactor::register_output(BasePort* port) {
   reactor_assert(port != nullptr);
   reactor::validate(this->environment()->phase() == Environment::Phase::Construction,
                     "Ports can only be registered during construction phase!");
-  reactor_assert(outputs_.insert(port).second);
+  [[maybe_unused]] bool result = inputs_.insert(port).second;
+  reactor_assert(result);
 }
 
 void Reactor::register_reaction([[maybe_unused]] Reaction* reaction) {
   reactor_assert(reaction != nullptr);
 
   validate(this->environment()->phase() == Environment::Phase::Construction,
            "Reactions can only be registered during construction phase!");
-  reactor_assert(reactions_.insert(reaction).second);
+  [[maybe_unused]] bool result = reactions_.insert(reaction).second;
+  reactor_assert(result);
 }
 
 void Reactor::register_reactor([[maybe_unused]] Reactor* reactor) {
   reactor_assert(reactor != nullptr);
   validate(this->environment()->phase() == Environment::Phase::Construction,
            "Reactions can only be registered during construction phase!");
-  reactor_assert(reactors_.insert(reactor).second);
+  [[maybe_unused]] bool result = reactors_.insert(reactor).second;
+  reactor_assert(result);
 }
 
 void Reactor::startup() {