From 428dfb93d0a618279493bb6ead09552d2c5582e8 Mon Sep 17 00:00:00 2001 From: overflowerror Date: Sat, 25 Dec 2021 16:10:54 +0100 Subject: [PATCH] fixed path collapse collapsed paths loose the containing token guard => fixed problem + added positional condition in token sequence guard constructor to not check positions twice (order matters: give local token guard first in case it is sufficient) removed testNestedAlternativesWithSingleTokenDifference, because it is wrong: 'a' 'b' 'd' with p1 satisfies semantic predicates but not the generated guard condition --- .../hoisting/HoistingProcessorTest.java | 72 ++++++++++++------- .../antlr/hoisting/HoistingProcessor.java | 12 +++- .../guards/AlternativeTokenSequenceGuard.java | 17 +++++ .../antlr/hoisting/guards/PathGuard.java | 2 +- .../hoisting/guards/SingleTokenGuard.java | 9 +++ .../antlr/hoisting/guards/TokenGuard.java | 4 ++ .../hoisting/guards/TokenSequenceGuard.java | 38 +++++++++- .../parser/antlr/hoisting/token/EofToken.java | 4 ++ .../antlr/hoisting/token/KeywordToken.java | 4 ++ .../hoisting/token/TerminalRuleToken.java | 5 ++ .../parser/antlr/hoisting/token/Token.java | 2 + 11 files changed, 139 insertions(+), 30 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 3226fc7a6..43620d876 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 @@ -722,30 +722,6 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertEquals("(" + getSyntaxForKeywordToken("a", 1) + " || (p0))", guard.render()); } - @Test - public void testNestedAlternativesWithSingleTokenDifference() throws Exception { - // @formatter:off - String model = - MODEL_PREAMBLE + - "S: $$ p0 $$?=> a=A \n" + - " | $$ p1 $$?=> b=B ;\n" + - "A: {A} $$ p2 $$?=> 'a' 'b' 'c' \n" + - " | {A} $$ p3 $$?=> 'a' 'c' 'c' ;\n" + - "B: {B} 'a' 'c' 'd' \n" + - " | {B} 'a' 'b' 'd' ;\n"; - - // @formatter:off - XtextResource resource = getResourceFromString(model); - 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()); - assertEquals("((" + getSyntaxForKeywordToken("b", 2) + " || ((p0) && (p2))) && (" + getSyntaxForKeywordToken("c", 2) + " || ((p0) && (p3))) && (" + getSyntaxForKeywordToken("d", 3) + " || (p1)))", guard.render()); - } - @Test public void testNestedAlternativesWithNoSingleTokenDifference() throws Exception { // @formatter:off @@ -772,7 +748,7 @@ public class HoistingProcessorTest extends AbstractXtextTests { } @Test - public void testNestedAlternativesWithCollapsablePaths() throws Exception { + public void testNestedAlternativesWithCollapsablePathsWithSingleDifferencePosition() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -794,6 +770,52 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertEquals("((" + getSyntaxForKeywordToken("b", 3) + " || ((p0) && (p2))) && (" + getSyntaxForKeywordToken("c", 3) + " || ((p0) && (p3))) && (" + getSyntaxForKeywordToken("d", 3) + " || (p1)))", guard.render()); } + @Test + public void testNestedAlternativesWithCollapsablePathsWithMultipleDifferencePosition() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: $$ p0 $$?=> a=A \n" + + " | $$ p1 $$?=> 'a' 'b' ;\n" + + "A: {A} $$ p2 $$?=> 'b' 'a' \n" + + " | {A} $$ p3 $$?=> 'b' 'b' ;"; + + // @formatter:off + XtextResource resource = getResourceFromString(model); + 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()); + + assertEquals("((" + getSyntaxForKeywordToken("a", 2) + " || " + getSyntaxForKeywordToken("b", 1) + " || ((p0) && (p2))) && (" + getSyntaxForKeywordToken("b", 2) + " || " + getSyntaxForKeywordToken("b", 1) + " || ((p0) && (p3))) && (" + getSyntaxForKeywordToken("a", 1) + " || (p1)))", guard.render()); + } + + @Test + public void testRecursiveRuleCallingAlternative_expectCorrectGuard() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: $$ p0 $$?=> a=A \n" + + " | $$ p1 $$?=> 'a' 'b' 'd' ;\n" + + "A: $$ p2 $$?=> 'a' a=A \n" + + " | $$ p3 $$?=> 'b' 'c' ;\n"; + + // @formatter:off + XtextResource resource = getResourceFromString(model); + 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()); + + assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (" + getSyntaxForKeywordToken("c", 3) + " && " + getSyntaxForEofToken(3) + ") || ((p0) && (p2))) && (" + getSyntaxForKeywordToken("c", 1) + " || ((p0) && (p3))) && (" + getSyntaxForKeywordToken("d", 3) + " || (p1)))", guard.render()); + } + @Test public void testAlternativeEmptyAndNonEmptyPaths_expectEofCheck() throws Exception { // @formatter:off diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/HoistingProcessor.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/HoistingProcessor.java index 4c55ffed3..51df56ba9 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/HoistingProcessor.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/HoistingProcessor.java @@ -273,7 +273,7 @@ public class HoistingProcessor { // -> size = 1 if (size > 1) { try { - return StreamUtils.zip( + AlternativesGuard result = StreamUtils.zip( analysis.findMinimalPathDifference(paths).stream() .map(a -> a.stream() .map(s -> s.stream() @@ -281,15 +281,21 @@ public class HoistingProcessor { .collect(Collectors.toList()) ) .map(TokenSequenceGuard::new) + .peek(g -> log.info(g)) .map(TokenGuard::reduce) + .peek(g -> log.info(g)) .collect(Collectors.toList()) ) .map(AlternativeTokenSequenceGuard::new) - .map(TokenGuard::reduce), + .peek(g -> log.info(g)) + .map(TokenGuard::reduce) + .peek(g -> log.info(g)), guards.stream(), (TokenGuard tokenGuard, MergedPathGuard pathGuard) -> Tuples.pair(tokenGuard, pathGuard) ).map(p -> new PathGuard(p.getFirst(), p.getSecond())) .collect(AlternativesGuard.collector()); + log.info(result); + return result; } catch(NestedPrefixAlternativesException e) { // nested prefix alternatives // -> flatten paths to alternative and try again @@ -308,7 +314,7 @@ public class HoistingProcessor { log.info(flattened.getElements().size()); // TODO: value configurable? if (flattened.getElements().size() > 100) { - throw new NestedPrefixAlternativesException("nested prefix alternatives cant be analysed because of too many paths"); + throw new NestedPrefixAlternativesException("nested prefix alternatives can't be analysed because of too many paths"); } //throw new RuntimeException(); diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/AlternativeTokenSequenceGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/AlternativeTokenSequenceGuard.java index df6485bbb..531e873ef 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/AlternativeTokenSequenceGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/AlternativeTokenSequenceGuard.java @@ -10,6 +10,8 @@ package org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.guards; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; +import java.util.Set; import java.util.stream.Collectors; /** @@ -56,4 +58,19 @@ public class AlternativeTokenSequenceGuard implements TokenGuard { ")\n"; } + + @Override + public Set getPositions() { + if (alternatives.isEmpty()) { + return new HashSet<>(); + } + + Set> positions = alternatives.stream() + .map(TokenGuard::getPositions) + .collect(Collectors.toSet()); + return positions.stream().findAny().get().stream() + .filter(p -> positions.stream() + .allMatch(s -> s.contains(p))) + .collect(Collectors.toSet()); + } } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/PathGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/PathGuard.java index de4305aa0..039de7ba6 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/PathGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/PathGuard.java @@ -92,7 +92,7 @@ public class PathGuard implements HoistingGuard { // construct new path guard and add to result GroupGuard groupGuard = new GroupGuard(destructedPaths.getFirst()); groupGuard.add(p.hoistngGuard); - result.add(new PathGuard(p.tokenGuard, groupGuard)); + result.add(new PathGuard(new TokenSequenceGuard(p.tokenGuard, path.tokenGuard), groupGuard)); }); } } else { diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/SingleTokenGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/SingleTokenGuard.java index 205b1fe53..75c2528c8 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/SingleTokenGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/SingleTokenGuard.java @@ -8,6 +8,10 @@ *******************************************************************************/ package org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.guards; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + import org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.token.Token; /** @@ -36,4 +40,9 @@ public class SingleTokenGuard implements TokenGuard { "\t" + token + "\n" + ")\n"; } + + @Override + public Set getPositions() { + return new HashSet<>(Arrays.asList(token.getPosition())); + } } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenGuard.java index 443b300a7..250511461 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenGuard.java @@ -8,6 +8,8 @@ *******************************************************************************/ package org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.guards; +import java.util.Set; + /** * @author overflow - Initial contribution and API */ @@ -17,5 +19,7 @@ public interface TokenGuard extends Guard { return false; } + Set getPositions(); + TokenGuard reduce(); } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenSequenceGuard.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenSequenceGuard.java index abf445147..3144790d8 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenSequenceGuard.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/guards/TokenSequenceGuard.java @@ -10,7 +10,12 @@ package org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.guards; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; +import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.eclipse.xtext.xtext.generator.parser.antlr.hoisting.utils.StreamUtils; /** * @author overflow - Initial contribution and API @@ -19,7 +24,30 @@ public class TokenSequenceGuard implements TokenGuard { private Collection sequence; public TokenSequenceGuard(Collection sequence) { - this.sequence = sequence; + Set checkedPositions = new HashSet<>(); + this.sequence = sequence.stream() + .flatMap(g -> { + if (g instanceof TokenSequenceGuard) { + return ((TokenSequenceGuard) g).sequence.stream() + .filter(s -> !s.getPositions().stream() + .allMatch(checkedPositions::contains)) + .peek(s -> checkedPositions.addAll(s.getPositions())); + } else { + Set positions = g.getPositions(); + if (positions.stream() + .allMatch(checkedPositions::contains) + ) { + return Stream.empty(); + } else { + checkedPositions.addAll(positions); + return Stream.of(g); + } + } + }).collect(StreamUtils.collectToLinkedHashSet()); + } + + public TokenSequenceGuard(TokenGuard ...guards) { + this(Arrays.asList(guards)); } @Override @@ -67,4 +95,12 @@ public class TokenSequenceGuard implements TokenGuard { .collect(Collectors.joining("\n")) + ")\n"; } + + @Override + public Set getPositions() { + return sequence.stream() + .map(TokenGuard::getPositions) + .flatMap(Set::stream) + .collect(Collectors.toSet()); + } } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/EofToken.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/EofToken.java index ab3bca0b6..7d6c1fb52 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/EofToken.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/EofToken.java @@ -58,4 +58,8 @@ public class EofToken implements Token { return null; } + @Override + public int getPosition() { + return position; + } } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/KeywordToken.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/KeywordToken.java index 757257f1b..403844d62 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/KeywordToken.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/KeywordToken.java @@ -66,4 +66,8 @@ public class KeywordToken implements Token { return keyword; } + @Override + public int getPosition() { + return position; + } } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/TerminalRuleToken.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/TerminalRuleToken.java index 224bb0161..577094672 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/TerminalRuleToken.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/TerminalRuleToken.java @@ -68,4 +68,9 @@ public class TerminalRuleToken implements Token { public AbstractElement getElement() { return call; } + + @Override + public int getPosition() { + return position; + } } diff --git a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/Token.java b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/Token.java index 7e1c6ba30..b6965f444 100644 --- a/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/Token.java +++ b/org.eclipse.xtext.xtext.generator/src/org/eclipse/xtext/xtext/generator/parser/antlr/hoisting/token/Token.java @@ -53,4 +53,6 @@ public interface Token { throw new NotATokenException(element.eClass().getName()); } + + int getPosition(); }