Add Parent and Child Within One Commit

include/vara/Feature/FeatureModelTransaction.h

@@ -91,11 +91,12 @@ class FeatureModelTransaction
   /// \returns a pointer to the inserted Feature in CopyMode, otherwise,
   ///          nothing.
   decltype(auto) addFeature(std::unique_ptr<Feature> NewFeature,
-                            Feature *Parent = nullptr) {
+                            Feature *Parent = nullptr,
+                            const std::set<Feature *> &Children = {}) {
     if constexpr (IsCopyMode) {
-      return this->addFeatureImpl(std::move(NewFeature), Parent);
+      return this->addFeatureImpl(std::move(NewFeature), Parent, Children);
     } else {
-      this->addFeatureImpl(std::move(NewFeature), Parent);
+      this->addFeatureImpl(std::move(NewFeature), Parent, Children);
     }
   }
 
@@ -314,16 +315,25 @@ class AddFeatureToModel : public FeatureModelModification {
       setParent(*InsertedFeature, *FM.getRoot());
       addEdge(*FM.getRoot(), *InsertedFeature);
     }
+    if (!Children.empty()) {
+      for (auto *Child : Children) {
+        setParent(*Child, *InsertedFeature);
+        addEdge(*InsertedFeature, *Child);
+      }
+    }
     return InsertedFeature;
   }
 
 private:
   AddFeatureToModel(std::unique_ptr<Feature> NewFeature,
-                    Feature *Parent = nullptr)
-      : NewFeature(std::move(NewFeature)), Parent(Parent) {}
+                    Feature *Parent = nullptr,
+                    std::set<Feature *> Children = {})
+      : NewFeature(std::move(NewFeature)), Parent(Parent),
+        Children(std::move(Children)) {}
 
   std::unique_ptr<Feature> NewFeature;
   Feature *Parent;
+  std::set<Feature *> Children;
 };
 
 //===----------------------------------------------------------------------===//
@@ -801,16 +811,23 @@ class FeatureModelCopyTransactionBase {
   // Modifications
 
   Result<FTErrorCode, Feature *>
-  addFeatureImpl(std::unique_ptr<Feature> NewFeature, Feature *Parent) {
+  addFeatureImpl(std::unique_ptr<Feature> NewFeature, Feature *Parent,
+                 const std::set<Feature *> &Children) {
     if (!FM) {
       return ERROR;
     }
 
     if (Parent) {
-      // To correctly add a parent, we need to translate it to a Feature in
-      // our copied FeatureModel
+      // To correctly add a parent and children, we need to translate them to
+      // Features in
+      //  our copied FeatureModel
+      std::set<Feature *> TranslatedChildren;
+      for (auto *Child : Children) {
+        TranslatedChildren.insert(translateFeature(*Child));
+      }
       return FeatureModelModification::makeModification<AddFeatureToModel>(
-          std::move(NewFeature), translateFeature(*Parent))(*FM);
+          std::move(NewFeature), translateFeature(*Parent),
+          TranslatedChildren)(*FM);
     }
 
     return FeatureModelModification::makeModification<AddFeatureToModel>(
@@ -979,12 +996,13 @@ class FeatureModelModifyTransactionBase {
   //===--------------------------------------------------------------------===//
   // Modifications
 
-  void addFeatureImpl(std::unique_ptr<Feature> NewFeature, Feature *Parent) {
+  void addFeatureImpl(std::unique_ptr<Feature> NewFeature, Feature *Parent,
+                      const std::set<Feature *> &Children) {
     assert(FM && "FeatureModel is null.");
 
     Modifications.push_back(
         FeatureModelModification::makeUniqueModification<AddFeatureToModel>(
-            std::move(NewFeature), Parent));
+            std::move(NewFeature), Parent, Children));
   }
 
   void removeFeatureImpl(FeatureVariantTy &F, bool Recursive = false) {
@@ -1095,6 +1113,10 @@ void addFeature(FeatureModel &FM, std::unique_ptr<Feature> NewFeature,
 void addFeatures(
     FeatureModel &FM,
     std::vector<std::pair<std::unique_ptr<Feature>, Feature *>> NewFeatures);
+void addFeatures(FeatureModel &FM,
+                 std::vector<std::tuple<std::unique_ptr<Feature>, Feature *,
+                                        std::set<Feature *>>>
+                     NewFeatures);
 
 /// Removes a Feature from the FeatureModel
 ///

lib/Feature/FeatureModelTransaction.cpp

@@ -31,6 +31,18 @@ void addFeatures(
   Trans.commit();
 }
 
+void addFeatures(FeatureModel &FM,
+                 std::vector<std::tuple<std::unique_ptr<Feature>, Feature *,
+                                        std::set<Feature *>>>
+                     NewFeatures) {
+  auto Trans = FeatureModelModifyTransaction::openTransaction(FM);
+  for (auto &NewFeature : NewFeatures) {
+    Trans.addFeature(std::move(std::get<0>(NewFeature)),
+                     std::get<1>(NewFeature), std::get<2>(NewFeature));
+  }
+  Trans.commit();
+}
+
 void removeFeature(FeatureModel &FM,
                    detail::FeatureVariantTy FeatureToBeDeleted,
                    bool Recursive) {

unittests/Feature/FeatureModelTransaction.cpp

@@ -203,6 +203,28 @@ TEST_F(FeatureModelTransactionModifyTest, addFeatureToModel) {
   EXPECT_EQ(FM->getFeature("a"), FM->getFeature("ab")->getParentFeature());
 }
 
+TEST_F(FeatureModelTransactionModifyTest, addFeatureToModelChildren) {
+  size_t FMSizeBefore = FM->size();
+
+  auto FT = FeatureModelModifyTransaction::openTransaction(*FM);
+  FT.addFeature(std::make_unique<BinaryFeature>("ab"), FM->getRoot(),
+                {FM->getFeature("a")});
+
+  EXPECT_EQ(FMSizeBefore, FM->size());
+  EXPECT_TRUE(FM->getFeature("a"));
+  EXPECT_TRUE(llvm::isa<RootFeature>(FM->getFeature("a")->getParentFeature()));
+  EXPECT_FALSE(FM->getFeature("ab")); // Change should not be visible
+
+  FT.commit(); // Commit changes
+
+  EXPECT_EQ(FMSizeBefore + 1, FM->size());
+  EXPECT_TRUE(FM->getFeature("a"));
+  EXPECT_FALSE(llvm::isa<RootFeature>(FM->getFeature("a")->getParentFeature()));
+  EXPECT_TRUE(llvm::isa<RootFeature>(FM->getFeature("ab")->getParentFeature()));
+  EXPECT_TRUE(FM->getFeature("ab")); // Change should be visible
+  EXPECT_EQ(FM->getFeature("ab"), FM->getFeature("a")->getParentFeature());
+}
+
 TEST_F(FeatureModelTransactionModifyTest, addFeatureToModelThenAboard) {
   size_t FMSizeBefore = FM->size();
 
@@ -922,6 +944,25 @@ TEST_F(FeatureModelTransactionTest, addFeaturesToModel) {
   EXPECT_EQ(FM->getFeature("a"), FM->getFeature("ac")->getParentFeature());
 }
 
+TEST_F(FeatureModelTransactionTest, addFeaturesToModelChildren) {
+  size_t FMSizeBefore = FM->size();
+
+  std::vector<
+      std::tuple<std::unique_ptr<Feature>, Feature *, std::set<Feature *>>>
+      NewFeatures;
+  NewFeatures.emplace_back(
+      std::make_tuple(std::make_unique<BinaryFeature>("ab"), FM->getRoot(),
+                      std::set<Feature *>{FM->getFeature("a")}));
+
+  vara::feature::addFeatures(*FM, std::move(NewFeatures));
+
+  EXPECT_EQ(FMSizeBefore, FM->size() - 1);
+  EXPECT_TRUE(FM->getFeature("a"));
+  EXPECT_EQ(FM->getFeature("ab"), FM->getFeature("a")->getParentFeature());
+  EXPECT_TRUE(FM->getFeature("ab"));
+  EXPECT_TRUE(llvm::isa<RootFeature>(FM->getFeature("ab")->getParentFeature()));
+}
+
 TEST_F(FeatureModelTransactionTest, removeFeatureFromModel) {
   // Prepare Model with two Features
   std::vector<std::pair<std::unique_ptr<Feature>, Feature *>> NewFeatures;