Merge pull request #13 from VariantSync/next-release

VEVOS Simulation v.1.1.2

Author: AlexanderSchultheiss

src/main/java/org/variantsync/vevos/simulation/io/kernelhaven/ArtefactCSVExporter.java

@@ -59,9 +59,9 @@ public CSV export() {
     private String[] toRow(final LineBasedAnnotation annotation) {
         final String[] row = makeRow();
         row[0] = currentFile.getFile().toString();
-        row[1] = FormulaUtils.toString(currentFile.getPresenceCondition(), NodeWriter.javaSymbols);
-        row[2] = FormulaUtils.toString(annotation.getFeatureMapping(), NodeWriter.javaSymbols);
-        row[3] = FormulaUtils.toString(annotation.getPresenceCondition(), NodeWriter.javaSymbols);
+        row[1] = FormulaUtils.toFormulaString(currentFile.getPresenceCondition(), NodeWriter.javaSymbols);
+        row[2] = FormulaUtils.toFormulaString(annotation.getFeatureMapping(), NodeWriter.javaSymbols);
+        row[3] = FormulaUtils.toFormulaString(annotation.getPresenceCondition(), NodeWriter.javaSymbols);
         row[4] = "" + annotation.getLineFrom();
         // -1 because Kernelhaven stores annotations as [#if, #endif) intervals, so we have to point one line before the annotation end (#endif).
         row[5] = "" + (annotation.getLineTo() - (annotation.isMacro() ? 1 : 0));

src/main/java/org/variantsync/vevos/simulation/repository/BusyboxRepository.java

@@ -275,13 +275,13 @@ private static boolean doNotNormalizeDefUndef(@NonNull final String line) {
             temp = notNull(temp.replace("ENABLE_", "defined CONFIG_"));
         }
         if (temp.contains("&& ENABLE_")) {
-            temp = notNull(temp.replace("ENABLE_", "'defined CONFIG_"));
+            temp = notNull(temp.replace("ENABLE_", "defined CONFIG_"));
         }
         if (temp.contains("|| !ENABLE_")) {
             temp = notNull(temp.replace("!ENABLE_", "!defined CONFIG_"));
         }
         if (temp.contains("&& !ENABLE_")) {
-            temp = notNull(temp.replace("!ENABLE_", "'!defined CONFIG_"));
+            temp = notNull(temp.replace("!ENABLE_", "!defined CONFIG_"));
         }
 
         return temp;

src/main/java/org/variantsync/vevos/simulation/util/fide/FormulaUtils.java

@@ -103,10 +103,39 @@ public static Node replaceAllInplace(final Node root, final Predicate<Node> who,
         }
     }
 
-    public static String toString(Node formula, final String[] symbols) {
+    /**
+     * Wraps a literal name in a defined statement.
+     * x -> defined(x)
+     * !x -> !defined(x)
+     */
+    private static Node ifdef(final Literal l) {
+        final Function<Object, Literal> toDefinedLiteral = s -> new Literal("defined(" + s + ")");
+
+        if (l.positive) {
+            return toDefinedLiteral.apply(l);
+        } else {
+            return new Not(toDefinedLiteral.apply(new Literal(l.var, true)));
+        }
+    }
+
+    /**
+     * Serializes the given formula to a string usable in C preprocessor conditions.
+     * True and false will be converted to 1 and 0, respectively.
+     * Variables will be wrapped in a defined expression.
+     * @see FixTrueFalse#TrueAs1
+     * @see FixTrueFalse#FalseAs0
+     */
+    public static String toCPPString(Node formula) {
         formula = replaceAll(formula, FixTrueFalse::isTrue, n -> FixTrueFalse.TrueAs1);
         formula = replaceAllInplace(formula, FixTrueFalse::isFalse, n -> FixTrueFalse.FalseAs0);
+        formula = replaceAllInplace(formula, FixTrueFalse::isVariable,
+                // we know that n is a literal because FixTureFalse::isVariable returned true
+                n -> ifdef((Literal) n)
+        );
+        return toFormulaString(formula, NodeWriter.javaSymbols);
+    }
 
+    public static String toFormulaString(final Node formula, final String[] symbols) {
         final NodeWriter writer = new NodeWriter(formula);
         writer.setNotation(NodeWriter.Notation.INFIX);
         writer.setEnquoteWhitespace(false);

src/main/java/org/variantsync/vevos/simulation/util/fide/bugfix/FixTrueFalse.java

@@ -50,6 +50,13 @@ public static boolean isFalse(final Node n) {
         return n instanceof Literal l && isFalseLiteral(l);
     }
 
+    /**
+     * @return True iff the given formula is a literal that neither {@link #isTrue} nor {@link #isFalse}.
+     */
+    public static boolean isVariable(final Node n) {
+        return n instanceof Literal l && !isTrueLiteral(l) && !isFalseLiteral(l);
+    }
+
     /**
      * @return True iff the given name represents the atomic value true w.r.t. the constant TrueNames.
      */

src/main/java/org/variantsync/vevos/simulation/variability/SPLCommit.java

@@ -138,7 +138,7 @@ public Optional<SPLCommit[]> parents() {
         return Optional.ofNullable(parents);
     }
 
-    public void setParents(final SPLCommit[] parents) {
+    public void setParents(final SPLCommit... parents) {
         this.parents = parents;
     }
 

src/main/java/org/variantsync/vevos/simulation/variability/pc/variantlines/VariantAnnotation.java

@@ -1,8 +1,8 @@
 package org.variantsync.vevos.simulation.variability.pc.variantlines;
 
 import org.prop4j.Node;
-import org.prop4j.NodeWriter;
 import org.variantsync.vevos.simulation.util.fide.FormulaUtils;
+import org.variantsync.vevos.simulation.util.fide.bugfix.FixTrueFalse;
 import org.variantsync.vevos.simulation.variability.pc.options.VariantGenerationOptions;
 
 import java.util.ArrayList;
@@ -16,18 +16,24 @@ public record VariantAnnotation(
     public List<String> project(final VariantGenerationOptions projectionOptions, final List<String> splFileLines) {
         final List<String> result = new ArrayList<>();
 
-        if (projectionOptions.withMacros()) {
-            result.add("#if " + FormulaUtils.toString(condition, NodeWriter.javaSymbols));
+        final boolean genMacro = projectionOptions.withMacros() && !isTrue();
+
+        if (genMacro) {
+            result.add("#if " + FormulaUtils.toCPPString(condition));
         }
 
         for (final VariantLineChunk child : lines) {
             result.addAll(child.project(projectionOptions, splFileLines));
         }
 
-        if (projectionOptions.withMacros()) {
+        if (genMacro) {
             result.add("#endif");
         }
 
         return result;
     }
+
+    public boolean isTrue() {
+        return FixTrueFalse.isTrue(condition);
+    }
 }

src/main/java/org/variantsync/vevos/simulation/variability/sequenceextraction/LongestNonOverlappingSequences.java

@@ -83,29 +83,64 @@ public List<NonEmptyList<SPLCommit>> extract(final Collection<SPLCommit> commits
             }
         }
 
+        // Sort the sequences once more, after filtering them
+        commitSequences.sort((o1, o2) -> Integer.compare(o2.size(), o1.size()));
         return commitSequences;
     }
 
-    // Recursively build sequences
     private static Set<LinkedList<SPLCommit>> retrieveSequencesForStart(final Map<SPLCommit, Set<SPLCommit>> parentChildMap, final SPLCommit start) {
-        if (!parentChildMap.containsKey(start)) {
-            // Create a new sequence that contains the start commit
-            final Set<LinkedList<SPLCommit>> sequenceSet = new HashSet<>();
+        // Set for holding all retrieved sequences
+        Set<LinkedList<SPLCommit>> sequenceSet = new HashSet<>();
+        {
+            // We start with the 'start' commit
             final LinkedList<SPLCommit> sequence = new LinkedList<>();
             sequence.add(start);
             sequenceSet.add(sequence);
-            return sequenceSet;
-        } else {
-            // Collect the sequences of the children and prepend the commit to each of them as parent
-            final Set<LinkedList<SPLCommit>> sequences = new HashSet<>();
-            for (final SPLCommit child : parentChildMap.get(start)) {
-                final Set<LinkedList<SPLCommit>> childSequences = retrieveSequencesForStart(parentChildMap, child);
-                for (final LinkedList<SPLCommit> childSequence : childSequences) {
-                    childSequence.addFirst(start);
-                    sequences.add(childSequence);
+        }
+
+        // We continue to build sequences, going from parent to descendents, until the ends of all possible sequences have been reached
+        boolean incomplete = true;
+        while(incomplete) {
+            // Why do we have to create a new HashSet every iteration?
+            // (See comments https://github.com/VariantSync/VEVOS_Simulation/pull/13#discussion_r960891984)
+            // It makes the code a lot simpler and runtime should not really be an issue here.
+            // If we do not create a new set in each iteration, we have to create additional checks, because we can
+            // only update the old sequence if there is only one child. If there is more than one child, we have to
+            // first create copies of the sequence for each child and add these sequences to the set.
+            // Finally, we could change the initial sequence in-place for the last child. However, because the children
+            // are provided as a set, we either first have to convert the set to a list or array (which is the same as
+            // creating a new set above), or we have to introduce additional counters and/or flags and checks.
+            // It is not pretty. I was at least not able to think of something simpler, without introducing additional
+            // cost elsewhere.
+            Set<LinkedList<SPLCommit>> updatedSet = new HashSet<>();
+
+            // Set to false at the start of the iteration, it is set to true if at least one sequence has been extended
+            incomplete = false;
+            // For all found sequences, check whether their last commit has more children that can be added to the sequence
+            // if multiple children exist, new sequences are created
+            for (LinkedList<SPLCommit> sequence : sequenceSet) {
+                final SPLCommit parent = sequence.getLast();
+                if (parentChildMap.containsKey(parent)) {
+                    final Set<SPLCommit> children = parentChildMap.get(parent);
+                    for (SPLCommit child : children) {
+                        // There is at least one child, we are not done yet
+                        incomplete = true;
+                        // Create a new sequence from the old sequence for each child
+                        final LinkedList<SPLCommit> anotherSequence = new LinkedList<>(sequence);
+                        anotherSequence.add(child);
+                        // Add the new sequence to the new set
+                        updatedSet.add(anotherSequence);
+                    }
+                } else {
+                    // There are no children, simply add the old sequence to the updated set
+                    updatedSet.add(sequence);
                 }
             }
-            return sequences;
+            // Update the sequence set
+            sequenceSet = updatedSet;
         }
+
+        return sequenceSet;
     }
 }
+

src/test/java/org/variantsync/vevos/simulation/variability/sequenceextraction/LongestNonOverlappingSequencesTest.java

@@ -0,0 +1,98 @@
+package org.variantsync.vevos.simulation.variability.sequenceextraction;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.variantsync.functjonal.list.NonEmptyList;
+import org.variantsync.vevos.simulation.variability.SPLCommit;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+public class LongestNonOverlappingSequencesTest {
+
+    @Test
+    public void exampleExtraction() {
+        // For example, if the commits comprise three partially overlapping sequences ([A-B-C-D-E], [X-Y-Z], [A-B-F-G]),
+        // the function will return the sequences ([A-B-C-D-E], [X-Y-Z], [F-G]).
+        final SPLCommit a = new SPLCommit("A");
+        final SPLCommit b = new SPLCommit("B");
+        final SPLCommit c = new SPLCommit("C");
+        final SPLCommit d = new SPLCommit("D");
+        final SPLCommit e = new SPLCommit("E");
+        final SPLCommit f = new SPLCommit("F");
+        final SPLCommit g = new SPLCommit("G");
+        final SPLCommit x = new SPLCommit("X");
+        final SPLCommit y = new SPLCommit("Y");
+        final SPLCommit z = new SPLCommit("Z");
+        // A has no parent
+        a.setParents();
+        // A-B
+        b.setParents(a);
+        // B-C
+        c.setParents(b);
+        // C-D
+        d.setParents(c);
+        // D-E
+        e.setParents(d);
+        // B-F
+        f.setParents(b);
+        // F-G
+        g.setParents(f);
+        // X has no parent
+        x.setParents();
+        // X-Y
+        y.setParents(x);
+        // Y-Z
+        z.setParents(y);
+
+        List<SPLCommit> exampleCommits = Arrays.asList(a, b, c, d, e, f, g, x, y, z);
+        NonEmptyList<SPLCommit> expectedOne = new NonEmptyList<>(Arrays.asList(a,b,c,d,e));
+        NonEmptyList<SPLCommit> expectedTwo = new NonEmptyList<>(Arrays.asList(x, y, z));
+        NonEmptyList<SPLCommit> expectedThree = new NonEmptyList<>(Arrays.asList(f, g));
+
+        LongestNonOverlappingSequences algorithm = new LongestNonOverlappingSequences();
+        List<NonEmptyList<SPLCommit>> result = algorithm.extract(exampleCommits);
+
+        Assert.assertEquals(result.size(), 3);
+        Assert.assertTrue(sequencesAreEqual(result.get(0), expectedOne));
+        Assert.assertTrue(sequencesAreEqual(result.get(1), expectedTwo));
+        Assert.assertTrue(sequencesAreEqual(result.get(2), expectedThree));
+    }
+
+    @Test
+    // This test caused a StackoverflowError for the previous implementation of LongestNonOverlappingSequences at
+    // confirms that the Error is no longer thrown.
+    public void stackOverflowPrevented() {
+        int size = 10_000;
+        Set<SPLCommit> commits = new HashSet<>();
+        SPLCommit previousCommit = new SPLCommit("0");
+        previousCommit.setParents();
+        commits.add(previousCommit);
+        for (int i = 1; i < size; i++) {
+            SPLCommit commit = new SPLCommit(String.valueOf(i));
+            commit.setParents(previousCommit);
+            commits.add(commit);
+            previousCommit = commit;
+        }
+
+        LongestNonOverlappingSequences algo = new LongestNonOverlappingSequences();
+        var result = algo.extract(commits);
+        // Some sanity checks (i.e., is the sequence extracted correctly?)
+        Assert.assertEquals(1, result.size());
+        Assert.assertEquals(result.get(0).size(), size);
+    }
+
+    private boolean sequencesAreEqual(final NonEmptyList<SPLCommit> a, final NonEmptyList<SPLCommit> b) {
+        if (a.size() != b.size()) {
+            return false;
+        }
+        for (int i = 0; i < a.size(); i++) {
+            if (!a.get(i).id().equals(b.get(i).id())) {
+                return false;
+            }
+        }
+        return true;
+    }
+}