From ac5dabc929c68877bfd53ad8665b3fdd235b4536 Mon Sep 17 00:00:00 2001 From: Lauritz Timm Date: Wed, 19 Apr 2023 07:52:22 +0000 Subject: [PATCH] Adds `FeatureModelConstraint` iterator and bindings (#114) --- CMakeLists.txt | 8 +-- .../TEST_INPUTS/example_feature_model.xml | 9 +++ .../tests/constraint/test_constraint.py | 32 ++++++++++ .../tests/feature_model/test_feature_model.py | 17 ++++++ .../python/vara-feature/pybind_Constraint.cpp | 3 +- .../vara-feature/pybind_FeatureModel.cpp | 59 ++++++++++++++++++- include/vara/Feature/FeatureModel.h | 50 ++++++++++------ include/vara/Utils/UniqueIterator.h | 56 ++++++++++++++++++ unittests/Feature/FeatureModelBuilder.cpp | 19 +++--- unittests/Feature/FeatureModelParser.cpp | 19 +++--- unittests/resources/xml/vm.dtd | 2 +- 11 files changed, 227 insertions(+), 47 deletions(-) create mode 100644 include/vara/Utils/UniqueIterator.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e1e74ffa..9f8766a86 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -119,13 +119,13 @@ endif() find_program( VARA_FEATURE_RUN_CLANG_TIDY - NAMES run-clang-tidy-12.py - run-clang-tidy-12 + NAMES run-clang-tidy-14.py + run-clang-tidy-14 run-clang-tidy.py run-clang-tidy - clang-tidy-12 + clang-tidy-14 clang-tidy - PATHS /usr/lib/llvm/*/share/clang/ /usr/bin/ + PATHS /usr/bin/ /usr/lib/llvm/*/share/clang/ ) add_custom_target( diff --git a/bindings/python/tests/TEST_INPUTS/example_feature_model.xml b/bindings/python/tests/TEST_INPUTS/example_feature_model.xml index fe7efcc28..2b5ae0aff 100644 --- a/bindings/python/tests/TEST_INPUTS/example_feature_model.xml +++ b/bindings/python/tests/TEST_INPUTS/example_feature_model.xml @@ -84,4 +84,13 @@ 1;5;9 + + (A | B) + + + (A + B) + + + ((A * B) = 0) + diff --git a/bindings/python/tests/constraint/test_constraint.py b/bindings/python/tests/constraint/test_constraint.py index 025c9f553..b5b933866 100644 --- a/bindings/python/tests/constraint/test_constraint.py +++ b/bindings/python/tests/constraint/test_constraint.py @@ -4,6 +4,7 @@ import unittest import vara_feature.constraint as constraint +import vara_feature.feature_model as FM class TestConstraintBuilder(unittest.TestCase): @@ -30,3 +31,34 @@ def test_full(self): cb.feature("D").closePar().closePar().closePar().closePar().implies() cb.feature("E").closePar() self.assertEqual("((A => (!!B => !(!C => !!D))) => E)", str(cb.build())) + + +class TestConstraints(unittest.TestCase): + """ Test Constraints functionality. """ + + def test_boolean_constraints(self): + """ Check if we can build boolean constraints. """ + cb = constraint.ConstraintBuilder() + cb.feature("A").lOr().feature("B") + c = FM.BooleanConstraint(cb.build()) + self.assertEqual(str(c), "(A | B)") + self.assertEqual(str(c.constraint.clone()), "(A | B)") + + def test_non_boolean_constraints(self): + """ Check if we can build non-boolean constraints. """ + cb = constraint.ConstraintBuilder() + cb.feature("A").add().feature("B") + c = FM.NonBooleanConstraint(cb.build()) + self.assertIsNotNone(c.constraint) + self.assertEqual(str(c), "(A + B)") + self.assertEqual(str(c.constraint.clone()), "(A + B)") + + def test_mixed_constraints(self): + """ Check if we can build mixed constraints. """ + cb = constraint.ConstraintBuilder() + cb.feature("A").multiply().feature("B").equal().constant(0) + c = FM.MixedConstraint(cb.build(), FM.Req.ALL, FM.ExprKind.POS) + self.assertEqual(c.req, FM.Req.ALL) + self.assertEqual(c.exprKind, FM.ExprKind.POS) + self.assertEqual(str(c), "((A * B) = 0)") + self.assertEqual(str(c.constraint.clone()), "((A * B) = 0)") diff --git a/bindings/python/tests/feature_model/test_feature_model.py b/bindings/python/tests/feature_model/test_feature_model.py index c584b0ef7..64957935c 100644 --- a/bindings/python/tests/feature_model/test_feature_model.py +++ b/bindings/python/tests/feature_model/test_feature_model.py @@ -148,6 +148,23 @@ def test_merge_models(self): self.assertEqual( test_merged.get_feature("Z").parent().name.str(), "root") + def test_boolean_constraints(self): + """ Check if we can access boolean constraints. """ + *_, c = iter(self.fm.booleanConstraints) + self.assertEqual(str(c), "(A | B)") + + def test_non_boolean_constraints(self): + """ Check if we can access non-boolean constraints. """ + *_, c = iter(self.fm.nonBooleanConstraints) + self.assertEqual(str(c), "(A + B)") + + def test_mixed_constraints(self): + """ Check if we can access mixed constraints. """ + *_, c = iter(self.fm.mixedConstraints) + self.assertEqual(c.req, FM.Req.ALL) + self.assertEqual(c.exprKind, FM.ExprKind.POS) + self.assertEqual(str(c), "((A * B) = 0)") + class TestFeatureModelModifications(unittest.TestCase): """ diff --git a/bindings/python/vara-feature/pybind_Constraint.cpp b/bindings/python/vara-feature/pybind_Constraint.cpp index 8b240a9d0..04b50c960 100644 --- a/bindings/python/vara-feature/pybind_Constraint.cpp +++ b/bindings/python/vara-feature/pybind_Constraint.cpp @@ -8,7 +8,8 @@ namespace py = pybind11; void init_constraint_module_constraint(py::module &M) { py::class_(M, "Constraint") - .def("__str__", &vf::Constraint::toString); + .def("__str__", &vf::Constraint::toString) + .def("clone", &vf::Constraint::clone); } void init_constraint_module_constraint_builder(py::module &M) { diff --git a/bindings/python/vara-feature/pybind_FeatureModel.cpp b/bindings/python/vara-feature/pybind_FeatureModel.cpp index d1fa4f3a1..c279a25b0 100644 --- a/bindings/python/vara-feature/pybind_FeatureModel.cpp +++ b/bindings/python/vara-feature/pybind_FeatureModel.cpp @@ -13,6 +13,46 @@ namespace vf = vara::feature; namespace py = pybind11; void init_feature_model_module(py::module &M) { + py::class_(M, "BooleanConstraint") + .def(py::init([](vf::Constraint &C) { + return std::make_unique(C.clone()); + })) + .def("__str__", &vf::FeatureModel::BooleanConstraint::toString) + .def_property_readonly("constraint", + &vf::FeatureModel::BooleanConstraint::constraint, + R"pbdoc(Get underlying constraint.)pbdoc"); + + py::class_(M, "NonBooleanConstraint") + .def(py::init([](vf::Constraint &C) { + return std::make_unique( + C.clone()); + })) + .def("__str__", &vf::FeatureModel::NonBooleanConstraint::toString) + .def_property_readonly( + "constraint", &vf::FeatureModel::NonBooleanConstraint::constraint, + R"pbdoc(Get underlying constraint.)pbdoc"); + + py::enum_(M, "Req") + .value("ALL", vf::FeatureModel::MixedConstraint::Req::ALL) + .value("NONE", vf::FeatureModel::MixedConstraint::Req::NONE); + py::enum_(M, "ExprKind") + .value("POS", vf::FeatureModel::MixedConstraint::ExprKind::POS) + .value("NEG", vf::FeatureModel::MixedConstraint::ExprKind::NEG); + py::class_(M, "MixedConstraint") + .def(py::init([](vf::Constraint &C, + vf::FeatureModel::MixedConstraint::Req R, + vf::FeatureModel::MixedConstraint::ExprKind E) { + return std::make_unique(C.clone(), R, + E); + })) + .def("__str__", &vf::FeatureModel::MixedConstraint::toString) + .def_property_readonly("req", &vf::FeatureModel::MixedConstraint::req) + .def_property_readonly("exprKind", + &vf::FeatureModel::MixedConstraint::exprKind) + .def_property_readonly("constraint", + &vf::FeatureModel::MixedConstraint::constraint, + R"pbdoc(Get underlying constraint.)pbdoc"); + py::class_(M, "FeatureModel") .def_property_readonly( "name", &vf::FeatureModel::getName, @@ -99,7 +139,24 @@ void init_feature_model_module(py::module &M) { [](const vf::FeatureModel &FM) { return py::make_iterator(FM.begin(), FM.end()); }, - py::keep_alive<0, 1>()); + py::keep_alive<0, 1>()) + .def_property_readonly("booleanConstraints", + [](const vf::FeatureModel &FM) { + return py::make_iterator( + FM.booleanConstraints().begin(), + FM.booleanConstraints().end()); + }) + .def_property_readonly("nonBooleanConstraints", + [](const vf::FeatureModel &FM) { + return py::make_iterator( + FM.nonBooleanConstraints().begin(), + FM.nonBooleanConstraints().end()); + }) + .def_property_readonly( + "mixedConstraints", [](const vf::FeatureModel &FM) { + return py::make_iterator(FM.mixedConstraints().begin(), + FM.mixedConstraints().end()); + }); M.def( "loadFeatureModel", [](const std::filesystem::path &Path) { diff --git a/include/vara/Feature/FeatureModel.h b/include/vara/Feature/FeatureModel.h index be1e2885b..af98ee877 100644 --- a/include/vara/Feature/FeatureModel.h +++ b/include/vara/Feature/FeatureModel.h @@ -6,6 +6,7 @@ #include "vara/Feature/Feature.h" #include "vara/Feature/Relationship.h" #include "vara/Utils/Result.h" +#include "vara/Utils/UniqueIterator.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringMap.h" @@ -236,12 +237,19 @@ class FeatureModel { /// \brief Base for different constraint kinds (boolean, non-boolean, etc.). class FeatureModelConstraint { public: - FeatureModelConstraint(std::unique_ptr C) : C(std::move(C)) {} + virtual ~FeatureModelConstraint() = default; [[nodiscard]] Constraint *constraint() { return C.get(); } [[nodiscard]] Constraint *operator*() { return C.get(); } + [[nodiscard]] virtual std::string toString() const { return C->toString(); } + + [[nodiscard]] virtual std::string toHTML() const { return C->toHTML(); } + + protected: + FeatureModelConstraint(std::unique_ptr C) : C(std::move(C)) {} + private: std::unique_ptr C; }; @@ -256,12 +264,13 @@ class FeatureModel { using BooleanConstraintContainerTy = std::vector>; using const_boolean_constraint_iterator = - typename BooleanConstraintContainerTy::const_iterator; + UniqueIterator; [[nodiscard]] llvm::iterator_range booleanConstraints() const { - return llvm::make_range(BooleanConstraints.begin(), - BooleanConstraints.end()); + return llvm::make_range( + const_boolean_constraint_iterator(BooleanConstraints.begin()), + const_boolean_constraint_iterator(BooleanConstraints.end())); } class NonBooleanConstraint : public FeatureModelConstraint { @@ -273,12 +282,13 @@ class FeatureModel { using NonBooleanConstraintContainerTy = std::vector>; using const_non_boolean_constraint_iterator = - typename NonBooleanConstraintContainerTy::const_iterator; + UniqueIterator; [[nodiscard]] llvm::iterator_range nonBooleanConstraints() const { - return llvm::make_range(NonBooleanConstraints.begin(), - NonBooleanConstraints.end()); + return llvm::make_range( + const_non_boolean_constraint_iterator(NonBooleanConstraints.begin()), + const_non_boolean_constraint_iterator(NonBooleanConstraints.end())); } class MixedConstraint : public FeatureModelConstraint { @@ -306,11 +316,13 @@ class FeatureModel { using MixedConstraintContainerTy = std::vector>; using const_mixed_constraint_iterator = - typename MixedConstraintContainerTy::const_iterator; + UniqueIterator; [[nodiscard]] llvm::iterator_range mixedConstraints() const { - return llvm::make_range(MixedConstraints.begin(), MixedConstraints.end()); + return llvm::make_range( + const_mixed_constraint_iterator(MixedConstraints.begin()), + const_mixed_constraint_iterator(MixedConstraints.end())); } //===--------------------------------------------------------------------===// @@ -388,12 +400,13 @@ class FeatureModel { } using boolean_constraint_iterator = - typename BooleanConstraintContainerTy::iterator; + UniqueIterator; [[nodiscard]] llvm::iterator_range booleanConstraints() { - return llvm::make_range(BooleanConstraints.begin(), - BooleanConstraints.end()); + return llvm::make_range( + boolean_constraint_iterator(BooleanConstraints.begin()), + boolean_constraint_iterator(BooleanConstraints.end())); } vara::feature::Constraint * @@ -403,12 +416,13 @@ class FeatureModel { } using non_boolean_constraint_iterator = - typename NonBooleanConstraintContainerTy::iterator; + UniqueIterator; [[nodiscard]] llvm::iterator_range nonBooleanConstraints() { - return llvm::make_range(NonBooleanConstraints.begin(), - NonBooleanConstraints.end()); + return llvm::make_range( + non_boolean_constraint_iterator(NonBooleanConstraints.begin()), + non_boolean_constraint_iterator(NonBooleanConstraints.end())); } vara::feature::Constraint *addConstraint(std::unique_ptr C) { @@ -416,12 +430,12 @@ class FeatureModel { return **MixedConstraints.back(); } - using mixed_constraint_iterator = - typename MixedConstraintContainerTy::iterator; + using mixed_constraint_iterator = UniqueIterator; [[nodiscard]] llvm::iterator_range mixedConstraints() { - return llvm::make_range(MixedConstraints.begin(), MixedConstraints.end()); + return llvm::make_range(mixed_constraint_iterator(MixedConstraints.begin()), + mixed_constraint_iterator(MixedConstraints.end())); } //===--------------------------------------------------------------------===// diff --git a/include/vara/Utils/UniqueIterator.h b/include/vara/Utils/UniqueIterator.h new file mode 100644 index 000000000..f03a85e28 --- /dev/null +++ b/include/vara/Utils/UniqueIterator.h @@ -0,0 +1,56 @@ +#ifndef VARA_UTILS_UNIQUEITERATOR_H +#define VARA_UTILS_UNIQUEITERATOR_H + +#include + +namespace vara { + +//===----------------------------------------------------------------------===// +// UniqueIterator Class +//===----------------------------------------------------------------------===// + +template ::value, + typename ContainerTy::const_iterator, + typename ContainerTy::iterator>::type> +class UniqueIterator { +public: + UniqueIterator(IteratorTy Iterator) : Iterator{Iterator} {} + UniqueIterator(const UniqueIterator &) = default; + UniqueIterator &operator=(const UniqueIterator &) = delete; + UniqueIterator(UniqueIterator &&) noexcept = default; + UniqueIterator &operator=(UniqueIterator &&) = delete; + ~UniqueIterator() = default; + + ContainedTy *operator*() const { return Iterator->get(); } + + ContainedTy *operator->() const { return operator*(); } + + UniqueIterator operator++() { + ++Iterator; + return *this; + } + + UniqueIterator operator++(int) { + auto Iter(*this); + ++*this; + return Iter; + } + + bool operator==(const UniqueIterator &Other) const { + return Iterator == Other.Iterator; + } + + bool operator!=(const UniqueIterator &Other) const { + return !(*this == Other); + } + +private: + IteratorTy Iterator; +}; + +} // namespace vara + +#endif // VARA_UTILS_UNIQUEITERATOR_H diff --git a/unittests/Feature/FeatureModelBuilder.cpp b/unittests/Feature/FeatureModelBuilder.cpp index 810498230..c193529ca 100644 --- a/unittests/Feature/FeatureModelBuilder.cpp +++ b/unittests/Feature/FeatureModelBuilder.cpp @@ -99,7 +99,7 @@ TEST(FeatureModelBuilder, addOrConstraint) { ASSERT_TRUE(FM); EXPECT_EQ( - (*FM->booleanConstraints().begin())->constraint()->getRoot()->toString(), + FM->booleanConstraints().begin()->constraint()->getRoot()->toString(), Expected); EXPECT_EQ( (*FM->getFeature("a")->constraints().begin())->getRoot()->toString(), @@ -122,11 +122,9 @@ TEST(FeatureModelBuilder, addNonBooleanConstraint) { std::unique_ptr FM = B.buildFeatureModel(); ASSERT_TRUE(FM); - EXPECT_EQ((*FM->nonBooleanConstraints().begin()) - ->constraint() - ->getRoot() - ->toString(), - Expected); + EXPECT_EQ( + FM->nonBooleanConstraints().begin()->constraint()->getRoot()->toString(), + Expected); EXPECT_EQ( (*FM->getFeature("a")->constraints().begin())->getRoot()->toString(), Expected); @@ -152,12 +150,11 @@ TEST(FeatureModelBuilder, addMixedConstraint) { std::unique_ptr FM = B.buildFeatureModel(); ASSERT_TRUE(FM); - EXPECT_EQ( - (*FM->mixedConstraints().begin())->constraint()->getRoot()->toString(), - Expected); - EXPECT_EQ((*FM->mixedConstraints().begin())->req(), + EXPECT_EQ(FM->mixedConstraints().begin()->constraint()->getRoot()->toString(), + Expected); + EXPECT_EQ(FM->mixedConstraints().begin()->req(), FeatureModel::MixedConstraint::Req::ALL); - EXPECT_EQ((*FM->mixedConstraints().begin())->exprKind(), + EXPECT_EQ(FM->mixedConstraints().begin()->exprKind(), FeatureModel::MixedConstraint::ExprKind::POS); EXPECT_EQ( (*FM->getFeature("a")->constraints().begin())->getRoot()->toString(), diff --git a/unittests/Feature/FeatureModelParser.cpp b/unittests/Feature/FeatureModelParser.cpp index 5a31ca61c..146a78fb3 100644 --- a/unittests/Feature/FeatureModelParser.cpp +++ b/unittests/Feature/FeatureModelParser.cpp @@ -117,7 +117,7 @@ TEST(FeatureModelParser, booleanConstraint) { ASSERT_TRUE(FM); EXPECT_EQ( - (*FM->booleanConstraints().begin())->constraint()->getRoot()->toString(), + FM->booleanConstraints().begin()->constraint()->getRoot()->toString(), C.toString()); } @@ -130,11 +130,9 @@ TEST(FeatureModelParser, nonBooleanConstraint) { auto FM = buildFeatureModel("test_constraints.xml"); ASSERT_TRUE(FM); - EXPECT_EQ((*FM->nonBooleanConstraints().begin()) - ->constraint() - ->getRoot() - ->toString(), - C.toString()); + EXPECT_EQ( + FM->nonBooleanConstraints().begin()->constraint()->getRoot()->toString(), + C.toString()); } TEST(FeatureModelParser, mixedConstraint) { @@ -148,12 +146,11 @@ TEST(FeatureModelParser, mixedConstraint) { auto FM = buildFeatureModel("test_constraints.xml"); ASSERT_TRUE(FM); - EXPECT_EQ( - (*FM->mixedConstraints().begin())->constraint()->getRoot()->toString(), - C.toString()); - EXPECT_EQ((*FM->mixedConstraints().begin())->req(), + EXPECT_EQ(FM->mixedConstraints().begin()->constraint()->getRoot()->toString(), + C.toString()); + EXPECT_EQ(FM->mixedConstraints().begin()->req(), FeatureModel::MixedConstraint::Req::ALL); - EXPECT_EQ((*FM->mixedConstraints().begin())->exprKind(), + EXPECT_EQ(FM->mixedConstraints().begin()->exprKind(), FeatureModel::MixedConstraint::ExprKind::POS); } diff --git a/unittests/resources/xml/vm.dtd b/unittests/resources/xml/vm.dtd index 1ef8ac061..7e91dd25b 100644 --- a/unittests/resources/xml/vm.dtd +++ b/unittests/resources/xml/vm.dtd @@ -1,5 +1,5 @@ - +