diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4bb5e6b5..2461a566 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,16 +10,11 @@ repos: - id: check-added-large-files - id: check-merge-conflict -- repo: https://github.com/pre-commit/mirrors-yapf - rev: 'v0.32.0' +- repo: https://github.com/google/yapf + rev: 'v0.43.0' hooks: - id: yapf -- repo: https://github.com/pylint-dev/pylint - rev: 'v3.0.0a6' - hooks: - - id: pylint - - repo: https://github.com/pre-commit/mirrors-clang-format rev: 'v16.0.3' hooks: diff --git a/config_utilities/CMakeLists.txt b/config_utilities/CMakeLists.txt index 97977682..99fa681b 100644 --- a/config_utilities/CMakeLists.txt +++ b/config_utilities/CMakeLists.txt @@ -9,9 +9,7 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wall -Wextra -Wpedantic) endif() - list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") - include(OptionalPackage) option(BUILD_SHARED_LIBS "Build shared libs" ON) @@ -21,7 +19,7 @@ option(CONFIG_UTILS_BUILD_TESTS "Build unit tests" ON) option(CONFIG_UTILS_BUILD_DEMOS "Build demo executables" ON) find_package(yaml-cpp REQUIRED) -find_package(Boost REQUIRED COMPONENTS filesystem system) +find_package(Boost CONFIG REQUIRED COMPONENTS filesystem system) find_optional(Eigen3 CONFIG_UTILS_ENABLE_EIGEN) find_optional_pkgcfg(libglog CONFIG_UTILS_ENABLE_GLOG) @@ -50,7 +48,7 @@ add_library( src/visitor.cpp src/yaml_parser.cpp src/yaml_utils.cpp - ) +) target_link_libraries( ${PROJECT_NAME} PUBLIC yaml-cpp @@ -86,4 +84,4 @@ if(CONFIG_UTILS_BUILD_TESTS) add_subdirectory(test) endif() -include(HandleInstall) \ No newline at end of file +include(HandleInstall) diff --git a/config_utilities/src/visitor.cpp b/config_utilities/src/visitor.cpp index 7cd8dc86..0cc1bc4d 100644 --- a/config_utilities/src/visitor.cpp +++ b/config_utilities/src/visitor.cpp @@ -99,11 +99,13 @@ std::optional Visitor::visitVirtualConfig(bool is_set, } } - if (is_set && (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetInfo)) { + if (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetInfo) { // Also write the type param back to file. std::string error; - YAML::Node type_node = - YamlParser::toYaml(Settings::instance().factory.type_param_name, type, visitor.name_space, error); + YAML::Node type_node = YamlParser::toYaml(Settings::instance().factory.type_param_name, + is_set ? type : kUninitializedVirtualConfigType, + visitor.name_space, + error); mergeYamlNodes(visitor.data.data, type_node); } diff --git a/config_utilities/test/tests/virtual_config.cpp b/config_utilities/test/tests/virtual_config.cpp index 9f47c498..42e57fd8 100644 --- a/config_utilities/test/tests/virtual_config.cpp +++ b/config_utilities/test/tests/virtual_config.cpp @@ -96,7 +96,7 @@ struct Derived2WithComplexParam : public Base2 { const std::shared_ptr i_; inline static const auto registration_ = config::RegistrationWithConfig>( - "Derived2WithComplexParam"); + "Derived2WithComplexParam"); }; void declare_config(Derived2WithComplexParam::Config& config) { @@ -108,13 +108,14 @@ struct Derived2WithMoveOnlyParam : public Base2 { struct Config { int i = 0; }; - explicit Derived2WithMoveOnlyParam(const Config& config, std::unique_ptr i) : config_(config), i_(std::move(i)) {} + explicit Derived2WithMoveOnlyParam(const Config& config, std::unique_ptr i) + : config_(config), i_(std::move(i)) {} std::string name() const override { return "Derived2WithMoveOnlyParam"; } const Config config_; const std::unique_ptr i_; inline static const auto registration_ = config::RegistrationWithConfig>( - "Derived2WithMoveOnlyParam"); + "Derived2WithMoveOnlyParam"); }; void declare_config(Derived2WithMoveOnlyParam::Config& config) { @@ -174,6 +175,48 @@ void declare_config(ObjectWithOptionalConfigs::Config& config) { config::field(config.modules, "modules"); } +struct DefaultedOptional { + struct Config { + int foo = 3; + } const config; + explicit DefaultedOptional(const Config& config) : config(config) {} +}; + +void declare_config(DefaultedOptional::Config& config) { + name(); + field(config.foo, "foo"); +} + +struct ParentOfDefaultedOptional { + struct Config { + VirtualConfig child{DefaultedOptional::Config()}; + } const config; + + explicit ParentOfDefaultedOptional(const Config& config) : config(config), child(config.child.create()) {} + std::unique_ptr child; +}; + +void declare_config(ParentOfDefaultedOptional::Config& config) { + name(); + field(config.child, "child"); + config.child.setOptional(); +} + +struct GrandparentOfDefaultedOptional { + struct Config { + VirtualConfig child; + } const config; + + explicit GrandparentOfDefaultedOptional(const Config& config) : config(config), child(config.child.create()) {} + std::unique_ptr child; +}; + +void declare_config(GrandparentOfDefaultedOptional::Config& config) { + name(); + field(config.child, "child"); + config.child.setOptional(); +} + TEST(VirtualConfig, isSet) { Settings().restoreDefaults(); @@ -570,4 +613,57 @@ TEST(VirtualConfig, optionalNullCreation) { ASSERT_EQ(object, nullptr); } +TEST(VirtualConfig, defaultedConfigCorrect) { + RegistrationGuard guard("DefaultedOptional"); + RegistrationGuard + parent_guard("ParentOfDefaultedOptional"); + RegistrationGuard + grandparent_guard("GrandparentOfDefaultedOptional"); + + { // default config does the right thing from YAML + const auto node = YAML::Load(R"""( +type: GrandparentOfDefaultedOptional +child: + type: ParentOfDefaultedOptional +)"""); + auto root = config::createFromYaml(node); + ASSERT_TRUE(root); + ASSERT_TRUE(root->child); + EXPECT_TRUE(root->child->child); + } + + { // manually specifying the type does the right thing + const auto node = YAML::Load(R"""( +type: GrandparentOfDefaultedOptional +child: + type: ParentOfDefaultedOptional + child: + type: DefaultedOptional + foo: 5 +)"""); + auto root = config::createFromYaml(node); + ASSERT_TRUE(root); + ASSERT_TRUE(root->child); + ASSERT_TRUE(root->child->child); + EXPECT_EQ(root->child->child->config.foo, 5); + } + + { // overriding default does the right thing + const auto node = YAML::Load(R"""( +type: GrandparentOfDefaultedOptional +child: + type: ParentOfDefaultedOptional + child: + type: '' +)"""); + auto root_config = config::fromYaml(node); + auto root = config::createFromYaml(node); + ASSERT_TRUE(root); + ASSERT_TRUE(root->child); + EXPECT_FALSE(root->child->child); + } +} + } // namespace config::test