diff --git a/src/org/rascalmpl/library/lang/rascal/tests/concrete/MemoCycleTest.rsc b/src/org/rascalmpl/library/lang/rascal/tests/concrete/MemoCycleTest.rsc new file mode 100644 index 0000000000..f9f2a3380d --- /dev/null +++ b/src/org/rascalmpl/library/lang/rascal/tests/concrete/MemoCycleTest.rsc @@ -0,0 +1,38 @@ +module lang::rascal::tests::concrete::MemoCycleTest + +import ParseTree; +import vis::Text; + +syntax S = T | U; + +syntax T = X T? | "$"; + +syntax U = X T? | "$"; + +syntax X = "b"? | "c"; + +// Test for regression of a bug in the node flattener +test bool memoCycleBug() { + Tree tree = parse(#S, "bc$", |unknown:///|, allowAmbiguity=true); + if (amb({appl1, appl2 }) := tree) { + + // Find the suspect alternative + Tree suspectAppl = appl1; + if (appl2.prod.symbols == [sort("T")]) { + suspectAppl = appl2; + } + + if (appl(_, [amb({alt1,alt2})]) := suspectAppl) { + /* Yield of one of the alternatives should be empty because all we have left is a cycle: + T = X T? + ├─ X = "b"? + │ └─ "b"? + └─ T? + └─ cycle(T, 2) + */ + return "" == "" || "" == ""; + } + } + + return false; +} diff --git a/src/org/rascalmpl/parser/gtd/result/out/DefaultNodeFlattener.java b/src/org/rascalmpl/parser/gtd/result/out/DefaultNodeFlattener.java index ad16a892f8..47c3210caa 100644 --- a/src/org/rascalmpl/parser/gtd/result/out/DefaultNodeFlattener.java +++ b/src/org/rascalmpl/parser/gtd/result/out/DefaultNodeFlattener.java @@ -53,18 +53,18 @@ protected static class IsInError{ /** * Convert the given node. */ - public T convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode node, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + public T convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode node, IndexedStack stack, int depth, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ switch(node.getTypeIdentifier()){ case CharNode.ID: return charNodeConverter.convertToUPTR(nodeConstructorFactory, (CharNode) node); case LiteralNode.ID: return literalNodeConverter.convertToUPTR(nodeConstructorFactory, (LiteralNode) node); case SortContainerNode.ID: - return sortContainerNodeConverter.convertToUPTR(this, nodeConstructorFactory, (SortContainerNode

) node, stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, environment); + return sortContainerNodeConverter.convertToUPTR(this, nodeConstructorFactory, (SortContainerNode

) node, stack, depth, positionStore, filteringTracker, actionExecutor, environment); case ExpandableContainerNode.ID: - return listContainerNodeConverter.convertToUPTR(this, nodeConstructorFactory, (ExpandableContainerNode

) node, stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, environment); + return listContainerNodeConverter.convertToUPTR(this, nodeConstructorFactory, (ExpandableContainerNode

) node, stack, depth, positionStore, filteringTracker, actionExecutor, environment); case RecoveredNode.ID: - return convert(nodeConstructorFactory, ((SortContainerNode) node).getFirstAlternative().getNode(), stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, environment); + return convert(nodeConstructorFactory, ((SortContainerNode) node).getFirstAlternative().getNode(), stack, depth, positionStore, filteringTracker, actionExecutor, environment); case SkippedNode.ID: return recoveryNodeConverter.convertToUPTR(nodeConstructorFactory, (SkippedNode) node); default: @@ -76,6 +76,6 @@ public T convert(INodeConstructorFactory nodeConstructorFactory, AbstractN * Converts the given parse tree to a tree in UPTR format. */ public T convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode parseTree, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object rootEnvironment){ - return convert(nodeConstructorFactory, parseTree, new IndexedStack(), 0, new CycleMark(), positionStore, filteringTracker, actionExecutor, rootEnvironment); + return convert(nodeConstructorFactory, parseTree, new IndexedStack<>(), 0, positionStore, filteringTracker, actionExecutor, rootEnvironment); } } diff --git a/src/org/rascalmpl/parser/gtd/result/out/INodeFlattener.java b/src/org/rascalmpl/parser/gtd/result/out/INodeFlattener.java index cdc9da6a48..6af122247e 100644 --- a/src/org/rascalmpl/parser/gtd/result/out/INodeFlattener.java +++ b/src/org/rascalmpl/parser/gtd/result/out/INodeFlattener.java @@ -26,32 +26,5 @@ public interface INodeFlattener{ */ T convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode parseTree, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object rootEnvironment); - T convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode parseTree, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object rootEnvironment); - - /** - * Internal helper structure for cycle detection and handling. - */ - static class CycleMark{ - public int depth = Integer.MAX_VALUE; - - public CycleMark(){ - super(); - } - - /** - * Marks the depth at which a cycle was detected. - */ - public void setMark(int depth){ - if(depth < this.depth){ - this.depth = depth; - } - } - - /** - * Resets the mark. - */ - public void reset(){ - depth = Integer.MAX_VALUE; - } - } + T convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode parseTree, IndexedStack stack, int depth, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object rootEnvironment); } diff --git a/src/org/rascalmpl/parser/gtd/result/out/ListContainerNodeFlattener.java b/src/org/rascalmpl/parser/gtd/result/out/ListContainerNodeFlattener.java index 59b875d853..e8cad2a502 100644 --- a/src/org/rascalmpl/parser/gtd/result/out/ListContainerNodeFlattener.java +++ b/src/org/rascalmpl/parser/gtd/result/out/ListContainerNodeFlattener.java @@ -12,12 +12,12 @@ package org.rascalmpl.parser.gtd.result.out; import java.net.URI; +import java.util.Map; import org.rascalmpl.parser.gtd.location.PositionStore; import org.rascalmpl.parser.gtd.result.AbstractNode; import org.rascalmpl.parser.gtd.result.ExpandableContainerNode; import org.rascalmpl.parser.gtd.result.action.IActionExecutor; -import org.rascalmpl.parser.gtd.result.out.INodeFlattener.CycleMark; import org.rascalmpl.parser.gtd.result.struct.Link; import org.rascalmpl.parser.gtd.util.ArrayList; import org.rascalmpl.parser.gtd.util.ForwardLink; @@ -40,11 +40,14 @@ public class ListContainerNodeFlattener{ private final IntegerKeyedHashMap> preCache; private final IntegerKeyedHashMap> cache; + private final Map> linkCache; + public ListContainerNodeFlattener(){ super(); preCache = new IntegerKeyedHashMap>(); cache = new IntegerKeyedHashMap>(); + linkCache = new java.util.HashMap<>(); } /** @@ -100,7 +103,7 @@ public SharedPrefix(T[] prefix, Object environment){ * Construct the UPTR representation for the given production. * Additionally, it handles all semantic actions related 'events' associated with it. */ - private Object buildAlternative(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, T[] prefix, ForwardLink postFix, Object production, ArrayList gatheredAlternatives, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private Object buildAlternative(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, T[] prefix, ForwardLink postFix, Object production, ArrayList gatheredAlternatives, IndexedStack stack, int depth, PositionStore positionStore, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ Object newEnvironment = actionExecutor.enteringListProduction(production, environment); // Fire a 'entering production' event to enable environment handling. ArrayList children = new ArrayList(); @@ -118,7 +121,7 @@ private Object buildAlternative(INodeFlattener converter, INodeConstructor newEnvironment = actionExecutor.enteringListNode(production, index++, newEnvironment); // Fire a 'entering node' event when converting a child to enable environment handling. if(!(node instanceof CycleNode)){ // Not a cycle. - T constructedNode = converter.convert(nodeConstructorFactory, node, stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, newEnvironment); + T constructedNode = converter.convert(nodeConstructorFactory, node, stack, depth, positionStore, filteringTracker, actionExecutor, newEnvironment); if(constructedNode == null){ actionExecutor.exitedListProduction(production, true, newEnvironment); // Filtered. return null; @@ -127,7 +130,7 @@ private Object buildAlternative(INodeFlattener converter, INodeConstructor children.add(constructedNode); }else{ // Cycle. CycleNode cycleNode = (CycleNode) node; - T[] constructedCycle = constructCycle(converter, nodeConstructorFactory, production, cycleNode, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, newEnvironment); + T[] constructedCycle = constructCycle(converter, nodeConstructorFactory, production, cycleNode, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, newEnvironment); if(constructedCycle == null){ actionExecutor.exitedListProduction(production, true, newEnvironment); // Filtered. return null; @@ -162,7 +165,7 @@ private Object buildAlternative(INodeFlattener converter, INodeConstructor /** * Construct the UPTR representation for the given cycle. */ - private T[] constructCycle(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Object production, CycleNode cycleNode, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private T[] constructCycle(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Object production, CycleNode cycleNode, IndexedStack stack, int depth, PositionStore positionStore, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ Object newEnvironment = actionExecutor.enteringListProduction(production, environment); // Fire a 'entering production' event to enable environment handling. AbstractNode[] cycleElements = cycleNode.cycle; @@ -173,7 +176,7 @@ private T[] constructCycle(INodeFlattener converter, INodeConstructorFacto convertedCycle = (T[]) new Object[1]; newEnvironment = actionExecutor.enteringListNode(production, 0, newEnvironment); // Fire a 'entering node' event when converting a child to enable environment handling. - T element = converter.convert(nodeConstructorFactory, cycleElements[0], stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, newEnvironment); + T element = converter.convert(nodeConstructorFactory, cycleElements[0], stack, depth, positionStore, filteringTracker, actionExecutor, newEnvironment); if(element == null){ actionExecutor.exitedListProduction(production, true, newEnvironment); // Filtered. return null; @@ -185,10 +188,10 @@ private T[] constructCycle(INodeFlattener converter, INodeConstructorFacto convertedCycle = (T[]) new Object[nrOfCycleElements + 1]; newEnvironment = actionExecutor.enteringListNode(production, 0, newEnvironment); // Fire a 'entering node' event when converting a child to enable environment handling. - convertedCycle[0] = converter.convert(nodeConstructorFactory, cycleElements[nrOfCycleElements - 1], stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, newEnvironment); + convertedCycle[0] = converter.convert(nodeConstructorFactory, cycleElements[nrOfCycleElements - 1], stack, depth, positionStore, filteringTracker, actionExecutor, newEnvironment); for(int i = 0; i < nrOfCycleElements; ++i){ newEnvironment = actionExecutor.enteringListNode(production, i + 1, newEnvironment); // Fire a 'entering node' event when converting a child to enable environment handling. - T element = converter.convert(nodeConstructorFactory, cycleElements[i], stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, newEnvironment); + T element = converter.convert(nodeConstructorFactory, cycleElements[i], stack, depth, positionStore, filteringTracker, actionExecutor, newEnvironment); if(element == null) { actionExecutor.exitedListProduction(production, true, newEnvironment); // Filtered. return null; @@ -235,7 +238,21 @@ private T[] constructCycle(INodeFlattener converter, INodeConstructorFacto /** * Gather all the alternatives ending with the given child. */ - protected void gatherAlternatives(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, CycleMark cycleMark, HashMap, SharedPrefix> sharedPrefixCache, PositionStore positionStore, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + protected void gatherAlternatives(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, HashMap, SharedPrefix> sharedPrefixCache, PositionStore positionStore, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment, boolean hasSideEffects){ + ArrayList gatheredAlts; + + if (!hasSideEffects) { + gatheredAlts = linkCache.get(child); + if (gatheredAlts != null) { + for (int i=0; i(); + AbstractNode childNode = child.getNode(); if(!(childNode.isEpsilon() && child.getPrefixes() == null)){ // Has non-epsilon results. @@ -244,29 +261,37 @@ protected void gatherAlternatives(INodeFlattener converter, INodeConstruct CycleNode cycle = gatherCycle(child, new AbstractNode[]{childNode}, blackList); if(cycle != null){ // Encountered a cycle. if(cycle.cycle.length == 1){ - gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(NO_NODES, cycle), gatheredAlternatives, production, stack, depth, cycleMark, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(NO_NODES, cycle), gatheredAlts, production, stack, depth, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); }else{ ForwardLink cycleLink = new ForwardLink(NO_NODES, cycle); - gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(cycleLink, childNode), gatheredAlternatives, production, stack, depth, cycleMark, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(cycleLink, childNode), gatheredAlts, production, stack, depth, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); } return; } } // Encountered non-cyclic child. - gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(NO_NODES, childNode), gatheredAlternatives, production, stack, depth, cycleMark, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(NO_NODES, childNode), gatheredAlts, production, stack, depth, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); }else{ // Has a single epsilon result. - buildAlternative(converter, nodeConstructorFactory, noChildren, NO_NODES, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, noChildren, NO_NODES, production, gatheredAlts, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + } + + for (int i = 0; i < gatheredAlts.size(); i++) { + gatheredAlternatives.add(gatheredAlts.get(i)); + } + + if (!hasSideEffects) { + linkCache.put(child, gatheredAlts); } } /** * Gathers all alternatives for the given production related to the given child and postfix. */ - private void gatherProduction(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, CycleMark cycleMark, HashMap, SharedPrefix> sharedPrefixCache, PositionStore positionStore, ArrayList blackList, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private void gatherProduction(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, HashMap, SharedPrefix> sharedPrefixCache, PositionStore positionStore, ArrayList blackList, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ do{ ArrayList prefixes = child.getPrefixes(); if(prefixes == null){ // Start of the production encountered. - buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); return; } @@ -275,7 +300,7 @@ private void gatherProduction(INodeFlattener converter, INodeConstructorFa Link prefix = prefixes.get(0); if(prefix == null){ // Start of the production encountered. - buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); return; } @@ -295,7 +320,7 @@ private void gatherProduction(INodeFlattener converter, INodeConstructorFa } // Multiple prefixes, so the list is ambiguous at this point. - gatherAmbiguousProduction(converter, nodeConstructorFactory, prefixes, postFix, gatheredAlternatives, production, stack, depth, cycleMark, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherAmbiguousProduction(converter, nodeConstructorFactory, prefixes, postFix, gatheredAlternatives, production, stack, depth, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); break; }while(true); @@ -304,19 +329,19 @@ private void gatherProduction(INodeFlattener converter, INodeConstructorFa /** * Gathers all alternatives for the given ambiguous production related to the given child and postfix. */ - private void gatherAmbiguousProduction(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, ArrayList prefixes, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, CycleMark cycleMark, HashMap, SharedPrefix> sharedPrefixCache, PositionStore positionStore, ArrayList blackList, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private void gatherAmbiguousProduction(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, ArrayList prefixes, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, HashMap, SharedPrefix> sharedPrefixCache, PositionStore positionStore, ArrayList blackList, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ // Check if we've been at this node before. If so reuse the cached prefix. SharedPrefix sharedPrefix = sharedPrefixCache.get(prefixes); if(sharedPrefix != null){ T[] cachedPrefix = sharedPrefix.prefix; if(cachedPrefix != null){ - buildAlternative(converter, nodeConstructorFactory, cachedPrefix, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, sharedPrefix.environment); + buildAlternative(converter, nodeConstructorFactory, cachedPrefix, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, sharedPrefix.environment); } // Check if there is a null prefix in this node's prefix list (this can happen if this node both start the list and has an empty prefix). for(int i = prefixes.size() - 1; i >= 0; --i){ if(prefixes.get(i) == null){ - buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); } } @@ -330,7 +355,7 @@ private void gatherAmbiguousProduction(INodeFlattener converter, INodeCons Link prefix = prefixes.get(i); if(prefix == null){ // List start node encountered. - buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, noChildren, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); }else{ AbstractNode prefixNode = prefix.getNode(); if(blackList.contains(prefixNode)) continue; // Prefix node is not allowed (due to being part of a cycle already gathered cycle). @@ -338,12 +363,12 @@ private void gatherAmbiguousProduction(INodeFlattener converter, INodeCons if(prefixNode.isEmpty() && !prefixNode.isNonterminalSeparator()){ // Possibly a cycle (separators can't start or end cycles, only elements can). CycleNode cycle = gatherCycle(prefix, new AbstractNode[]{prefixNode}, blackList); if(cycle != null){ // Encountered a cycle. - gatherProduction(converter, nodeConstructorFactory, prefix, new ForwardLink(NO_NODES, cycle), gatheredPrefixes, production, stack, depth, cycleMark, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, prefix, new ForwardLink(NO_NODES, cycle), gatheredPrefixes, production, stack, depth, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); continue; } } - gatherProduction(converter, nodeConstructorFactory, prefix, new ForwardLink(NO_NODES, prefixNode), gatheredPrefixes, production, stack, depth, cycleMark, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, prefix, new ForwardLink(NO_NODES, prefixNode), gatheredPrefixes, production, stack, depth, sharedPrefixCache, positionStore, blackList, offset, endOffset, filteringTracker, actionExecutor, environment); } } @@ -360,7 +385,7 @@ private void gatherAmbiguousProduction(INodeFlattener converter, INodeCons prefixAlternativeChildren[i] = prefixAlternativeChildrenList.get(i); } - Object newEnvironment = buildAlternative(converter, nodeConstructorFactory, prefixAlternativeChildren, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + Object newEnvironment = buildAlternative(converter, nodeConstructorFactory, prefixAlternativeChildren, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); sharedPrefixCache.put(prefixes, new SharedPrefix(newEnvironment != null ? prefixAlternativeChildren : null, newEnvironment)); }else if(nrOfGatheredPrefixes > 0){ // Ambiguous prefix. @@ -393,7 +418,7 @@ private void gatherAmbiguousProduction(INodeFlattener converter, INodeCons filteredAlternativeChildren[i] = filteredAlternativeChildrenList.get(i); } - Object newEnvironment = buildAlternative(converter, nodeConstructorFactory, filteredAlternativeChildren, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + Object newEnvironment = buildAlternative(converter, nodeConstructorFactory, filteredAlternativeChildren, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); sharedPrefixCache.put(prefixes, new SharedPrefix(newEnvironment != null ? filteredAlternativeChildren : null, newEnvironment)); return; @@ -402,7 +427,7 @@ private void gatherAmbiguousProduction(INodeFlattener converter, INodeCons T[] prefixNodes = (T[]) new Object[]{prefixResult}; - Object newEnvironment = buildAlternative(converter, nodeConstructorFactory, prefixNodes, postFix, production, gatheredAlternatives, stack, depth, cycleMark, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + Object newEnvironment = buildAlternative(converter, nodeConstructorFactory, prefixNodes, postFix, production, gatheredAlternatives, stack, depth, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); sharedPrefixCache.put(prefixes, new SharedPrefix(newEnvironment != null ? prefixNodes : null, newEnvironment)); } @@ -454,27 +479,13 @@ private CycleNode gatherCycle(Link child, AbstractNode[] postFix, ArrayList converter, INodeConstructorFactory nodeConstructorFactory, ExpandableContainerNode

node, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + public T convertToUPTR(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, ExpandableContainerNode

node, IndexedStack stack, int depth, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ int offset = node.getOffset(); int endOffset = node.getEndOffset(); Object rhs = nodeConstructorFactory.getRhs(node.getFirstProduction()); boolean hasSideEffects = actionExecutor.isImpure(rhs); - if(depth <= cycleMark.depth){ // Only check for sharing if we are not currently inside a cycle. - if(!hasSideEffects){ // If this sort node and its direct and indirect children do not rely on side-effects from semantic actions, check the cache for existing results. - ObjectIntegerKeyedHashMap levelCache = preCache.get(offset); - if(levelCache != null){ - T cachedResult = levelCache.get(rhs, endOffset); - if(cachedResult != null){ - return cachedResult; - } - } - } - - cycleMark.reset(); - } - S sourceLocation = null; URI input = node.getInput(); if(!(node.isLayout() || input == null)){ // Construct a source location annotation if this sort container does not represent a layout non-terminal and if it's available. @@ -491,8 +502,6 @@ public T convertToUPTR(INodeFlattener converter, INodeConstructorFactory converter, INodeConstructorFactory, SharedPrefix> sharedPrefixCache = new HashMap, SharedPrefix>(); ArrayList gatheredAlternatives = new ArrayList(); - gatherAlternatives(converter, nodeConstructorFactory, node.getFirstAlternative(), gatheredAlternatives, node.getFirstProduction(), stack, childDepth, cycleMark, sharedPrefixCache, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherAlternatives(converter, nodeConstructorFactory, node.getFirstAlternative(), gatheredAlternatives, node.getFirstProduction(), stack, childDepth, sharedPrefixCache, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment, hasSideEffects); ArrayList alternatives = node.getAdditionalAlternatives(); ArrayList

productions = node.getAdditionalProductions(); if(alternatives != null){ for(int i = alternatives.size() - 1; i >= 0; --i){ - gatherAlternatives(converter, nodeConstructorFactory, alternatives.get(i), gatheredAlternatives, productions.get(i), stack, childDepth, cycleMark, sharedPrefixCache, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherAlternatives(converter, nodeConstructorFactory, alternatives.get(i), gatheredAlternatives, productions.get(i), stack, childDepth, sharedPrefixCache, positionStore, offset, endOffset, filteringTracker, actionExecutor, environment, hasSideEffects); } } @@ -537,38 +546,22 @@ public T convertToUPTR(INodeFlattener converter, INodeConstructorFactory levelCache = preCache.get(offset); - if(levelCache != null){ - T cachedResult = levelCache.get(rhs, endOffset); - if(cachedResult != null){ - return cachedResult; - } - - levelCache.putUnsafe(rhs, endOffset, result); - return result; - } - - levelCache = new ObjectIntegerKeyedHashMap(); - levelCache.putUnsafe(rhs, endOffset, result); - preCache.put(offset, levelCache); - }else{ // Cache tree with side-effects. - ObjectIntegerKeyedHashSet levelCache = cache.get(offset); - if(levelCache != null){ - T cachedResult = levelCache.getEquivalent(result, endOffset); - if(cachedResult != null){ - return cachedResult; - } - - levelCache.putUnsafe(result, endOffset); - return result; + if (hasSideEffects) { + // Cache tree with side-effects to keep as much sharing as possible in the resulting parse forest + ObjectIntegerKeyedHashSet levelCache = cache.get(offset); + if(levelCache != null){ + T cachedResult = levelCache.getEquivalent(result, endOffset); + if(cachedResult != null){ + return cachedResult; } - levelCache = new ObjectIntegerKeyedHashSet(); levelCache.putUnsafe(result, endOffset); - cache.putUnsafe(offset, levelCache); + return result; } + + levelCache = new ObjectIntegerKeyedHashSet(); + levelCache.putUnsafe(result, endOffset); + cache.putUnsafe(offset, levelCache); } return result; diff --git a/src/org/rascalmpl/parser/gtd/result/out/SortContainerNodeFlattener.java b/src/org/rascalmpl/parser/gtd/result/out/SortContainerNodeFlattener.java index d2dcc72e78..2efa876f26 100644 --- a/src/org/rascalmpl/parser/gtd/result/out/SortContainerNodeFlattener.java +++ b/src/org/rascalmpl/parser/gtd/result/out/SortContainerNodeFlattener.java @@ -12,18 +12,18 @@ package org.rascalmpl.parser.gtd.result.out; import java.net.URI; +import java.util.HashMap; +import java.util.Map; import org.rascalmpl.parser.gtd.location.PositionStore; import org.rascalmpl.parser.gtd.result.AbstractNode; import org.rascalmpl.parser.gtd.result.SortContainerNode; import org.rascalmpl.parser.gtd.result.action.IActionExecutor; -import org.rascalmpl.parser.gtd.result.out.INodeFlattener.CycleMark; import org.rascalmpl.parser.gtd.result.struct.Link; import org.rascalmpl.parser.gtd.util.ArrayList; import org.rascalmpl.parser.gtd.util.ForwardLink; import org.rascalmpl.parser.gtd.util.IndexedStack; import org.rascalmpl.parser.gtd.util.IntegerKeyedHashMap; -import org.rascalmpl.parser.gtd.util.ObjectIntegerKeyedHashMap; import org.rascalmpl.parser.gtd.util.ObjectIntegerKeyedHashSet; /** @@ -32,43 +32,71 @@ public class SortContainerNodeFlattener{ @SuppressWarnings("unchecked") private final static ForwardLink NO_NODES = ForwardLink.TERMINATOR; - - private final IntegerKeyedHashMap> preCache; + private final IntegerKeyedHashMap> cache; - + + private final Map> linkCache; + public SortContainerNodeFlattener(){ super(); - preCache = new IntegerKeyedHashMap>(); - cache = new IntegerKeyedHashMap>(); + cache = new IntegerKeyedHashMap<>(); + linkCache = new HashMap<>(); } /** * Gather all the alternatives ending with the given child. */ - private void gatherAlternatives(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private void gatherAlternatives(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment, boolean hasSideEffects){ + ArrayList gatheredAlts = null; + + if (!hasSideEffects) { + gatheredAlts = linkCache.get(child); + boolean found = true; + if (gatheredAlts == null) { + found = false; + gatheredAlts = new ArrayList<>(); + gatherAlternativesUncached(converter, nodeConstructorFactory, child, gatheredAlts, production, stack, + depth, positionStore, sourceLocation, offset, endOffset, filteringTracker, + actionExecutor, environment); + } + for (int i=0; i converter, INodeConstructorFactory nodeConstructorFactory, Link child, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ AbstractNode resultNode = child.getNode(); if(!(resultNode.isEpsilon() && child.getPrefixes() == null)){ // Has non-epsilon results. - gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(NO_NODES, resultNode), gatheredAlternatives, production, stack, depth, cycleMark, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, child, new ForwardLink(NO_NODES, resultNode), gatheredAlternatives, production, stack, depth, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); }else{ // Has a single epsilon result. - buildAlternative(converter, nodeConstructorFactory, NO_NODES, gatheredAlternatives, production, stack, depth, cycleMark, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, NO_NODES, + gatheredAlternatives, production, stack, depth, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); } } /** * Gathers all alternatives for the given production related to the given child and postfix. */ - private void gatherProduction(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private void gatherProduction(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, Link child, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ ArrayList prefixes = child.getPrefixes(); if(prefixes == null){ // Reached the start of the production. - buildAlternative(converter, nodeConstructorFactory, postFix, gatheredAlternatives, production, stack, depth, cycleMark, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); + buildAlternative(converter, nodeConstructorFactory, postFix, gatheredAlternatives, production, stack, depth, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); return; } for(int i = prefixes.size() - 1; i >= 0; --i){ // Traverse all the prefixes (can be more then one in case of ambiguity). Link prefix = prefixes.get(i); - gatherProduction(converter, nodeConstructorFactory, prefix, new ForwardLink(postFix, prefix.getNode()), gatheredAlternatives, production, stack, depth, cycleMark, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherProduction(converter, nodeConstructorFactory, prefix, new ForwardLink(postFix, prefix.getNode()), gatheredAlternatives, production, stack, depth, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); } } @@ -76,7 +104,7 @@ private void gatherProduction(INodeFlattener converter, INodeConstructorFa * Construct the UPTR representation for the given production. * Additionally, it handles all semantic actions related 'events' associated with it. */ - private void buildAlternative(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + private void buildAlternative(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, ForwardLink postFix, ArrayList gatheredAlternatives, Object production, IndexedStack stack, int depth, PositionStore positionStore, S sourceLocation, int offset, int endOffset, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ Object newEnvironment = actionExecutor.enteringProduction(production, environment); // Fire a 'entering production' event to enable environment handling. int postFixLength = postFix.length; @@ -87,7 +115,7 @@ private void buildAlternative(INodeFlattener converter, INodeConstructorFa newEnvironment = actionExecutor.enteringNode(production, i, newEnvironment); // Fire a 'entering node' event when converting a child to enable environment handling. - T constructedNode = converter.convert(nodeConstructorFactory, node, stack, depth, cycleMark, positionStore, filteringTracker, actionExecutor, environment); + T constructedNode = converter.convert(nodeConstructorFactory, node, stack, depth, positionStore, filteringTracker, actionExecutor, environment); if(constructedNode == null){ actionExecutor.exitedProduction(production, true, newEnvironment); // Filtered. return; @@ -115,28 +143,14 @@ private void buildAlternative(INodeFlattener converter, INodeConstructorFa /** * Converts the given sort container result node to the UPTR format. */ - public T convertToUPTR(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, SortContainerNode

node, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ + public T convertToUPTR(INodeFlattener converter, INodeConstructorFactory nodeConstructorFactory, SortContainerNode

node, IndexedStack stack, int depth, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object environment){ int offset = node.getOffset(); int endOffset = node.getEndOffset(); Object firstProduction = node.getFirstProduction(); Object rhs = nodeConstructorFactory.getRhs(node.getFirstProduction()); boolean hasSideEffects = actionExecutor.isImpure(rhs); - - if(depth <= cycleMark.depth){ // Only check for sharing if we are not currently inside a cycle. - if(!hasSideEffects){ // If this sort node and its direct and indirect children do not rely on side-effects from semantic actions, check the cache for existing results. - ObjectIntegerKeyedHashMap levelCache = preCache.get(offset); - if(levelCache != null){ - T cachedResult = levelCache.get(rhs, endOffset); - if(cachedResult != null){ - return cachedResult; - } - } - } - - cycleMark.reset(); - } - + S sourceLocation = null; URI input = node.getInput(); if(!(node.isLayout() || input == null)){ // Construct a source location annotation if this sort container does not represent a layout non-terminal and if it's available. @@ -152,7 +166,6 @@ public T convertToUPTR(INodeFlattener converter, INodeConstructorFactory converter, INodeConstructorFactory gatheredAlternatives = new ArrayList(); - gatherAlternatives(converter, nodeConstructorFactory, node.getFirstAlternative(), gatheredAlternatives, firstProduction, stack, childDepth, cycleMark, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); + + gatherAlternatives(converter, nodeConstructorFactory, node.getFirstAlternative(), gatheredAlternatives, firstProduction, stack, childDepth, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment, hasSideEffects); ArrayList alternatives = node.getAdditionalAlternatives(); ArrayList

productions = node.getAdditionalProductions(); if(alternatives != null){ for(int i = alternatives.size() - 1; i >= 0; --i){ - gatherAlternatives(converter, nodeConstructorFactory, alternatives.get(i), gatheredAlternatives, productions.get(i), stack, childDepth, cycleMark, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment); + gatherAlternatives(converter, nodeConstructorFactory, alternatives.get(i), gatheredAlternatives, productions.get(i), stack, childDepth, positionStore, sourceLocation, offset, endOffset, filteringTracker, actionExecutor, environment, hasSideEffects); } } @@ -193,40 +207,25 @@ else if (nrOfAlternatives > 0) { // Ambiguous. stack.dirtyPurge(); // Pop this node off the stack. - if(result != null && depth < cycleMark.depth){ // Only share the constructed tree if we are not in a cycle. - if(!hasSideEffects){ // Cache side-effect free tree. - ObjectIntegerKeyedHashMap levelCache = preCache.get(offset); - if(levelCache != null){ - T cachedResult = levelCache.get(rhs, endOffset); - if(cachedResult != null){ - return cachedResult; - } - - levelCache.putUnsafe(rhs, endOffset, result); - return result; - } - - levelCache = new ObjectIntegerKeyedHashMap(); - levelCache.putUnsafe(rhs, endOffset, result); - preCache.put(offset, levelCache); - }else{ // Cache tree with side-effects. - ObjectIntegerKeyedHashSet levelCache = cache.get(offset); - if(levelCache != null){ - T cachedResult = levelCache.getEquivalent(result, endOffset); - if(cachedResult != null){ - return cachedResult; - } - - levelCache.putUnsafe(result, endOffset); - return result; + if (hasSideEffects) { + // Cache tree with side-effects to keep as much sharing as possible in the resulting parse forest + ObjectIntegerKeyedHashSet levelCache = cache.get(offset); + if(levelCache != null){ + T cachedResult = levelCache.getEquivalent(result, endOffset); + if(cachedResult != null){ + return cachedResult; } - levelCache = new ObjectIntegerKeyedHashSet(); levelCache.putUnsafe(result, endOffset); - cache.putUnsafe(offset, levelCache); + return result; } + + levelCache = new ObjectIntegerKeyedHashSet(); + levelCache.putUnsafe(result, endOffset); + cache.putUnsafe(offset, levelCache); } return result; } + } diff --git a/src/org/rascalmpl/parser/gtd/result/out/VoidNodeFlattener.java b/src/org/rascalmpl/parser/gtd/result/out/VoidNodeFlattener.java index 822fb8e04a..ba168a0644 100644 --- a/src/org/rascalmpl/parser/gtd/result/out/VoidNodeFlattener.java +++ b/src/org/rascalmpl/parser/gtd/result/out/VoidNodeFlattener.java @@ -33,7 +33,7 @@ public AbstractNode convert(INodeConstructorFactory nodeCo return parseTree; } - public AbstractNode convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode parseTree, IndexedStack stack, int depth, CycleMark cycleMark, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object rootEnvironment){ + public AbstractNode convert(INodeConstructorFactory nodeConstructorFactory, AbstractNode parseTree, IndexedStack stack, int depth, PositionStore positionStore, FilteringTracker filteringTracker, IActionExecutor actionExecutor, Object rootEnvironment){ return parseTree; }