From 8970ef67d4493f7a8bd1caa041766b070cd0e311 Mon Sep 17 00:00:00 2001 From: overflowerror Date: Sun, 6 Feb 2022 22:15:05 +0100 Subject: [PATCH] optimized alternative collapsing only collapse alternative if the result is smaller than the uncollapsed version --- .../hoisting/HoistingProcessorTest.java | 42 ++++++++++++++++++- .../hoistingGuards/AlternativesGuard.java | 10 +++++ .../tokenGuards/TokenSequenceGuard.java | 4 +- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/org.eclipse.xtext.tests/src/org/eclipse/xtext/xtext/generator/hoisting/HoistingProcessorTest.java b/org.eclipse.xtext.tests/src/org/eclipse/xtext/xtext/generator/hoisting/HoistingProcessorTest.java index 3202986ef..dabdc0469 100644 --- a/org.eclipse.xtext.tests/src/org/eclipse/xtext/xtext/generator/hoisting/HoistingProcessorTest.java +++ b/org.eclipse.xtext.tests/src/org/eclipse/xtext/xtext/generator/hoisting/HoistingProcessorTest.java @@ -802,7 +802,25 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertFalse(guard.isTrivial()); assertTrue(guard.hasTerminal()); - assertEquals("(" + keyword("a", 2) + " || " + keyword("b", 1) + " || ((p0) && (p2))) && (" + keyword("b", 2) + " || " + keyword("b", 1) + " || ((p0) && (p3))) && (" + keyword("a", 1) + " || (p1))", guard.render()); + // collapsed version is actually not optimal + //assertEquals("(" + keyword("a", 2) + " || " + keyword("b", 1) + " || ((p0) && (p2))) && (" + keyword("b", 2) + " || " + keyword("b", 1) + " || ((p0) && (p3))) && (" + keyword("a", 1) + " || (p1))", guard.render()); + assertEquals( + "(" + + keyword("b", 1) + " || " + + "(" + + "(p0) && (" + + keyword("a", 2) + " || " + + "(p2)" + + ") && (" + + keyword("b", 2) + " || " + + "(p3)" + + ")" + + ")" + + ") && (" + + keyword("a", 1) + " || " + + "(p1)" + + ")", + guard.render()); } // currently not able to find optimal solution @@ -833,7 +851,27 @@ public class HoistingProcessorTest extends AbstractXtextTests { //assertEquals("(" + keyword("a", 1) + " || (" + keyword("c", 3) + " && " + eof(3) + ") || ((p0) && (p2))) && (" + keyword("c", 1) + " || ((p0) && (p3))) && (" + keyword("d", 3) + " || (p1))", guard.render()); // still valid but non-optimal - assertEquals("(" + keyword("a", 1) + " || (" + keyword("a", 3)+ " && " + keyword("b", 3) + " && " + keyword("c", 3) + " && " + eof(3) + ") || ((p0) && (p2))) && (" + keyword("b", 1) + " || (" + keyword("a", 3)+ " && " + keyword("b", 3) + " && " + keyword("c", 3) + " && " + eof(3) + ") || ((p0) && (p3))) && (" + keyword("d", 3) + " || (p1))", guard.render()); + assertEquals( + "(" + + "(" + + keyword("a", 3) + " && " + + keyword("b", 3) + " && " + + keyword("c", 3) + " && " + + eof(3) + + ") || (" + + "(p0) && (" + + keyword("a", 1) + " || " + + "(p2)" + + ") && (" + + keyword("b", 1) + " || " + + "(p3)" + + ")" + + ")" + + ") && (" + + keyword("d", 3) + " || " + + "(p1)" + + ")", + guard.render()); } @Test diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/hoistingGuards/AlternativesGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/hoistingGuards/AlternativesGuard.java index 313659e1c..54e66781f 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/hoistingGuards/AlternativesGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/hoistingGuards/AlternativesGuard.java @@ -31,9 +31,19 @@ public class AlternativesGuard implements HoistingGuard { } public AlternativesGuard(List paths) { + // collapsing nested alternatives might reduce output size + // -> try both and use smaller one + + this.paths = paths; + int lengthWithoutCollapse = render().length(); this.paths = PathGuard.collapse(paths); + int lengthWithCollapse = render().length(); + + if (lengthWithoutCollapse < lengthWithCollapse) + this.paths = paths; } + // package private so PathGuard can access this method List getPaths() { return paths; diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/tokenGuards/TokenSequenceGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/tokenGuards/TokenSequenceGuard.java index 1fc2cfd1e..ad8be9313 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/tokenGuards/TokenSequenceGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/tokenGuards/TokenSequenceGuard.java @@ -28,8 +28,10 @@ public class TokenSequenceGuard implements TokenGuard { public TokenSequenceGuard(Collection sequence) { Set checkedPositions = new HashSet<>(); + // remove all guards from sequences who's positions are already in the sequence this.sequence = sequence.stream() .flatMap(g -> { + // special case: reduce TokenSequenceGuards if (g instanceof TokenSequenceGuard) { return ((TokenSequenceGuard) g).sequence.stream() .filter(s -> !s.getPositions().stream() @@ -65,7 +67,7 @@ public class TokenSequenceGuard implements TokenGuard { @Override public String render() { if (sequence.size() == 1) { - return sequence.stream().findAny().get().render(); + return sequence.stream().findAny().get().render(); } else { return render(ContextConnective.DISJUNCTION); }