runtime: Take string arguments by value where reasonable

This simplifies the code as well as providing a more efficient
implementation for string instantiation.

Author: cassava

runtime/include/cloe/core/error.hpp

@@ -18,22 +18,23 @@
 /**
  * \file cloe/core/error.hpp
  * \see  cloe/core/error.cpp
- * \see  cloe/core.hpp
  */
 
 #pragma once
 
-#include <stdexcept>  // for exception
-#include <string>     // for string
+#include <stdexcept>    // for exception
+#include <string>       // for string
+#include <string_view>  // for string
 
-#include <cloe/core/logger.hpp>  // for fmt::format
+#include <fmt/format.h>  // for fmt::format
 
 namespace cloe {
 
+class Error;
+
 class Error : public std::exception {
  public:
-  explicit Error(const std::string& what) : err_(what) {}
-  explicit Error(const char* what) : err_(what) {}
+  Error(std::string_view what) : err_(what.data()) {}
 
   template <typename... Args>
   Error(std::string_view format, Args&&... args)
@@ -45,7 +46,7 @@ class Error : public std::exception {
 
   bool has_explanation() const { return !explanation_.empty(); }
 
-  void set_explanation(const std::string& explanation);
+  void set_explanation(std::string explanation);
 
   template <typename... Args>
   void set_explanation(std::string_view format, Args&&... args) {
@@ -54,8 +55,8 @@ class Error : public std::exception {
 
   const std::string& explanation() const { return explanation_; }
 
-  Error explanation(const std::string& explanation) && {
-    set_explanation(explanation);
+  Error explanation(std::string explanation) && {
+    set_explanation(std::move(explanation));
     return std::move(*this);
   }
 

runtime/include/cloe/entity.hpp

@@ -34,8 +34,8 @@ namespace cloe {
  */
 class InvalidNameError : public Error {
  public:
-  explicit InvalidNameError(const std::string& name)
-      : Error("name is invalid: " + name), name_(name) {}
+  explicit InvalidNameError(std::string name)
+      : Error("name is invalid: " + name), name_(std::move(name)) {}
   virtual ~InvalidNameError() noexcept = default;
 
   const std::string& name() const { return name_; }
@@ -49,22 +49,22 @@ class InvalidNameError : public Error {
  */
 class Entity {
  public:
-  explicit Entity(const std::string& name) {
-    set_name(name);
+  explicit Entity(std::string name) {
+    set_name(std::move(name));
     set_description("");
   }
 
-  Entity(const std::string& name, const std::string& desc) {
-    set_name(name);
-    set_description(desc);
+  Entity(std::string name, std::string desc) {
+    set_name(std::move(name));
+    set_description(std::move(desc));
   }
 
   virtual ~Entity() noexcept = default;
 
   /**
    * Return the name of the Entity.
    */
-  std::string name() const { return name_; }
+  const std::string& name() const { return name_; }
 
   /**
    * Set the name of the Entity.
@@ -80,16 +80,19 @@ class Entity {
    *   start
    *   _/strange_but_0k
    */
-  void set_name(const std::string& name);
+  void set_name(std::string name);
 
   /**
    * Return the optional description of the Entity.
    *
    * If there is no description, the empty string is returned.
    */
-  std::string description() const { return desc_; }
+  const std::string& description() const { return desc_; }
 
-  void set_description(const std::string& desc) { desc_ = desc; }
+  /**
+   * Set the free-form description of the entity.
+   */
+  void set_description(std::string desc) { desc_ = std::move(desc); }
 
  protected:
   /**

runtime/include/cloe/model.hpp

@@ -48,17 +48,14 @@
 
 #pragma once
 
-#include <fable/confable.hpp> // for Confable
+#include <fable/confable.hpp>  // for Confable
 
-#include <cloe/core.hpp>    // for Duration, Error
-#include <cloe/entity.hpp>  // for Entity
+#include <cloe/cloe_fwd.hpp>  // for Sync, Registrar
+#include <cloe/core.hpp>      // for Duration, Error
+#include <cloe/entity.hpp>    // for Entity
 
 namespace cloe {
 
-// Forward declaration:
-class Sync;       // from cloe/sync.hpp
-class Registrar;  // from cloe/registrar.hpp
-
 /**
  * ModelError indicates that an error in a model has occurred.
  */
@@ -67,16 +64,14 @@ class ModelError : public Error {
   using Error::Error;
   virtual ~ModelError() noexcept = default;
 
-  const std::string& explanation() const { return Error::explanation(); }
-
-  ModelError explanation(const std::string& explanation) && {
-    this->set_explanation(explanation);
+  ModelError explanation(std::string explanation) && {
+    this->set_explanation(std::move(explanation));
     return std::move(*this);
   }
 
   template <typename... Args>
-  ModelError explanation(std::string_view format, const Args&... args) && {
-    this->set_explanation(fmt::format(fmt::runtime(format), args...));
+  ModelError explanation(std::string_view format, Args&&... args) && {
+    this->set_explanation(fmt::format(fmt::runtime(format), std::forward<Args>(args)...));
     return std::move(*this);
   }
 };
@@ -92,16 +87,14 @@ class ModelAbort : public ModelError {
   using ModelError::ModelError;
   virtual ~ModelAbort() noexcept = default;
 
-  const std::string& explanation() const { return Error::explanation(); }
-
-  ModelError explanation(const std::string& explanation) && {
-    this->set_explanation(explanation);
+  ModelAbort explanation(std::string explanation) && {
+    this->set_explanation(std::move(explanation));
     return std::move(*this);
   }
 
   template <typename... Args>
-  ModelAbort explanation(std::string_view format, const Args&... args) && {
-    this->set_explanation(fmt::format(fmt::runtime(format), args...));
+  ModelAbort explanation(std::string_view format, Args&&... args) && {
+    this->set_explanation(fmt::format(fmt::runtime(format), std::forward<Args>(args)...));
     return std::move(*this);
   }
 };
@@ -115,16 +108,14 @@ class ModelReset : public ModelError {
   using ModelError::ModelError;
   virtual ~ModelReset() noexcept = default;
 
-  const std::string& explanation() const { return Error::explanation(); }
-
-  ModelError explanation(const std::string& explanation) && {
-    this->set_explanation(explanation);
+  ModelReset explanation(std::string explanation) && {
+    this->set_explanation(std::move(explanation));
     return std::move(*this);
   }
 
   template <typename... Args>
-  ModelReset explanation(std::string_view format, const Args&... args) && {
-    this->set_explanation(fmt::format(fmt::runtime(format), args...));
+  ModelReset explanation(std::string_view format, Args&&... args) && {
+    this->set_explanation(fmt::format(fmt::runtime(format), std::forward(args)...));
     return std::move(*this);
   }
 };
@@ -138,16 +129,14 @@ class ModelStop : public ModelError {
   using ModelError::ModelError;
   virtual ~ModelStop() noexcept = default;
 
-  const std::string& explanation() const { return Error::explanation(); }
-
-  ModelError explanation(const std::string& explanation) && {
-    this->set_explanation(explanation);
+  ModelStop explanation(std::string explanation) && {
+    this->set_explanation(std::move(explanation));
     return std::move(*this);
   }
 
   template <typename... Args>
-  ModelStop explanation(std::string_view format, const Args&... args) && {
-    this->set_explanation(fmt::format(fmt::runtime(format), args...));
+  ModelStop explanation(std::string_view format, Args&&... args) && {
+    this->set_explanation(fmt::format(fmt::runtime(format), std::forward<Args>(args)...));
     return std::move(*this);
   }
 };

runtime/include/cloe/trigger.hpp

@@ -90,7 +90,7 @@ struct InlineSchema {
    * Construct an implicit inline schema if enabled is true, i.e., one where
    * only the name itself is sufficient.
    *
-   * If possible, it is recommended to use InlineSchema(std::string&&) instead.
+   * If possible, it is recommended to use InlineSchema(std::string) instead.
    * This constructor is primarily useful when you want to explicitly disable
    * an inline schema.
    */
@@ -99,14 +99,14 @@ struct InlineSchema {
   /**
    * Construct an implicit inline schema with the given description.
    */
-  explicit InlineSchema(std::string&& desc) : required_(false), desc_(std::move(desc)) {}
+  explicit InlineSchema(std::string desc) : required_(false), desc_(std::move(desc)) {}
 
   /**
    * Construct an inline schema that takes a particular primitive type.
    *
    * The type may not be null, object, or array.
    */
-  InlineSchema(std::string&& desc, fable::JsonType type, bool required = true);
+  InlineSchema(std::string desc, fable::JsonType type, bool required = true);
 
   /**
    * Construct an inline schema that takes a string with the given format.
@@ -131,7 +131,7 @@ struct InlineSchema {
    * ambiguity but should be used sparingly (for now, as usage is often
    * directly read from JSON).
    */
-  InlineSchema(std::string&& desc, std::string&& format, bool required = true)
+  InlineSchema(std::string desc, std::string format, bool required = true)
       : type_(fable::JsonType::string)
       , required_(required)
       , usage_(std::move(format))
@@ -221,52 +221,42 @@ struct TriggerSchema {
    *      "name": "stop"
    *    }
    */
-  TriggerSchema(const std::string& name, const std::string& desc)
-      : name_(name), schema_(std::string(desc)), inline_(true) {}
+  TriggerSchema(std::string name, std::string desc)
+      : name_(std::move(name)), schema_(std::move(desc)), inline_(true) {}
 
   /**
    * Construct a TriggerSchema that describes a trigger with parameters
    * but no inline format.
    */
-  TriggerSchema(const std::string& name, const std::string& desc,
-                fable::schema::PropertyList<> props)
-      : name_(name), schema_(std::string(desc), props), inline_(false) {}
+  TriggerSchema(std::string name, std::string desc, fable::schema::PropertyList<> props)
+      : name_(std::move(name)), schema_(std::move(desc), std::move(props)), inline_(false) {}
 
   /**
    * Construct a TriggerSchema that describes a trigger with the given Schema
    * but no inline format.
    */
-  TriggerSchema(const std::string& name, const std::string& desc, Schema&& s)
-      : name_(name), schema_(std::move(s)), inline_(false) {
-    schema_.set_description(desc);
+  TriggerSchema(std::string name, std::string desc, Schema s)
+      : name_(std::move(name)), schema_(std::move(s)), inline_(false) {
+    schema_.set_description(std::move(desc));
   }
 
   /**
    * Construct a TriggerSchema that describes a trigger with parameters
    * and a specified inline format.
    */
-  TriggerSchema(const std::string& name, const std::string& desc, InlineSchema&& usage,
+  TriggerSchema(std::string name, std::string desc, InlineSchema usage,
                 fable::schema::PropertyList<> props)
-      : name_(name), schema_(std::string(desc), props), inline_(std::move(usage)) {}
-
-  /**
-   * Construct a TriggerSchema that describes a trigger with the given Schema
-   * and a specified inline format.
-   */
-  TriggerSchema(const std::string& name, const std::string& desc, InlineSchema&& usage,
-                Schema&& init)
-      : name_(name), schema_(std::move(init)), inline_(std::move(usage)) {
-    schema_.set_description(desc);
-  }
+      : name_(std::move(name))
+      , schema_(std::move(desc), std::move(props))
+      , inline_(std::move(usage)) {}
 
   /**
    * Construct a TriggerSchema that describes a trigger with the given Schema
    * and a specified inline format.
    */
-  TriggerSchema(const std::string& name, const std::string& desc, InlineSchema&& usage,
-                const Schema& init)
-      : name_(name), schema_(init), inline_(std::move(usage)) {
-    schema_.set_description(desc);
+  TriggerSchema(std::string name, std::string desc, InlineSchema usage, Schema init)
+      : name_(std::move(name)), schema_(std::move(init)), inline_(std::move(usage)) {
+    schema_.set_description(std::move(desc));
   }
 
   const std::string& name() const { return name_; }

runtime/include/cloe/utility/tcp_transceiver_config.hpp

@@ -81,9 +81,7 @@ struct TcpTransceiverConfiguration : public Confable {
  */
 struct TcpTransceiverFullConfiguration : public TcpTransceiverConfiguration {
   TcpTransceiverFullConfiguration() = default;
-  TcpTransceiverFullConfiguration(const std::string& host, uint16_t port)
-      : host(host), port(port) {}
-  TcpTransceiverFullConfiguration(std::string&& host, uint16_t port)
+  TcpTransceiverFullConfiguration(std::string host, uint16_t port)
       : host(std::move(host)), port(port) {}
 
   /**

runtime/src/cloe/core/error.cpp

@@ -28,13 +28,13 @@
 
 namespace cloe {
 
-void Error::set_explanation(const std::string& s) {
+void Error::set_explanation(std::string s) {
   // Writing long explanations is a PITA, so we do some preprocessing on the
   // string s to make it nicer:
   //
   //    If s starts with a newline, then the following string of spaces is
   //    interpreted as the amount to be removed following every newline.
-  explanation_ = s;
+  explanation_ = std::move(s);
   if (explanation_.empty() || explanation_[0] != '\n') {
     return;
   }

runtime/src/cloe/entity.cpp

@@ -27,11 +27,11 @@
 namespace cloe {
 
 static std::regex VALID_NAME_REGEX{"^[a-zA-Z_][a-zA-Z0-9_]*(/[a-zA-Z_][a-zA-Z0-9_]*)*$"};
-void Entity::set_name(const std::string& name) {
+void Entity::set_name(std::string name) {
   if (!std::regex_match(name, VALID_NAME_REGEX)) {
     throw InvalidNameError(name);
   }
-  name_ = name;
+  name_ = std::move(name);
 }
 
 }  // namespace cloe

runtime/src/cloe/trigger.cpp

@@ -29,7 +29,7 @@
 
 namespace cloe {
 
-InlineSchema::InlineSchema(std::string&& desc, JsonType type, bool required)
+InlineSchema::InlineSchema(std::string desc, JsonType type, bool required)
     : type_(type)
     , required_(required)
     , usage_("<" + fable::to_string(type) + ">")