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 a9faf69d5..0419990cf 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 @@ -21,7 +21,6 @@ import org.eclipse.xtext.testing.GlobalRegistries; import org.eclipse.xtext.tests.AbstractXtextTests; import org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.HoistingProcessor; import org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.exceptions.TokenAnalysisAbortedException; -import org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.exceptions.UnsupportedConstructException; import org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.guards.HoistingGuard; import org.junit.After; import org.junit.Before; @@ -435,9 +434,8 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); } - // TODO: this is no longer unsupported - // @Test(expected = UnsupportedConstructException.class) - public void testUnorderedGroupWithEmptyPathsWithoutContext_expectUnsupportedConstruct() throws Exception { + @Test(expected = TokenAnalysisAbortedException.class) + public void testUnorderedGroupWithEmptyPathsWithoutContext_expectTokenAnalysisAbortedException() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -448,11 +446,14 @@ public class HoistingProcessorTest extends AbstractXtextTests { hoistingProcessor.init(grammar); AbstractRule rule = getRule(grammar, "S"); - hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + HoistingGuard guard = hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + assertFalse(guard.isTrivial()); + assertTrue(guard.hasTerminal()); + assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); } - //@Test - public void testUnorderedGroupWithoutMandatoryContent() throws Exception { + @Test + public void testUnorderedGroupWithoutMandatoryContentWithContext() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -466,12 +467,13 @@ public class HoistingProcessorTest extends AbstractXtextTests { HoistingGuard guard = hoistingProcessor.findHoistingGuard(rule.getAlternatives()); assertFalse(guard.isTrivial()); assertTrue(guard.hasTerminal()); - assertEquals("((" + getSyntaxForKeywordToken("s", 1) + " || (p2)) && (" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); + assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); } - // TODO: this is no longer unsupported - @Test(expected = UnsupportedConstructException.class) - public void testUnorderedGroupWithoutMandatoryContentWithoutContext_expectUnsupportedConstruct() throws Exception { + @Test + public void testUnorderedGroupWithoutMandatoryContentWithoutContext_expectNoEofCheck() throws Exception { + // no eof check needed since that case is covered by the two alternatives + // @formatter:off String model = MODEL_PREAMBLE + @@ -482,7 +484,11 @@ public class HoistingProcessorTest extends AbstractXtextTests { hoistingProcessor.init(grammar); AbstractRule rule = getRule(grammar, "S"); - hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + HoistingGuard guard = hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + assertFalse(guard.isTrivial()); + assertTrue(guard.hasTerminal()); + System.out.println(guard.toString()); + assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); } @Test @@ -888,8 +894,45 @@ public class HoistingProcessorTest extends AbstractXtextTests { Grammar grammar = ((Grammar) resource.getContents().get(0)); hoistingProcessor.init(grammar); AbstractRule rule = getRule(grammar, "S"); + + HoistingGuard guard = hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + assertFalse(guard.isTrivial()); + assertTrue(guard.hasTerminal()); + System.out.println(guard.render()); + } + + @Test + public void testNestedAlternativesWithIdenticalPrefix_bugPathIdentityCheckDoesNotConsiderContext_expectContextCheckInResult() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: {S} $$ p0 $$?=> (\n" + + " 'a' \n" + + " | 'a' 'b' \n" + + " ) \n" + + " | {S} $$ p1 $$?=> (\n" + + " 'a' \n" + + " | 'a' 'c' \n" + + " ) \n" + + " | {S} $$ p2 $$?=> 'd' ;\n"; + // @formatter:off + XtextResource resource = getResourceFromString(model); + Grammar grammar = ((Grammar) resource.getContents().get(0)); + hoistingProcessor.init(grammar); + AbstractRule rule = getRule(grammar, "S"); - hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + HoistingGuard guard = hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + assertFalse(guard.isTrivial()); + assertTrue(guard.hasTerminal()); + System.out.println(guard.render()); + assertEquals( + "(" + + "(" + getSyntaxForKeywordToken("a", 1) + " || (" + getSyntaxForEofToken(2) + " && " + getSyntaxForKeywordToken("b", 2) + ") || (p0)) && " + + "(" + getSyntaxForKeywordToken("a", 1) + " || (" + getSyntaxForEofToken(2) + " && " + getSyntaxForKeywordToken("c", 2) + ") || (p1)) && " + + "(" + getSyntaxForKeywordToken("d", 1) + " || (p2))" + + ")", + guard.render() + ); } // symbolic analysis not yet implemented diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysis.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysis.java index ba8bcb15e..81231a4f2 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysis.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysis.java @@ -30,6 +30,8 @@ import org.eclipse.xtext.CompoundElement; import org.eclipse.xtext.Grammar; import org.eclipse.xtext.GrammarUtil; import org.eclipse.xtext.Group; +import org.eclipse.xtext.Keyword; +import org.eclipse.xtext.ParserRule; import org.eclipse.xtext.RuleCall; import org.eclipse.xtext.UnorderedGroup; import org.eclipse.xtext.util.XtextSwitch; @@ -96,12 +98,10 @@ public class TokenAnalysis { log.info("getNext: " + abstractElementToShortString(last)); log.info("container: " + abstractElementToShortString(container)); - final AbstractElement finalLast = last; - if (container instanceof UnorderedGroup) { List result = new ArrayList<>(); result.addAll(container.getElements().stream() - .filter(e -> e != finalLast).collect(Collectors.toList()) + .collect(Collectors.toList()) ); result.addAll(getNextElementsInContext(container)); return result; @@ -156,11 +156,17 @@ public class TokenAnalysis { // TODO: is this special case necessary? throw new TokenAnalysisAbortedException("context analysis failed: no context"); } + + int currentPosition = prefix.getMinPosition(); for (AbstractElement element : context) { TokenAnalysisPaths path = new TokenAnalysisPaths(prefix); path = getTokenPaths(element, path, false, false); if (!path.isDone() && element != null) { + if (path.getMinPosition() == currentPosition) { + throw new TokenAnalysisAbortedException("no progress in context analysis"); + } + path = getTokenPathsContext(element, path); } if (path.isDone()) { @@ -205,12 +211,18 @@ public class TokenAnalysis { result = TokenAnalysisPaths.empty(prefix); } + int currentPosition = result.getMinPosition(); + do { current = TokenAnalysisPaths.empty(result); for (AbstractElement element : path.getElements()) { current = current.merge(getTokenPaths(element, result, false, false)); } + if (current.getMinPosition() == currentPosition) { + throw new TokenAnalysisAbortedException("no progress in loop"); + } + result = result.merge(current); } while(!current.isDone()); @@ -291,7 +303,9 @@ public class TokenAnalysis { boolean loop = isMultipleCardinality(path); - do { + int currentPosition = result.getMinPosition(); + + do { TokenAnalysisPaths tokenPaths = getTokenPathsTrivial(path, result); if (tokenPaths.isDone()) { @@ -305,6 +319,14 @@ public class TokenAnalysis { } else { result = result.merge(tokenPaths); } + + if (loop) { + if (result.getMinPosition() == currentPosition) { + throw new TokenAnalysisAbortedException("no progress in loop"); + } else { + currentPosition = result.getMinPosition(); + } + } } while (loop); return result; @@ -330,36 +352,43 @@ public class TokenAnalysis { private boolean arePathsIdenticalFallback(AbstractElement path1, AbstractElement path2) { // + 1, because otherwise identical paths of length token limit can't be checked - for (int i = 0; i < config.getTokenLimit() + 1; i++) { - Set> tokenListSet1; - Set> tokenListSet2; + + log.info("path1: " + abstractElementToString(path1)); + log.info("path2: " + abstractElementToString(path2)); + + int i; + int limit = config.getTokenLimit() + 1; + for (i = 0; i < limit; i++) { + TokenAnalysisPaths tokenPaths1; + TokenAnalysisPaths tokenPaths2; List range = range(0, i); - try { - tokenListSet1 = new HashSet<>(getTokenPaths(path1, range, false)); - } catch (TokenAnalysisAbortedException e) { - tokenListSet1 = null; - } + // there shouldn't be a TokenAnalysisAbortedException if needsLength is false + tokenPaths1 = getTokenPaths(path1, new TokenAnalysisPaths(range), false, false); + tokenPaths2 = getTokenPaths(path2, new TokenAnalysisPaths(range), false, false); - try { - tokenListSet2 = new HashSet<>(getTokenPaths(path2, range, false)); - } catch (TokenAnalysisAbortedException e) { - tokenListSet2 = null; - } + Set> tokenListSet1 = tokenPaths1.getTokenPaths().stream().map(HashSet::new).collect(Collectors.toSet()); + Set> tokenListSet2 = tokenPaths2.getTokenPaths().stream().map(HashSet::new).collect(Collectors.toSet()); + + int maxPosition1 = tokenPaths1.getMaxPosition(); + int maxPosition2 = tokenPaths2.getMaxPosition(); + + log.info("set1: " + tokenListSet1 + ", " + maxPosition1); + log.info("set2: " + tokenListSet2 + ", " + maxPosition2); - if (tokenListSet1 == null && tokenListSet2 == null) { - return true; - } - if (tokenListSet1 == null || tokenListSet2 == null) { - // the paths have different length - return false; - } if (!tokenListSet1.equals(tokenListSet2)) { return false; } + + if (maxPosition1 < i + 1) { + // different max positions would have failed the equals-Operation + // if the max position is smaller than i + 1 the end of the path has been reached + return true; + } } + // token analysis limit reached // we can't analyze the paths any further // TODO maybe assume paths are equal and show warning instead of exception throw new TokenAnalysisAbortedException("token limit exhausted while looking for identical paths"); @@ -383,7 +412,6 @@ public class TokenAnalysis { } } // we tried all possible combinations - // TODO: can only happen if no symbolic analysis is implemented // -> abort if (limit.get().equals(config.getTokenLimit())) { throw new TokenAnalysisAbortedException("token limit exhausted while searching for minimal differences"); diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPath.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPath.java index 365492503..60c50aa6f 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPath.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPath.java @@ -32,6 +32,10 @@ public class TokenAnalysisPath { path = new LinkedList<>(prefix.path); position = prefix.position; } + + int getPosition() { + return position - 1; + } public boolean isDone() { return remainingIndexes.isEmpty(); diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPaths.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPaths.java index 850fb967c..6379e8153 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPaths.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/pathAnalysis/TokenAnalysisPaths.java @@ -63,6 +63,14 @@ public class TokenAnalysisPaths { return empty; } + public int getMinPosition() { + return tokenPaths.stream().map(TokenAnalysisPath::getPosition).mapToInt(Integer::intValue).min().getAsInt(); + } + + public int getMaxPosition() { + return tokenPaths.stream().map(TokenAnalysisPath::getPosition).mapToInt(Integer::intValue).max().getAsInt(); + } + @Override public String toString() { if (isEmpty) {