changed unordered group hoisting to just use + instead

cardinality * is only possible if all element in the unordered group are
optional. This case doesn't matter for hoisting or rather is already
dealt with in the containing group (the guard of the resulting
alternatives won't have a terminal).
This commit is contained in:
overflowerror 2021-12-04 17:28:45 +01:00
parent 1d4b649755
commit 1058d2bf3c
2 changed files with 109 additions and 24 deletions

View file

@ -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 +

View file

@ -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<AbstractElement> 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);