feat: allow factories to deal with move-only constructor arguments

MIT-SPARK · Feb 25, 2025 · 13e5078 · 13e5078
1 parent 0dfe952
commit 13e5078
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 32 deletions.
diff --git a/config_utilities/include/config_utilities/factory.h b/config_utilities/include/config_utilities/factory.h
@@ -243,7 +243,7 @@ class ModuleRegistry {
 
     // wrap factory call to register any allocations
     return [factory, key, type, create_callback](Args... args) -> BaseT* {
-      auto pointer = factory(args...);
+      auto pointer = factory(std::forward<Args>(args)...);
       create_callback(key, type, pointer);
       return pointer;
     };
@@ -367,17 +367,17 @@ struct ObjectFactory {
   // Add entries.
   template <typename DerivedT>
   static void addEntry(const std::string& type) {
-    const Constructor method = [](Args... args) -> BaseT* { return new DerivedT(args...); };
+    const Constructor method = [](Args&&... args) -> BaseT* { return new DerivedT(std::forward<Args>(args)...); };
     ModuleRegistry::addModule<BaseT, DerivedT, Args...>(type, method);
   }
 
-  static std::unique_ptr<BaseT> create(const std::string& type, Args... args) {
+  static std::unique_ptr<BaseT> create(const std::string& type, Args&&... args) {
     const auto factory = ModuleRegistry::getModule<BaseT, Args...>(type, registration_info);
     if (!factory) {
       return nullptr;
     }
 
-    return std::unique_ptr<BaseT>(factory(args...));
+    return std::unique_ptr<BaseT>(factory(std::forward<Args>(args)...));
   }
 };
 
@@ -391,16 +391,16 @@ struct ObjectWithConfigFactory {
   // Add entries.
   template <typename DerivedT, typename DerivedConfigT>
   static void addEntry(const std::string& type) {
-    const Constructor method = [](const YAML::Node& data, Args... args) -> BaseT* {
+    const Constructor method = [](const YAML::Node& data, Args&&... args) -> BaseT* {
       DerivedConfigT config;
       Visitor::setValues(config, data);
-      return new DerivedT(config, args...);
+      return new DerivedT(config, std::forward<Args>(args)...);
     };
 
     ModuleRegistry::addModule<BaseT, DerivedT, const YAML::Node&, Args...>(type, method, true);
   }
 
-  static std::unique_ptr<BaseT> create(const YAML::Node& data, Args... args) {
+  static std::unique_ptr<BaseT> create(const YAML::Node& data, Args&&... args) {
     std::string type;
     if (!getType(data, type)) {
       return nullptr;
@@ -411,7 +411,7 @@ struct ObjectWithConfigFactory {
       return nullptr;
     }
 
-    return std::unique_ptr<BaseT>(factory(data, args...));
+    return std::unique_ptr<BaseT>(factory(data, std::forward<Args>(args)...));
   }
 };
 
@@ -468,8 +468,9 @@ struct RegistrationWithConfig {
  * @returns Unique pointer of type base that contains the derived object.
  */
 template <typename BaseT, typename... ConstructorArguments>
-std::unique_ptr<BaseT> create(const std::string& type, ConstructorArguments... args) {
-  return internal::ObjectFactory<BaseT, ConstructorArguments...>::create(type, args...);
+std::unique_ptr<BaseT> create(const std::string& type, ConstructorArguments&&... args) {
+  return internal::ObjectFactory<BaseT, ConstructorArguments...>::create(type,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 }  // namespace config
diff --git a/config_utilities/include/config_utilities/internal/config_context.h b/config_utilities/include/config_utilities/internal/config_context.h
@@ -58,14 +58,16 @@ class Context {
   static YAML::Node toYaml();
 
   template <typename BaseT, typename... ConstructorArguments>
-  static std::unique_ptr<BaseT> create(ConstructorArguments... args) {
-    return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(instance().contents_, args...);
+  static std::unique_ptr<BaseT> create(ConstructorArguments&&... args) {
+    return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(instance().contents_,
+    std::forward<ConstructorArguments>(args)...);
   }
 
   template <typename BaseT, typename... ConstructorArguments>
-  static std::unique_ptr<BaseT> createNamespaced(const std::string& name_space, ConstructorArguments... args) {
+  static std::unique_ptr<BaseT> createNamespaced(const std::string& name_space, ConstructorArguments&&... args) {
     const auto ns_node = internal::lookupNamespace(instance().contents_, name_space);
-    return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node, args...);
+    return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node,
+      std::forward<ConstructorArguments>(args)...);
   }
 
   template <typename ConfigT>

diff --git a/config_utilities/include/config_utilities/parsing/commandline.h b/config_utilities/include/config_utilities/parsing/commandline.h
@@ -113,10 +113,11 @@ ConfigT fromCLI(const std::vector<std::string>& args, const std::string& name_sp
  * @returns Unique pointer of type base that contains the derived object.
  */
 template <typename BaseT, typename... ConstructorArguments>
-std::unique_ptr<BaseT> createFromCLI(int argc, char* argv[], ConstructorArguments... args) {
+std::unique_ptr<BaseT> createFromCLI(int argc, char* argv[], ConstructorArguments&&... args) {
   // when parsing CLI locally we don't want to modify the arguments ever
   const auto node = internal::loadFromArguments(argc, argv, false);
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node, args...);
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**
@@ -131,9 +132,10 @@ std::unique_ptr<BaseT> createFromCLI(int argc, char* argv[], ConstructorArgument
  * @returns Unique pointer of type base that contains the derived object.
  */
 template <typename BaseT, typename... ConstructorArguments>
-std::unique_ptr<BaseT> createFromCLI(const std::vector<std::string>& argv, ConstructorArguments... args) {
+std::unique_ptr<BaseT> createFromCLI(const std::vector<std::string>& argv, ConstructorArguments&&... args) {
   const auto node = internal::loadFromArguments(argv);
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node, args...);
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**
@@ -153,11 +155,12 @@ template <typename BaseT, typename... ConstructorArguments>
 std::unique_ptr<BaseT> createFromCLIWithNamespace(int argc,
                                                   char* argv[],
                                                   const std::string& name_space,
-                                                  ConstructorArguments... args) {
+                                                  ConstructorArguments&&... args) {
   // when parsing CLI locally we don't want to modify the arguments ever
   const auto node = internal::loadFromArguments(argc, argv, false);
   const auto ns_node = internal::lookupNamespace(node, name_space);
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node, args...);
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**
@@ -175,10 +178,11 @@ std::unique_ptr<BaseT> createFromCLIWithNamespace(int argc,
 template <typename BaseT, typename... ConstructorArguments>
 std::unique_ptr<BaseT> createFromCLIWithNamespace(const std::vector<std::string>& argv,
                                                   const std::string& name_space,
-                                                  ConstructorArguments... args) {
+                                                  ConstructorArguments&&... args) {
   const auto node = internal::loadFromArguments(argv);
   const auto ns_node = internal::lookupNamespace(node, name_space);
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node, args...);
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 }  // namespace config
diff --git a/config_utilities/include/config_utilities/parsing/yaml.h b/config_utilities/include/config_utilities/parsing/yaml.h
@@ -129,8 +129,9 @@ bool toYamlFile(const ConfigT& config, const std::string& file_name) {
  * @returns Unique pointer of type base that contains the derived object.
  */
 template <typename BaseT, typename... ConstructorArguments>
-std::unique_ptr<BaseT> createFromYaml(const YAML::Node& node, ConstructorArguments... args) {
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node, args...);
+std::unique_ptr<BaseT> createFromYaml(const YAML::Node& node, ConstructorArguments&&... args) {
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**
@@ -151,9 +152,10 @@ std::unique_ptr<BaseT> createFromYaml(const YAML::Node& node, ConstructorArgumen
 template <typename BaseT, typename... ConstructorArguments>
 std::unique_ptr<BaseT> createFromYamlWithNamespace(const YAML::Node& node,
                                                    const std::string& name_space,
-                                                   ConstructorArguments... args) {
+                                                   ConstructorArguments&&... args) {
   const YAML::Node ns_node = internal::lookupNamespace(node, name_space);
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node, args...);
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(ns_node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**
@@ -170,8 +172,9 @@ std::unique_ptr<BaseT> createFromYamlWithNamespace(const YAML::Node& node,
  * @returns Unique pointer of type base that contains the derived object.
  */
 template <typename BaseT, typename... ConstructorArguments>
-std::unique_ptr<BaseT> createFromYamlFile(const std::string& file_name, ConstructorArguments... args) {
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(YAML::LoadFile(file_name), args...);
+std::unique_ptr<BaseT> createFromYamlFile(const std::string& file_name, ConstructorArguments&&... args) {
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(YAML::LoadFile(file_name),
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**
@@ -192,9 +195,10 @@ std::unique_ptr<BaseT> createFromYamlFile(const std::string& file_name, Construc
 template <typename BaseT, typename... ConstructorArguments>
 std::unique_ptr<BaseT> createFromYamlFileWithNamespace(const std::string& file_name,
                                                        const std::string& name_space,
-                                                       ConstructorArguments... args) {
+                                                       ConstructorArguments&&... args) {
   const YAML::Node node = internal::lookupNamespace(YAML::LoadFile(file_name), name_space);
-  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node, args...);
+  return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(node,
+    std::forward<ConstructorArguments>(args)...);
 }
 
 /**

diff --git a/config_utilities/include/config_utilities/virtual_config.h b/config_utilities/include/config_utilities/virtual_config.h
@@ -172,7 +172,7 @@ class VirtualConfig {
    * @return The created object of DerivedT that inherits from BaseT.
    */
   template <typename... ConstructorArguments>
-  std::unique_ptr<BaseT> create(ConstructorArguments... args) const {
+  std::unique_ptr<BaseT> create(ConstructorArguments&&... args) const {
     if (!config_) {
       return nullptr;
     }
@@ -181,7 +181,8 @@ class VirtualConfig {
     // also be de-serialized so this should not result in any warnings, we print them anyways to be sure. The factory
     // should take proper care of any other verbose error management.
     const internal::MetaData data = internal::Visitor::getValues(*this);
-    return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(data.data, args...);
+    return internal::ObjectWithConfigFactory<BaseT, ConstructorArguments...>::create(data.data,
+      std::forward<ConstructorArguments>(args)...);
   }
 
  private:

diff --git a/config_utilities/test/tests/factory.cpp b/config_utilities/test/tests/factory.cpp
@@ -194,6 +194,64 @@ TEST(Factory, createWithConfig) {
   EXPECT_EQ(dynamic_cast<DerivedD*>(base.get())->config_.i, 3);
 }
 
+struct MoveOnlyBase {
+  virtual ~MoveOnlyBase() = default;
+  virtual void print() { std::cout << "Hi! I'm Base\n"; }
+};
+
+struct MoveOnlyDerived : public MoveOnlyBase {
+  explicit MoveOnlyDerived(std::unique_ptr<int> i) : i_(std::move(i)) {}
+  ~MoveOnlyDerived() override = default;
+
+  std::unique_ptr<int> i_;
+  inline static const auto registration =
+    config::Registration<MoveOnlyBase, MoveOnlyDerived, std::unique_ptr<int>>("MoveOnlyDerived");
+};
+
+struct MoveOnlyDerivedWithConfig : public MoveOnlyBase {
+  struct Config {
+    int i = 0;
+  };
+
+  explicit MoveOnlyDerivedWithConfig(const Config& config, std::unique_ptr<int> i) : config_(config), i_(std::move(i)) {}
+  ~MoveOnlyDerivedWithConfig() override = default;
+
+  Config config_;
+  std::unique_ptr<int> i_;
+  inline static const auto registration =
+    config::RegistrationWithConfig<MoveOnlyBase, MoveOnlyDerivedWithConfig, Config, std::unique_ptr<int>>(
+      "MoveOnlyDerivedWithConfig");
+};
+
+void declare_config(MoveOnlyDerivedWithConfig::Config& config) {
+  // Declare the config using the config utilities.
+  config::name("MoveOnlyDerivedWithConfig");
+  config::field(config.i, "i");
+}
+
+TEST(Factory, createWithMoveOnlyArgument) {
+  {
+    auto i = std::make_unique<int>(42);
+    auto base = config::create<MoveOnlyBase>("MoveOnlyDerived", std::move(i));
+    EXPECT_NE(base, nullptr);
+    auto ptr = dynamic_cast<MoveOnlyDerived*>(base.get());
+    EXPECT_NE(ptr, nullptr);
+    EXPECT_EQ(*ptr->i_, 42);
+  }
+  {
+    auto i = std::make_unique<int>(24);
+    YAML::Node yaml;
+    yaml["type"] = "MoveOnlyDerivedWithConfig";
+    yaml["i"] = 24;
+
+    auto base = config::createFromYaml<MoveOnlyBase>(yaml, std::move(i));
+    EXPECT_NE(base, nullptr);
+    auto ptr = dynamic_cast<MoveOnlyDerivedWithConfig*>(base.get());
+    EXPECT_NE(ptr, nullptr);
+    EXPECT_EQ(*ptr->i_, 24);
+  }
+}
+
 TEST(Factory, moduleNameConflicts) {
   auto logger = TestLogger::create();