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 835348fb1..004b4844d 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 @@ -20,7 +20,9 @@ import org.eclipse.xtext.resource.XtextResource; 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.HoistingException; 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; @@ -225,11 +227,11 @@ public class HoistingProcessorTest extends AbstractXtextTests { } @Test - public void testCardinalityPlusWithTokens() throws Exception { + public void testCardinalityPlusPredicate_expectPredicateAfterGroupNotInGuard() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + - "S: ($$ p0 $$?=> 'a')+ $$ p1 $$?=> 's';"; + "S: ($$ p0 $$?=> 'a')+ $$ p1 $$?=> ;"; // @formatter:off XtextResource resource = getResourceFromString(model); Grammar grammar = ((Grammar) resource.getContents().get(0)); @@ -242,7 +244,7 @@ public class HoistingProcessorTest extends AbstractXtextTests { } @Test - public void testCardinalityQuestionmarkWithTokens() throws Exception { + public void testCardinalityQuestionmarkWithContext() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -257,9 +259,23 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertTrue(guard.hasTerminal()); assertEquals("((" + getSyntaxForKeywordToken("s", 1) + " || (p1)) && (" + getSyntaxForKeywordToken("a", 1) + " || (p0)))", guard.render()); } + + @Test(expected = UnsupportedConstructException.class) + public void testCardinalityQuestionmarkWithoutContext_expectUnsupportedConstruct() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: ($$ p0 $$?=> 'a')?;"; + // @formatter:off + XtextResource resource = getResourceFromString(model); + Grammar grammar = ((Grammar) resource.getContents().get(0)); + AbstractRule rule = getRule(grammar, "S"); + + hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + } @Test - public void testCardinalityStarWithTokens() throws Exception { + public void testCardinalityStarWithContext() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -275,9 +291,23 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertEquals("((" + getSyntaxForKeywordToken("s", 1) + " || (p1)) && (" + getSyntaxForKeywordToken("a", 1) + " || (p0)))", guard.render()); } + @Test(expected = UnsupportedConstructException.class) + public void testCardinalityStarWithoutContext_expectUnsupporedConstruct() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: ($$ p0 $$?=> 'a')* ;"; + // @formatter:off + XtextResource resource = getResourceFromString(model); + Grammar grammar = ((Grammar) resource.getContents().get(0)); + AbstractRule rule = getRule(grammar, "S"); + + hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + } + @Test - public void testUnorderedGroups() throws Exception { + public void testUnorderedGroup() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -293,8 +323,39 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); } + @Test + public void testUnorderedGroupWithNoEmptyPathsWithoutContext() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: ($$ p0 $$?=> 'a') & ($$ p1 $$?=> 'b');"; + // @formatter:off + XtextResource resource = getResourceFromString(model); + Grammar grammar = ((Grammar) resource.getContents().get(0)); + AbstractRule rule = getRule(grammar, "S"); + + HoistingGuard guard = hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + assertFalse(guard.isTrivial()); + assertTrue(guard.hasTerminal()); + assertEquals("((" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); + } + + @Test(expected = UnsupportedConstructException.class) + public void testUnorderedGroupWithEmptyPathsWithoutContext_expectUnsupportedConstruct() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: ($$ p0 $$?=> 'a')? & ($$ p1 $$?=> 'b');"; + // @formatter:off + XtextResource resource = getResourceFromString(model); + Grammar grammar = ((Grammar) resource.getContents().get(0)); + AbstractRule rule = getRule(grammar, "S"); + + hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + } + //@Test - public void testUnorderedGroupsWithoutMandatoryContent() throws Exception { + public void testUnorderedGroupWithoutMandatoryContent() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + @@ -310,8 +371,22 @@ public class HoistingProcessorTest extends AbstractXtextTests { assertEquals("((" + getSyntaxForKeywordToken("s", 1) + " || (p2)) && (" + getSyntaxForKeywordToken("a", 1) + " || (p0)) && (" + getSyntaxForKeywordToken("b", 1) + " || (p1)))", guard.render()); } + @Test(expected = UnsupportedConstructException.class) + public void testUnorderedGroupWithoutMandatoryContentWithoutContext_expectUnsupportedConstruct() throws Exception { + // @formatter:off + String model = + MODEL_PREAMBLE + + "S: ($$ p0 $$?=> 'a')? & ($$ p1 $$?=> 'b')?;"; + // @formatter:off + XtextResource resource = getResourceFromString(model); + Grammar grammar = ((Grammar) resource.getContents().get(0)); + AbstractRule rule = getRule(grammar, "S"); + + hoistingProcessor.findHoistingGuard(rule.getAlternatives()); + } + @Test - public void testUnorderedGroups_bugGroupsChangeDuringHoisting_expectNoChange() throws Exception { + public void testUnorderedGroup_bugGroupsChangeDuringHoisting_expectNoChange() throws Exception { // @formatter:off String model = MODEL_PREAMBLE + 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 4aa85c17a..a2a4cb718 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 @@ -70,7 +70,7 @@ public class HoistingProcessor { } // TODO: handling for TokenAnalysisAbortedException - private HoistingGuard findGuardForAlternatives(Alternatives alternatives, AbstractRule currentRule) { + private HoistingGuard findGuardForAlternatives(CompoundElement alternatives, AbstractRule currentRule) { log.info("find guard for alternative"); List paths = new ArrayList<>(alternatives.getElements()); @@ -148,17 +148,15 @@ public class HoistingProcessor { for (int i = 0; i < size; i++) { AbstractElement element = elements.get(i); if (element instanceof UnorderedGroup) { - // Unordered group (A & B) is the same as (A | B)* + // Unordered group (A & B) is the same as (A | B)+ or (A | B)* (is A and B are optional) // -> create virtual element Alternatives virtualAlternatives = XtextFactory.eINSTANCE.createAlternatives(); addElementsToCompoundElement(virtualAlternatives, ((UnorderedGroup) element).getElements()); - if (((UnorderedGroup) element).getElements().stream().allMatch(GrammarUtil::isOptionalCardinality)) { - virtualAlternatives.setCardinality("*"); - // TODO: alternatives analysis needs special case - } else { - virtualAlternatives.setCardinality("+"); - } + + // if A and B are optional the cardinality would be * + // but it doesn't matter for hoisting + virtualAlternatives.setCardinality("+"); elements.set(i, virtualAlternatives); } @@ -367,16 +365,28 @@ public class HoistingProcessor { } else if (element instanceof JavaAction) { return HoistingGuard.action(); } else if (element instanceof UnorderedGroup) { - if (pathHasTokenOrAction(element)) { - // if unordered group has tokens or actions we need the context which is not available here - // only works when analyzing groups - - // TODO: maybe add warning and return unguarded - throw new UnsupportedConstructException("unordered groups are only supported in groups", currentRule); + if (((UnorderedGroup) element).getElements().stream().allMatch(GrammarUtil::isOptionalCardinality)) { + if (pathHasTokenOrAction(element)) { + // if unordered group has tokens or actions we need the context which is not available here + // only works when analyzing groups + + // TODO: maybe add warning and return unguarded + throw new UnsupportedConstructException("unordered groups with hoisting-relevant elements and optional cardinalities are only supported in groups", currentRule); + } else { + // the path is accessible whether or not any guard is satisfied + // -> assume it's unguarded + return HoistingGuard.unguarded(); + } } else { - // the path is accessible whether or not any guard is satisfied - // -> assume it's unguarded - return HoistingGuard.unguarded(); + if (pathHasTokenOrAction(element)) { + // there is no context but it might still be possible to find a guard for this group + // only works if none of the paths is empty or optional + return findGuardForAlternatives((CompoundElement) element, currentRule); + } else { + // the path is accessible whether or not any guard is satisfied + // -> assume it's unguarded + return HoistingGuard.unguarded(); + } } } else if (element instanceof Assignment) { return findGuardForElement(((Assignment) element).getTerminal(), currentRule);